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

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

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

1. コンパイラーの仕事をしない

MySQL* プロジェクトから抜粋した以下のコードについて考えてみます。このコードにはエラーが含まれています。PVS-Studio Analyzer は、次の診断を出力します。

V525 The code containing the collection of similar blocks. Check items ‘0’, ‘1’, ‘2’, ‘3’, ‘4’, ‘1’, ‘6’ in lines 680, 682, 684, 689, 691, 693, 695. (V525 コードに類似ブロックのコレクションが含まれています。行 680、682、684、689、691、693、695 の項目 ‘0’、’1’、’2’、’3’、’4’、’1’、’6′ を確認してください。)

static int rr_cmp(uchar *a,uchar *b)
{
  if (a[0] != b[0])
    return (int) a[0] - (int) b[0];
  if (a[1] != b[1])
    return (int) a[1] - (int) b[1];
  if (a[2] != b[2])
    return (int) a[2] - (int) b[2];
  if (a[3] != b[3])
    return (int) a[3] - (int) b[3];
  if (a[4] != b[4])
    return (int) a[4] - (int) b[4];
  if (a[5] != b[5])
    return (int) a[1] - (int) b[5];     <<<<====
  if (a[6] != b[6])
    return (int) a[6] - (int) b[6];
  return (int) a[7] - (int) b[7];
}

説明

これは、コードブロックのコピーに起因する古典的なエラーです (コピー & ペースト (英語))。プログラマーは、コードブロック “if (a[1] != b[1]) return (int) a[1] – (int) b[1];” をコピーして、インデックスを変更しましたが、”1″ を “5” に変更するのを忘れてしまったようです。そのため、比較関数が不正な値を返すことがあります。この問題は、気付くのも、検出するのも困難です。実際、PVS-Studio で MySQL* をスキャンするまで、すべてのテストでこの問題は検出されませんでした。

正しいコード

if (a[5] != b[5])
  return (int) a[5] - (int) b[5];

推奨事項

簡潔で読みやすいコードですが、プログラマーがエラーを見落とすのを防ぐことはできませんでした。このように類似ブロックが続く場合、コードの細部に集中することは困難です。

このような類似したブロックは、通常、コードをできるだけ最適化したいというプログラマーの願望によるものです。上記の例では、手動でループをアンロールしていますが、このケースでは良い選択肢であるとは言えません。

1 つ目の理由は、そうすることでプログラマーが何か達成できたとは思えないからです。現代のコンパイラーは非常に洗練されており、プログラムのパフォーマンスを向上できる場合、自動でループをアンロールします。

2 つ目の理由は、コードを最適化しようとしたため、このバグが発生したことです。単純なループを記述していれば、ミスを犯す可能性は低くなります。

この関数を次のように書き直すことを推奨します。

static int rr_cmp(uchar *a,uchar *b)
{
  for (size_t i = 0; i < 7; ++i)
  {
    if (a[i] != b[i])
      return a[i] - b[i]; 
  }
  return a[7] - b[7];
}

利点:

  • 読みやすく、理解しやすい関数
  • 記述時にミスをする可能性が低い

この関数が、オリジナルのコードよりも遅くなることはないでしょう。

私のアドバイスは、単純で分かりやすいコードを記述することであり、原則として単純なコードは概して正しいコードです。コンパイラーの仕事 (例えば、ループアンロール) は横取りしないことです。コンパイラーは、プログラマーの助けがなくてもその役割を果たすことができます。このような細かい手動による最適化は、プロファイラーが問題 (遅いこと) を推定した、特に重要なコードでのみ意味があります。

< 前へ 目次 次へ >

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

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