えびちゃんの日記

えびちゃん(競プロ)の日記です。

コンパイラの警告を無視してバグらせる人かわいそう

無視されるコンパイラちゃんもかわいそうです。

とはいえ、「ここのコードこうなってるよ」という警告の仕方だけされても(特に初心者が)困ってしまうのは当然かなという気もします。 コンパイラが気にしてくれている理由や、実際にバグっているコードでその警告が出る例などを以下に挙げるので、「たしかにそれは無視するとまずいかもしれないな」という気持ちになる助けになってくれたらうれしいです。

解説のコーナー

未定義動作の可能性があるのを気にして警告してくれているケースや、そういうわけではないがよくあるバグの原因を警告してくれているケースがあります。ここで挙げる大半は後者です。

GCC では、コンパイル時に -Wall などのオプションをつけることで大抵の警告が有効になります。-Wextra をつけないと有効にならない警告もあります。 -Wextra をつけていても有効にならない警告もあります。たとえば -Wfloat-equal-Wdouble-promotion というのがあるみたいです。

以下を見るといいかもしれません。

gcc.gnu.org

警告は個別に -Wunused-parameter のように追加したり、-Wno-unused-parameter のように除外することもできます。

他のコンパイラを使っている人は別途調べてみるといいかもしれません。 とにかく、(やばいコードを書いても)警告が出ない環境でコードを書くのはおすすめしません。

-Wunused

-Wunused-parameter-Wunused-variable などいくつか種類がありますが、宣言・定義されたのに使われていないものに対する警告です。 「使ってないものが残ってたって別によくない?」と言う人がいるかもしれないのですが、使うべきものを使い漏らすことはまぁよくあると思います。

int dot(int x11, int x12, int x21, int x22) {
    return x11 * x12 + x12 * 22;
    // 実際に意図されていたのは x11 * x21 + x12 * x22 などのはず。
    // x21 が x12 になっているのに加え、x22 も 22 と typo している。
}

使わなくてよくなった変数は消す(せめてコメントアウト)方が、unused 系の警告を無視する癖をつけないようにできてよいと思います。

-Wmisleading-indentation

実際の挙動とは異なるインデントに対する警告です。 「別にそんなのよくない?」という人が目に見えるのですが、実際にあった脆弱性から導入された 警告です。

    if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
        goto fail;
        goto fail;
    /* more checking here.  */
  fail:
    /* cleanups */
    return err;

goto fail; が実行されるとエラー時の処理が走るのですが、二つめの goto fail; は if 文に含まれておらず常に実行されてしまいます。

-Wsign-compare

整数同士の比較を行う際、片方(たとえば左辺)は符号つき整数型で、他方(たとえば右辺)は符号なし整数型だったときに出てくる警告です。 これも無視している人がたくさんいると思います。これ起因のバグのうち競プロで頻出のものは次のようなものです。

bool has_xx(std::string s) {
    for (int i = 0; i < s.length() - 1; ++i) {
        if (s[i] == s[i + 1]) {
            return true;
        }
    }
    return false;
}

s.length() の部分は符号なし整数型なので、符号あり整数型である i とは符号の有無が異なります。

まず前提として、C++ においてはこういう状況では、型の符号の有無(signed か unsigned かということで、signedness という呼ばれ方をします)が統一されてから比較が行われます*1。 どちらに合わせるかは状況によりますが、多少複雑なので、下手に覚えると誤ってしまいそうな気もします。たとえば i < int(s.length()) - 1 のように signedness を陽に統一してしまう方が安全そうです。int に収まらないサイズだった場合はこれまたバグの原因ですが、競プロでそういうサイズを扱うことはほぼないと思いますので。

型を合わせる部分に関して

片方が浮動小数点数型の場合は、そちらに合わせられます。両方が浮動小数点数型だった場合は float < double < long double の順で強いです。 以下ではどちらも整数型だとします。

bool < signed char < short < int < long < long long の順で conversion rank というのが定められています。signed Tunsigned T の conversion rank は同じです。charsigned char unsigned char も同じ conversion rank です。

