🧹

clang-tidy の自動修正でレガシーコードを改善しよう!

2022/02/06に公開

clang-tidy の自動修正でレガシーコードを改善しよう!

コンパイル警告の山に埋もれていませんか?

-Wall を有効にして、コンパイル警告をすべて潰そう…昔から言われているプラクティスですが、引き継いだコードがすでに大量の警告を発している場合、膨大な修正量を前に立ちすくんでしまいます。

たとえ一度は重大な影響がないか総ざらいチェックしても、目視確認では見落としの可能性がありますし、ある時点でのチェック結果は、その後改変を重ねると無意味なものになってしまいます。

そのような場合、警告項目を限定することになりますが、以下のような課題が残ります。

  • C/C++形式のキャストの不統一など可読性に影響する項目は、動作不具合に繋がらないことが確認できれば修正は後回しになり、プログラマーの生産性を下げ続ける
  • そもそも修正の要否を判断する作業が、主観的なうえ生産性が低い

しかし本来は警告を大量に出してしまうようなコードこそ、静的解析で検出可能な不具合をはらんでいる可能性が高い、というジレンマがあります。高価な有償ツールを導入して回し続ける「意識の高い」隣の新規開発チームは、もともとコード品質が高いので滅多に警告が検出されず、古いコードを保守し続ける自分のチームは、いつも未定義の動作に悩まされ続ける、という笑えないに陥りがちです。

そんな状況を打破しうる非常に強力なツールが clang-tidy です。VSCode の C++ extension で標準的に使えるようになった ため、気軽に試してみることができます。

disclaimer

この記事では機能紹介を優先し、具体的な操作・設定方法については徐々に書き足していきたいと思っています。

clang-tidy

clang-tidy (クラング・タイディ)は clang をベースとした静的解析ツールです。無償で利用できる点が強調されがちですが、実は他の静的解析ツールにはあまりない機能として、 フォーマッタと静的解析をいいとこ取りしたような、魔法のコマンド引数 --fix が存在します。

このオプションを有効にすることで、ただ警告を出力するだけでなく、 修正案を自動的に適用 してくれるのです。clang の構文解析をベースにチェックが行われますので、インデントや括弧の位置などの書式に留まらない、多様で精度の高い修正が適用されます。まさに静的型付け言語の面目躍如という感じです。

たまに誤修正することがあるので、 変更内容の精査は欠かせません が、従来ある程度の C++ 知識を持つメンバーが手作業でやっていた修正が機械で代行できて、いきなりレビュー可能な状態に進むことができます。

類似機能

同様の機能は Visual Studio や CLion が quick fix として提供しているようです。GUIで承認/拒絶を選択できるのは魅力ですが、そのためにメンバー全員に IDE を乗り換えさせるのはハードルが高いかもしれません。

clang-tidy なら実行前後の差分を任意の diff ツールで確認するだけなので、既存の開発環境に簡単に追加できることが利点です。

その前に: clang-format

コードレビューのたびに、インデント位置や括弧のつけ方で指摘を出していませんか?

ルートディレクトリに “.clang-format” ファイルを設定しておけば、VSCode の “Format Document”(Alt+Shift+F) で一括成形可能です。そもそも規約がない場合は、 BasedOnStyle=google に設定することから始めるとよいのではないでしょうか。

また、コードリーディングの際に、GNU など自分好みの書式ではない場合、ローカルファイルを一括整形すれば、頭に入りやすくなると思います。

チェック項目の選定

運用

解析処理は重い(コンパイル時間の数倍〜10倍程度)ので、項目を絞った運用がおすすめです。
また、既存コードが大幅に変わると他メンバーの理解を妨げ反発を生みますので、既に合意できているルールの逸脱修正から始めると、導入がスムーズだと思われます。

一覧

チェック項目の一覧は 公式ドキュメント にあります。解説文だけでは想像がつかない場合、 github のテストコード がそのままサンプルとして読める形式になっています。

実行

解析には clang がコンパイルするのと同等の情報を使います。そのため書式設定ファイル1つでよい clang-format と異なり、インクルードディレクトリ(-I) やマクロ定義(-D) を -- オプション以降に渡す必要があります。

cmake 環境下では以下のオプションを有効にすることで、コンパイル設定を "compile_commands.json" というファイルに出力してくれます。

cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .

Makefile 環境では RedHat のブログ記事 のように Makefile に組み込んでしまうのが良いでしょう。

オプション

前述の “.clang-format” を配置済みの場合、 --format-style=file オプションによって、 fix 適用後自動的に整形してくれます。

Visual C++ 用の環境で unused-command-line-argument 警告などが大量に出て解析に失敗する場合、 --driver-mode=cl オプションを有効にすると改善するかもしれません。これは clang のコマンドライン引数を cl.exe 互換モードで解釈するものです。

