-Wsign-compare fails to warn about mismatches involving const types

[llvmbot]

Bugzilla Link	12646
Version	trunk
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor,@zygoloid

Extended Description

Compiling the following with [clang++ -Wsign-compare -c] (trunk 155352) results in warnings for the first i==u comparison and then for all the comparisons in g(). I believe the other comparisons in f() should also trigger the warning, for consistency (but they appear not too, presumably because of the "const"s).

int f() {
int i = 1;
const int ci = 2;
unsigned u = 3;
const unsigned cu = 4;
if (i == u) return 1; // Only this triggers a warning!
if (ci == cu) return 2;
if (i == cu) return 3;
if (ci == u) return 4;
return 0;
}

int bar();
int g() {
int i = bar();
const int ci = bar();
unsigned u = bar();
const unsigned cu = bar();
if (i == u) return 1;
if (ci == cu) return 2;
if (i == cu) return 3;
if (ci == u) return 4;
return 0;
}

[zygoloid]

This is intentional. We do not warn when we can prove that the comparison operation will have its mathematical meaning. When checking that, we evaluate the operands as constant expressions (which ci and cu are). Hence we don't warn on three of your examples, but do warn on these cases:

// Warning: will compare equal if u == UINT_MAX - 1
unsigned u = 1; const int ci = -2; if (u == ci) { ... }

// Warning: will compare equal if i == -2147483648
const unsigned cu = 2147483648; int i = 4; if (cu == i) { ... }

// Warning: will compare less if i is negative
int i = 1; const unsigned cu = 4; if (cu < i) { ... }

Is there a solid reason for wanting warnings in the cases where we suppress them?

[llvmbot]

This is intentional. We do not warn when we can prove that the comparison
operation will have its mathematical meaning. When checking that, we evaluate
the operands as constant expressions (which ci and cu are). Hence we don't warn
on three of your examples, but do warn on these cases:

// Warning: will compare equal if u == UINT_MAX - 1
unsigned u = 1; const int ci = -2; if (u == ci) { ... }

// Warning: will compare equal if i == -2147483648
const unsigned cu = 2147483648; int i = 4; if (cu == i) { ... }

// Warning: will compare less if i is negative
int i = 1; const unsigned cu = 4; if (cu < i) { ... }

Is there a solid reason for wanting warnings in the cases where we suppress
them?

My reason is that I want clang++ & g++ to warn about the same set of things when using the same flags (-Wsign-compare), because the same project is worked on by developers (and bots) running both clang++ & g++ (so when a clang++-using developer checks in warning-free code IWBN to not trigger warnings on g++-using developers' builds, and vice versa).

I see the argument for only warning when the possibility of a problem exists, but I don't believe that goal is reached here. For example, the warning in f() in my original report can be proven to have be spurious, because of the specific values involved (in fact the entire method could be compiled to a single "return 0", with no comparisons much less mismatched comparisons). Given the impossibility (I'm assuming) of reaching the stated goal, predictability of warnings (and thus "compatibility" w/ g++) seems like a bigger win to me.

[YutongZhuu]

I think g++ warn about the same thing: https://godbolt.org/z/M69GKo57b, so we are good. I think this issue can be closed. @Quuxplusone