int より conversion rank の低い型に対しては、多くの場面では integer promotion というのがまず適用されます。

  • signed charshortint になる
  • unsigned charunsigned char は、int でその型の値すべてを表せるなら int、そうでないなら unsigned int になる
  • char は signed なら signed char、unsigned なら unsigned char 同様に変換される
  • boolint になる

そのあと integer conversion が行われます。

  • 両方が signed または両方が unsigned ならば、conversion rank が低い方の型が、高い方の型に変換される。
  • そうではなく、unsigned の方の conversion rank が signed の conversion rank 以上だった場合、signed の方の型が unsigned の方の型に変換される。
  • そうではなく、signed の方の型で unsigned の方の型の値すべてを表せる場合、unsigned の方の型が signed の方の型に変換される。
  • そうではない場合、両方の型が signed の方の型の unsigned 版に変換される。

たとえば、intunsigned long だった場合、二つめの規則により intunsigned long になります。 long (64-bit) と unsigned int (32-bit) だった場合、三つめの規則により unsigned intlong になります。 long (32-bit) と unsigned int (32-bit) だった場合、四つめの規則に longunsigned long かつ unsigned intunsigned long になります。...だと思います。

整数のビット長は一般に処理系定義ですが、有名な処理系においては long だけが異なりがちです。AtCoder の環境では 64-bit、Codeforces の環境では 32-bit なのが有名でしょう。

さて、上記のコードの該当箇所では多くの処理系では unsigned の方に合わせられるはずです。すなわち、たとえば unsigned(i) < s.length() - 1 のようになります。 このとき、s.is_empty() の状況では unsigned(i) < unsigned(-1) のようになります。unsigned(-1) はその符号なし整数型の最大値になるので、意図せず i が大きい値までループする(実際には s が空なので、s[1] にアクセスした時点で未定義動作になります(s[0]'\0' が返るのが保証されているため))ことになってしまいます。

-Wreorder

以下のように、メンバ変数の宣言順とコンストラクタ(の : name(value), ... で初期化する部分)の初期化順が異なるときの警告です。

struct Foo {
    int b, a;
    Foo(int x): a(x++), b(x++) {}
};

これだけ聞いても「別によくない? 合わせた方が気持ちいいでしょというお気持ち押しつけか?」以外の気持ちにならない気がします。 実際には、メンバ変数の宣言順に合わせて初期化されることになっているため、上記でたとえば Foo(1) とすると Foo { b: 1, a: 2 } のように初期化されてしまいます。 これはおそらく人間様の意図に沿っていないでしょうから、警告を出してくれているというわけです。

初期化時に副作用がないコードを書いていればあまり影響がない気もしますが、あまり自信はありません。なにか忘れているかもしれません。

-Wsequence-point

C++ には “Sequenced before” rules と呼ばれる概念があります*2。 これは “evaluation A is sequenced before evaluation B” という形式で記述され、「処理 A は、処理 B が開始されるよりも前に完了される」ということを規定しています。

この順序関係以外には、“A and B are unsequenced”(処理が並行して行われうる)や “A and B are indeterminately sequenced”(順序は決まっておらず、実行ごとに前後することもありうるが、片方が終わってから他方が始まることは保証される)というのがあります。

int f(int i, std::vector<int> const& a, std::vector<int> const& b) {
    int x = a[i++] + b[i++];
    int y = a[i++] + b[i];
    return x + y;
}

さて、i++ のような副作用*3を持つ処理があったとき、その処理に対して unsequenced な別の処理が同じメモリ箇所を読み書きするときの動作は未定義です。

LLL + RRR のような式に対して、LLL の評価は RRR の評価に対して unsequenced なので、上記のコード(x の定義部分も y の定義部分も)は未定義となります。 x = a[i] + b[i + 1]; y = a[i + 2] + b[i + 3] のようにはなってくれません*4

詳しくは下記を読んでください。

en.cppreference.com

-Wreturn-type

初心者がバグらせているとき、実はこれが出ていることがとても多い気がします。

int f(int x) {
    if (なんか条件) return 1;
    if (なんか条件) return 2;
    if (なんか条件) return 3;
    // 上記の条件でカバーしたつもりになっている

    // 実際には条件が漏れていて、int を返さないまま関数の終端に来てしまう
}