また、デフォルトでは、複数の修正候補が提示される場合には、自動修正が行われません。 --fix-notes オプションで最初の候補を自動適用することができるようですが、正しく修正されているか注意が必要です。

おすすめのチェック項目

太字は特に効果が実感しやすい項目です。

潜在的バグかもしれないチェック項目

静的解析の一番の目的である、不具合の検出に該当するかもしれない項目です。
コード品質向上以前に、指摘箇所が既存不具合に繋がっている可能性があります。

  • bugprone-macro-parentheses
  • bugprone-redundant-branch-condition
  • cppcoreguidelines-init-variables
    • コンパイル警告の二大巨塔「変数の未初期化」にゼロなど挿入
    • windows は 浮動小数点型の初期値 NAN がないので MathHeader で自前定義の指定が必要
  • cppcoreguidelines-pro-type-member-init
    • 未初期化のメンバ変数を自動修正
    • クラス宣言ではなくコンストラクタで初期化させるには、コンパイルオプションで C++98 を指定する
  • misc-redundant-expression
  • bugprone-assert-side-effect : 自動修正ではない

今すぐ使ってみたいチェック項目

コーディング規約もしくは暗黙知として、実践されているであろうプラクティスのヌケモレを検出する項目です。

  • bugprone-copy-constructor-init
  • bugprone-virtual-near-miss
  • cppcoreguidelines-prefer-member-initializer
  • misc-unused-parameters
    • コンパイル警告の二大巨塔「未使用の引数」をコメントアウト
  • performance-for-range-copy
  • performance-inefficient-vector-operation
  • performance-type-promotion-in-math-fn
  • performance-unnecessary-copy-initialization, performance-unnecessary-value-param
    • 値渡しの引数を const 参照渡しに修正
    • cv::Mat など自身がメモリ管理するのでコピー可能なクラスは AllowedTypes で指定する
  • readability-else-after-return
  • readability-implicit-bool-conversion
  • readability-simplify-boolean-expr
    • if (cond()==true) などの無意味なブール演算を削除
    • clang の最適化が基礎なので、かなり賢く簡略化してくれる
  • readability-static-accessed-through-instance

レビューのたびに使わせたいチェック項目

コードレビューで頻出する初歩的な指摘を自動修正する項目です。レビュー毎に細かな体裁の修正指示を出す手間が減少します。

  • cppcoreguidelines-pro-type-cstyle-cast
  • cppcoreguidelines-virtual-class-destructor
  • llvm-include-order
    • ヘッダファイルの並び順をソート。cpplint で指摘されても、なかなか手が回らないですよね。
  • misc-definitions-in-headers
  • modernize-redundant-void-arg
  • modernize-use-bool-literals
  • portability-restrict-system-includes
  • readability-convert-member-functions-to-static
    • コードレビュー指摘でよくある、 this へのアクセスがない関数を static
  • readability-make-member-function-const, readability-non-const-parameter
    • コードレビュー指摘でよくある、 引数や関数の const 化を徹底
  • readability-qualified-auto
  • readability-redundant-control-flow
  • readability-redundant-member-init
  • readability-const-return-type

レガシーコードから脱却するためのチェック項目

既存の C++98 コードを C++11 以降にモダン化する際に役立つ項目です。新製品への旧機能移植などの際に、既存コードの表記に全体が引きずられることを抑止します。

  • misc-static-assert
  • modernize-make-shared
  • modernize-make-unique
  • modernize-use-auto
  • modernize-use-default-member-init
    • コンストラクタでのメンバ変数初期化をクラス宣言に移動
    • 手作業でやるのはとても大変
  • modernize-use-emplace
  • modernize-use-equals-delete
  • modernize-use-nullptr
  • modernize-use-override
  • modernize-use-using

賛否が別れそうなチェック項目

  • modernize-loop-convert
    • range-based for に自動修正する、非常に強力な機能
    • C++11 の象徴的な機能だが、項目解説 にある通り、同一挙動を維持できない可能性があるうえ、差分チェックでの判断が難しい
  • modernize-use-trailing-return-type
    • 関数戻り値を east end に置換する
    • 既存コードがかなり変わるので、よく議論が必要
  • readability-uppercase-literal-suffix
    • 数値リテラル末尾の L, F などを大文字で統一

命名規則の自動適用

readability-identifier-naming を規約に合わせて設定することで、無秩序になりがちなローカル変数の snake_case や camelCase を統一することができます。(参考: clang-tidyで命名規則のチェック(&自動修正))

ちなみに ReShaper は C#なら命名規約を自動検出して適用できる らしいです。

Discussion