warning絶許マンがCIで捗る話し

# はじめに

私は「warning絶許マン」ですが CI 使ったら色々捗った情報（小ネタ）を共有します。

# 前提環境

- ローカル: macOS
- 開発言語: C++
- リポジトリ: https://github.com/suzukiplan/z80
- CI: CircleCI

# CircleCI configuration

```.circleci/config.yml
version: 2.1

jobs:
  test:
    docker:
      - image: cimg/base:stable
    steps:
      - checkout
      - run:
          name: "Install Prerequests"
          command: "sudo apt-get install -y clang g++-10"
      - run:
          name: "Build and Test Z80 Emulator"
          command: "make ci"

workflows:
  test-workflow:
    jobs:
      - test
```

- ワークフローはシンプルに `test` のみ
- test では前提モジュール（clang と g++-10）を `apt-get` してから `make ci` で Makefile を実行するだけ

```Makefile.
all:
	clang-format -style=file < z80.hpp > z80.hpp.bak
	cat z80.hpp.bak > z80.hpp
	rm z80.hpp.bak
	cd test && make
	cd test-ex && make zexall

ci:
	cd test && make
	cd test-ex && make ci
```

- ローカル: `make [all]` でメインモジュールを clang-format で整形してからテスト実行
- CI: clang-format での整形を省略しつつテスト実行

`text-ex/Makefile` の実行手順がローカルとCIで異なっています。

- ローカル: `cd test-ex && make zexall`
- CI: `cd test-ex && make ci`

```text-ex/Makefile.
CFLAGS=-I../ \
	-std=c++11 \
	-O2 \
	-Wall \
	-Wfloat-equal \
	-Wshadow \
	-Wunused-variable \
	-Wsign-conversion \
	-Wtype-limits \
	-Werror

all: cpm zexdoc zexall

clean:
	-rm cpm

cpm: cpm.cpp ../z80.hpp
	clang $(CFLAGS) cpm.cpp -lstdc++ -o cpm

zexdoc: cpm
	./cpm -e zexdoc.cim

zexall: cpm
	./cpm -e zexall.cim

ci:
	g++-10 $(CFLAGS) -Wclass-memaccess cpm.cpp -lstdc++ -o cpm
	./cpm -e -n zexall.cim

full: cpm
	./cpm zexall.cim
```

`-Werror`はwarning絶許マンの証し...

プロジェクトを作る時、息を吐くようにこのオプションを追加するようになればモノホンです。

流石に途中参加したプロジェクトで勝手に付けたりすることはありませんが、付けることを提案するタイミングを息を殺して覗います。

あとは **ローカルでは clang、CI では g++-10** でビルドしている点も重要な捗りポイントです。

- ローカル: `clang $(CFLAGS) cpm.cpp -lstdc++ -o cpm`
- CI: `g++-10 $(CFLAGS) -Wclass-memaccess cpm.cpp -lstdc++ -o cpm`


# 捗るポイント

macOS だと g++-10 (GCC10) を導入するのが若干面倒なので、基本的にAppleが提供している clang のみを普段（ローカル）のビルドやテストで使っています。

> _そろそろメインマシンをmacOSからLinuxに変えたいのですが、スマホアプリ（iOS）の開発をやっている内はmacOSから抜け出せません..._

現時点だと GCC10 の方が先進的なので、同じコードをコンパイルした時に warning が沢山出やすいので便利です。

Linuxなら導入は簡単なので、 **CI（Linux）の方で GCC10（+別のテストではclangのダブル）で警告を拾う** ようにしました。

`-Werror` を指定しているので警告が1つでも出ればバッチリCIがコケてくれます :+1:

# 捗った具体例

