プログラミング、リファクタリング、そしてすべてにおける究極の疑問: No. 10

2018.07.10
その他インテル® DPC++/C++ コンパイラー特集
C/C++エラー検証実装

この記事は、インテル® デベロッパー・ゾーンに公開されている「The Ultimate Question of Programming, Refactoring, and Everything」の日本語参考訳です。

10. 複数の小さな #ifdef ブロックの使用は避ける

CoreCLR プロジェクトから抜粋した以下のコードについて考えてみます。このエラーは、次の PVS-Studio 診断によって検出されます。

V522 Dereferencing of the null pointer ‘hp’ might take place. (V522 null ポインター ‘hp’ を逆参照している可能性があります。)

heap_segment* gc_heap::get_segment_for_loh (size_t size
#ifdef MULTIPLE_HEAPS
                                           , gc_heap* hp
#endif //MULTIPLE_HEAPS
                                           )
{
#ifndef MULTIPLE_HEAPS
    gc_heap* hp = 0;
#endif //MULTIPLE_HEAPS
    heap_segment* res = hp->get_segment (size, TRUE);
    if (res != 0)
    {
#ifdef MULTIPLE_HEAPS
        heap_segment_heap (res) = hp;
#endif //MULTIPLE_HEAPS
  ....
}

説明

私は、#ifdef/#endif 構文は必要悪であると考えています。これらの構文は必要であり、使用しなければならない場合があります。そのため、#ifdef を使用しないように促すつもりはありません。「過度に」使用しないように注意して欲しいのです。

多くの方は、#ifdefs で埋め尽くされているコードをご覧になったことがあるかと思います。特に、#ifdef が 10 行ごとあるいはそれよりも頻繁に使用されているコードを扱うのは苦痛です。通常、そのようなコードはシステム依存で、#ifdef を使用せずに処理を行うことはできません。だからと言って、状況が良くなるわけではありません。

コードが読みづらいことに変わりはありません。上記のコードが良い例です。コードを理解することは、プログラマーがしなければならない基本作業です。実際、プログラマーは、新しいコードを記述するよりもはるかに多くの時間を、既存のコードの理解と調査に費やします。そのため、読みづらいコードはプログラマーの効率を大幅に下げ、ミスの可能性を高めます。

上記のコード例では、null ポインターの逆参照でエラーが見つかり、MULTIPLE_HEAPS マクロが宣言されていないときに問題が発生します。分かりやすくするため、マクロを展開してみましょう。

heap_segment* gc_heap::get_segment_for_loh (size_t size)
{
  gc_heap* hp = 0;
  heap_segment* res = hp->get_segment (size, TRUE);
  ....

プログラマーは、hp 変数を宣言して、NULL に初期化し、直後に逆参照しています。MULTIPLE_HEAPS が定義されていない場合、このコードはエラーになります。

正しいコード

このエラーは、私の同僚が「CoreCLR の 25 の疑わしいコード」 (英語) で報告しましたが、CoreCLR (12.04.2016) ではまだ存在 (英語) しているため、最良の修正方法は分かりません。

私の考えでは、(hp == nullptr) なので、’res’ 変数をほかの値に初期化すべきだと思います。ただ、どの値にすべきかは分かりません。そのため、このエラーについては、現時点で解決方法はありません。

推奨事項

小さな #ifdef/#endif ブロックは、コードを読みづらく、理解しづらくするため、コードから排除します。#ifdefs の林を含むコードは、保守が困難でミスを引き起こしやすくします。

すべてのケースに適合する推奨はありません。すべて特定の状況に依存します。#ifdef は問題の原因となるため、コードをできるだけ明確に保つよう常に努力する必要があることを覚えておいてください。

ヒント N1: #ifdef は使用しないようにします。

#ifdef は、場合によっては、定数と通常の if 文に置き換えることができます。次の 2 つのコードを比較してみます。1 つ目のコードは、マクロを使用するバージョンです。

#define DO 1

#ifdef DO
static void foo1()
{
  zzz();
}
#endif //DO

void F()
{
#ifdef DO
  foo1();
#endif // DO
  foo2();
}

このコードは読みづらく、読もうとする気にもなりません。皆さんもスキップしませんでしたか? 次のマクロを使用しないバージョンと比較します。

const bool DO = true;

static void foo1()
{
  if (!DO)
    return;
  zzz();
}

void F()
{
  foo1();
  foo2();
}

非常に読みやすくなりました。関数呼び出しとチェックの追加により、コードの効率が低下したと言う方がいるでしょう。私はそうは思いません。1 つ目の理由として、現代のコンパイラーは非常に洗練されており、リリースバージョンでは、追加のチェックや関数呼び出しを行わなくてもおそらく同じコードになるでしょう。2 つ目に、パフォーマンス低下の可能性はわずかで、懸念するほどのものではありません。簡潔で分かりやすいコードのほうが重要です。

ヒント N2: #ifdef ブロックを大きくします。

get_segment_for_loh() 関数を記述する場合、複数の #ifdefs を使用する代わりに、私だったら 2 つのバージョンを作成します。コードの量は増えますが、そのほうが分かりやすく、編集しやすくなります。

重複コードだと言う方がいるかもしれません。また、#ifdef を含む長い関数がたくさんあるため、関数に 2 つのバージョンがあると、変更時に1 つのバージョンのみを変更し、もう一方のバージョンを変更するのを忘れてしまう可能性があるという方もいるでしょう。

そもそも、関数が長いのはなぜでしょうか? 一般的なロジックを個別の補助関数にすることで、関数の 2 つのバージョンが短くなり、違いが明確になります。

このヒントは万能薬ではありませんが、検討してみてください。

ヒント N3: テンプレートの使用を検討してください。役立つことがあります。

ヒント N4: #ifdef を使用する前に、慎重に検討します。場合によっては、使用しなくても済むかもしれません。あるいは、#ifdefs の数を減らし、この「悪因」を 1 個所にまとめます。

< 前へ 目次 次へ >

コンパイラーの最適化に関する詳細は、最適化に関する注意事項を参照してください。

タイトルとURLをコピーしました