上記は一例ですが、返り値型が void 以外の関数で値を返さないまま終端に来てしまった場合の動作は未定義です。main は例外で、勝手に return 0; が挿入されますが...

「それを見抜けるならコンパイルエラーにしてくれ」という声はよく聞こえますが、以下のような不都合があります。

int infty_loop() {
    while (実際には常に真となる非自明な条件) {
        if (適当なタイミングで真になる条件) return 1;
        // なんか処理
    }
}

たとえばこのような状況では、コンパイラからは無事に while 文の中で値を返すことを看破できないため、勝手にコンパイルエラーにされると困るわけですね。

(以下は妄想)無限ループ用の特別な構文を別途用意して、

int infty_loop() {
    loop {
        if (実際には常に偽となる非自明な条件) break;
    }
    // 実際には到達しない
}

のようなコードも「loop を抜けうる break があるのでエラーとする」としていいなら、そうするのがよかったのでしょうか。

-Wuninitialized, -Wmaybe-uninitialized

初期化するコードがない場合や、初期化しないコードパスがある場合の警告です。

int main() {
    int x;
    printf("%d\n", x);  // is used uninitialized
}
int foo(int y) {
    int x;
    switch (y) {
    case 0:
        x = 100;
        break;
    case 1:
        x = 123;
    }
    return x;  // may be used uninitialized
}
int foo(int y) {
    int x;
    if (y == 0) {
        x = 100;
    } else if (y == 1) {
        x = 123;
    }
    return x;  // may be used uninitialized
}

初期化せずに使った場合の動作は未定義なので気にしてくれています。ただ、maybe の方は検出してくれないこともある(警告を出すフェイズが、コード解析によって不要と判断した箇所を削除した後という事情があるらしい?)ようで、あまり信用ならないかもしれません。 たとえば、上記コード例で case 1:else if (y == 1) の部分がなく初期化が一箇所だけの場合は(手元環境では)警告が出ませんでした。

余談として、以下も未定義です。

int main() {
    bool x;
    printf("%d\n", int(x & !x));  // 0 を期待しがち
    printf("%d\n", int(x | !x));  // 1 を期待しがち
}

-Wparentheses

括弧をつけた方がいいですよという警告です。

void foo(int x) {
    if (x = 0) {
        puts("hello");
    }
}
suggest parentheses around assignment used as truth value
      |   if (x = 0) {
      |       ~~^~~

のようなことを言ってくれるのですが、「if (x == 0) のつもりか?」などと言ってくれた方がわかりやすい気がします。

他にも、1 + 2 << 3 * 4 / 5 >> 6 | 7 ^ 8 のような式を書いたときも「<< の内側の + のまわりには括弧をつけよう」「|オペランドの算術演算には括弧をつけよう」とか言ってくれるようです。

-Wfloat-equal

rsk0315.hatenablog.com

こちらでも触れていますが、浮動小数点数の比較を ==!= で行おうとすると誤差などによって意図しない挙動になりがちです。

int main() {
    assert(1.0 / 49 * 49 == 1);
}

これはおそらく false になって abort します。

おまけ

きもち 1

C++ に詳しくない人ほど「警告が出ているから書き方を改めるか...」ではなく「よくわからないから無視するか...」となってしまいがち感があり、かなしいね。

でもいきなり -Wstrict-aliasing で “... will break strict-aliasing rules” とか言われてもよくわかんなくてこまっちゃうよね。

きもち 2

コンパイル時警告の出し方と、基本的な典型バグだけでも押さえておけば、「なんでバグってますかこれ、助けてください」「警告見ましたか? いろいろ出てますよ」のようなやり取りが減らせてうれしそうな気がします。

おわり

ねこねこ

*1:実際には型が統一されているわけですが、ここでは符号のことに言及すれば十分だったので。

*2:C や、古い C++ では sequence point と呼ばれる概念がありました。

*3:副作用という用語も、わりとふんわりお気持ち定義で使われている感があるかもしれません。?

*4:未定義動作の結果、たまたまそういう挙動になることを否定しているわけではありません。