[cfe-commits] [PATCH] Improve -Wtautological-constant-out-of-range-compare

[Eli Friedman]

On Mon, Nov 12, 2012 at 1:04 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Mon, Nov 12, 2012 at 11:22 AM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
>>
>> Comments below.
>>
>> On Mon, Nov 12, 2012 at 9:46 AM, Richard Trieu <rtrieu at google.com> wrote:
>> > Make -Wtautological-constant-out-of-range-compare checking take into
>> > account types and conversion between types.  The old version merely checked
>> > the bit widths, which allowed failed to catch a few cases, while warning on
>> > other safe comparisons.
>> >
>> > http://llvm-reviews.chandlerc.com/D113
>> >
>> > Files:
>> >   test/Analysis/additive-folding.cpp
>> >   test/SemaCXX/compare.cpp
>> >   test/SemaCXX/warn-enum-compare.cpp
>> >   lib/Sema/SemaChecking.cpp
>>
>> Index: test/SemaCXX/warn-enum-compare.cpp
>> ===================================================================
>> --- test/SemaCXX/warn-enum-compare.cpp
>> +++ test/SemaCXX/warn-enum-compare.cpp
>> @@ -39,8 +39,8 @@
>>    while (b == c);
>>    while (B1 == name1::B2);
>>    while (B2 == name2::B1);
>> -  while (x == AnonAA); // expected-warning {{comparison of constant
>> 42 with expression of type 'Foo' is always false}}
>> -  while (AnonBB == y); // expected-warning {{comparison of constant
>> 45 with expression of type 'Bar' is always false}}
>> +  while (x == AnonAA);
>> +  while (AnonBB == y);
>>    while (AnonAA == AnonAB);
>>    while (AnonAB == AnonBA);
>>    while (AnonBB == AnonAA);
>>
>> Why are you changing this warning?
>
>
> Because it is valid to do:
> x = (Foo)42;
> which makes the condition true.

That makes sense... but on the other hand, it reduces the usefulness
of this warning for the vast majority of enums which don't play tricks
like this.  In any case, please separate this into its own patch.

There's been some discussion at Apple (in the context of
-Wassign-enum) about using some heuristics based on the form of enum
initializers (e.g. whether the initializers are written in the form "a
<< b") to separate enums into two classes: enums which play tricks
with bitfields, and enums which enumerate simple values.  Do you have
an opinion here?

-Eli