[コチラの Pull Request](https://github.com/suzukiplan/z80/pull/48) の [commit b6d3316](https://github.com/suzukiplan/z80/pull/48/commits/b6d3316069795525dc71894056ae95eb9628d40b) で実際に良い感じに GCC10 でのみ警告（-Werrorでエラー）になり CI がコケてました。
![image.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/2161417/cbcfc1b1-4240-91c3-a3d8-9ec1d9d1c4d1.png)

[CircleCI の Details](https://app.circleci.com/pipelines/github/suzukiplan/z80/56/workflows/5d82ede0-f4d6-4791-952e-9646e90a80ff/jobs/51)を開けば、どこが警告でコケたか一目瞭然です。
![image.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/2161417/0e150803-3db1-dda9-d922-a9f1326a960b.png)

同じ変数に対する後置インクリメント（`ctx->reg.SP++`）を同じ行内の関数呼び出しで2回呼んでいます。（[ココ](https://github.com/suzukiplan/z80/blob/b6d3316069795525dc71894056ae95eb9628d40b/z80.hpp#L776)）

```c++
unsigned short hl = ctx->make16BitsFromLE(
    ctx->readByte(ctx->reg.SP++ /* ← 1回目の++ */, 3),
    ctx->readByte(ctx->reg.SP++ /* ← 2回目の++ */, 3));
```

これは普通に良くないですね。

ありがとう、GCC10。

_という訳でマッハで修正（[commit a138156](https://github.com/suzukiplan/z80/pull/48/commits/a13815695a550ea6a235aa67f308fe722156acb9)）_

# 要点

- CIで複数のコンパイラ（clang, gcc）でコンパイルを通せばキャッチアップできる警告が増えて捗る
- `-Werror` しておけば CI でコケるので警告を残したままマージができなくて捗る

人手ではなく機械的にちゃんとチェックアウトできるのが良いですね。

ただし、せっかく警告を直しても修正ミスでデグったらシャレにならないので、以下の記事でも書いている **デグレードを自動的にチェックアウトする仕組みとのセットでの対応** が肝要かと思われます。

https://qiita.com/suzukiplan/items/4373e39b2ed22a66840b

# 余談（ポエム）

非warning絶許マンの方々からしばしば、

_警告が出ていても正常にプログラムが動作できているんだから、わざわざデグレードリスクを負ってまで直さなくても良いんじゃね？_

というご指摘を頂きます。

デグらない最大の秘訣はコードを弄らないことなので、そのご指摘はごもっともです。

しかし、それでもwarningは1件残らず潰さなければならないと思います。

というのも、私の経験上 **新規作り込みで発生したwarningの見落としで事故に繋がるケース** が割と多いので。以前勤めていた会社では、社外事故が起きるとその原因追求をした報告書が全社的に共有されていたのですが、根本的な作り込み原因は __「それwarningをちゃんと直していれば作り込まなかったのでは？」__ というものが結構多かった気がします。（全部の報告書をコードまで追いかけて見たり統計取った訳ではないので単なる肌感ですが）

なので、既存処理の部分のwarningは必ずしも全て直さなくて良いかもしれませんが、最低限 **「新規で作り込んだ部分のwarningは全部直すべき」** だと思われます。

warning抑止のオプションは、新規で作り込んだ部分の警告も出さなくなってしまうので論外です。（警告抑止オプションは、修正が不可能なサードパーティーのモジュールのwarningを抑止する時限定で使うものだと思います）

既に何百、何千とwarningが出ている場合、新規のwarningを見落とさずにチェックするのは少なくとも目視でのチェックはほぼ無理だと思います。特にC++のテンプレート系の警告は情報量が無駄に多く、重要な警告（C++ではなくCの警告は致命的なものが多い）を見落とすスケープゴートになりやすいです。

という訳で、warningは全部潰して `-Werror` で warning をエラー化するのが、新規で作り込んだwarningを確実（機械的）にチェックアウトできる最も合理的な方法だと思います。

_まぁ、数百件程度ならともかく、数千とか数万だと流石に気が遠くなりますが...（それは流石に技術的負債レベルだからちゃんと予算取って直しましょうと言いたい）_

プログラムのテスト工程は、

1. コードレビュー（机上テスト）
2. ユニットテスト
3. 結合テスト
4. システムテスト

という流れが一般的で、

- プログラム起因の摘出バグ件数は後工程になるほど減っていく事が理想形
- バグ対策に要するコスト（費用・時間）は後工程になるほど増大する

というのが一般論かと思われます。（[参考](https://www.ipa.go.jp/files/000049404.pdf)）

実際、「後工程での摘出バグ件数が収束傾向にならなかったら前工程にもどって見直し&再検査をしないと次工程に進めない」という定量ルールを設定している現場もあります。_拝承です。_

コンパイラが出してくれる warning は言わば **「機械によるコードレビュー」** なので、そこで bugfix しておけばバグ対策に要するトータルコストの大幅な削減が期待できます。そのため、全ての warning を潰した方が良い...と、warning絶許マンは思います。