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

[David Blaikie]

On Mon, Nov 12, 2012 at 1:17 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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?

Having just been playing with an LLVM bitfield enum it seems like a
not uncommon approach is to use hex literals for bitfield enums (0x1,
0x2, 0x4, ...). Perhaps that would be a useful heuristic for picking
out at least some enums that are intended to contain values that might
not be exact enumerants (such as bitwise ORs of multiple enumerants).

>
> -Eli
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits