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

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

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

39. 不正なコードが動作する理由

この問題は、Miranda NG プロジェクトで見つかりました。このコードにはエラーが含まれています。PVS-Studio アナライザーは、次の診断を出力します。

V502 Perhaps the ‘?:’ operator works in a different way than was expected. The ‘?:’ operator has a lower priority than the ‘|’ operator. (V502 ‘?:’ 演算子の動作が想定と異なる可能性があります。’|’ 演算子のほうが ‘?:’ よりも優先されます。)

#define MF_BYCOMMAND 0x00000000L
void CMenuBar::updateState(const HMENU hMenu) const
{
  ....
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
    MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED);
  ....
}

説明

これまで、プログラムの不正な動作につながる多くの問題を見てきました。ここでは、別の考えさせられるトピックについて述べたいと思います。場合によっては、完全に間違ったコードが、あらゆる予想に反して、問題なく動作することがあります。経験豊富なプログラマーにとってこれは驚くべきことではありませんが、C/C++ を習い始めたばかりの初心者は困惑させられるでしょう。ここでは、そのような例について考えます。

上記のコードでは、特定のフラグを設定して CheckMenuItem() を呼び出す必要があります。そして、bShowAvatar が true の場合は MF_BYCOMMAND と MF_CHECKED のビット単位の OR (論理和) を、false の場合は MF_BYCOMMAND と MF_UNCHECKED のビット単位の OR (論理和) を計算します。単純でしょう!

上記のコードでプログラマーは、非常に自然な三項演算子 (if-then-else の便利な省略形) を選択して以下を表現しています。

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

“|” 演算子の優先順位は、”?:” 演算子よりも高いため (詳細は、「C/C++ の操作の優先順位」 (英語) を参照)、一度に 2 つのエラーが発生します。

1 つ目のエラーは、条件の変更です。”dat->bShowAvatar” ではなく、”MF_BYCOMMAND | dat->bShowAvatar” になります。

2 つ目のエラーは、MF_CHECKED または MF_UNCHECKED のいずれか 1 つのフラグのみが選択されることです。MF_BYCOMMAND フラグは無視されます。

しかし、これらの問題があるにもかかわらず、コードは正しく動作します。その理由は、単に運が良かっただけです。幸運にも、MF_BYCOMMAND (英語) フラグが 0x00000000L と等しかったのです。MF_BYCOMMAND フラグが 0 と等しい場合、コードへの影響はありません。経験豊富なプログラマーの方はすでにお分かりでしょうが、初心者のために説明したいと思います。

最初に、括弧を追加した正しい式を見てみましょう。

MF_BYCOMMAND | (dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED)

マクロを数値に置き換えます。

0x00000000L | (dat->bShowAvatar ? 0x00000008L : 0x00000000L)

| 演算子のオペランドの 1 つがゼロの場合、式を簡素化できます。

dat->bShowAvatar ? 0x00000008L : 0x00000000L

不正なコードについて詳しく見てみます。

MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED

マクロを数値に置き換えます。

0x00000000L | dat->bShowAvatar ? 0x00000008L : 0x00000000L

部分式 “0x00000000L | dat->bShowAvatar” で | 演算子のオペランドの 1 つがゼロです。式を簡素化します。

dat->bShowAvatar ? 0x00000008L : 0x00000000L

結果、同じ式になり、これがエラーを含むコードが正しく動作する理由です。また 1 つプログラミングの奇蹟が起こりました。

正しいコード

正しいコードの記述方法はさまざまです。その 1 つは括弧を追加することで、別の方法として中間値を追加することもできます。古き良き if 演算子も役立ちます。

if (dat->bShowAvatar)
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR, 
                  MF_BYCOMMAND | MF_CHECKED);
else
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
                  MF_BYCOMMAND | MF_UNCHECKED);

私は、この方法でコードを修正するように主張しているわけではありません。読みやすいかもしれませんが、長いため、好みの問題です。

推奨事項

単純に、複雑な式、特に三項演算子を使用しないようにします。括弧を追加するのも忘れないでください。

「4. ?: 演算子に注意して、括弧で囲む」で述べたように、?: は非常に危険です。優先順位が非常に低いことをうっかり忘れてしまい、不正な式を記述してしまう場合があります。この演算子は、文字列を詰める場合に使用される傾向にありますが、その場合この演算子を使用しないようにします。

< 前へ 目次 次へ >

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

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