<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">On Mon, Nov 12, 2012 at 1:17 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Nov 12, 2012 at 1:04 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>

> On Mon, Nov 12, 2012 at 11:22 AM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Comments below.<br>
>><br>
>> On Mon, Nov 12, 2012 at 9:46 AM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
>> > Make -Wtautological-constant-out-of-range-compare checking take into<br>
>> > account types and conversion between types.  The old version merely checked<br>
>> > the bit widths, which allowed failed to catch a few cases, while warning on<br>
>> > other safe comparisons.<br>
>> ><br>
>> > <a href="http://llvm-reviews.chandlerc.com/D113" target="_blank">http://llvm-reviews.chandlerc.com/D113</a><br>
>> ><br>
>> > Files:<br>
>> >   test/Analysis/additive-folding.cpp<br>
>> >   test/SemaCXX/compare.cpp<br>
>> >   test/SemaCXX/warn-enum-compare.cpp<br>
>> >   lib/Sema/SemaChecking.cpp<br>
>><br>
>> Index: test/SemaCXX/warn-enum-compare.cpp<br>
>> ===================================================================<br>
>> --- test/SemaCXX/warn-enum-compare.cpp<br>
>> +++ test/SemaCXX/warn-enum-compare.cpp<br>
>> @@ -39,8 +39,8 @@<br>
>>    while (b == c);<br>
>>    while (B1 == name1::B2);<br>
>>    while (B2 == name2::B1);<br>
>> -  while (x == AnonAA); // expected-warning {{comparison of constant<br>
>> 42 with expression of type 'Foo' is always false}}<br>
>> -  while (AnonBB == y); // expected-warning {{comparison of constant<br>
>> 45 with expression of type 'Bar' is always false}}<br>
>> +  while (x == AnonAA);<br>
>> +  while (AnonBB == y);<br>
>>    while (AnonAA == AnonAB);<br>
>>    while (AnonAB == AnonBA);<br>
>>    while (AnonBB == AnonAA);<br>
>><br>
>> Why are you changing this warning?<br>
><br>
><br>
> Because it is valid to do:<br>
> x = (Foo)42;<br>
> which makes the condition true.<br>
<br>
</div></div>That makes sense... but on the other hand, it reduces the usefulness<br>
of this warning for the vast majority of enums which don't play tricks<br>
like this.  In any case, please separate this into its own patch.<br>
<br>
There's been some discussion at Apple (in the context of<br>
-Wassign-enum) about using some heuristics based on the form of enum<br>
initializers (e.g. whether the initializers are written in the form "a<br>
<< b") to separate enums into two classes: enums which play tricks<br>
with bitfields, and enums which enumerate simple values.  Do you have<br>
an opinion here?<br>
<span class="HOEnZb"><font color="#888888"><br>
-Eli<br>
</font></span></blockquote></div><br><div>I think that enum checking should be split off into its own warning.  With "tautological" in its name, this warning seems to imply a certain absoluteness to it.  The existence of counter-examples would undermine that.</div>
<div><br></div><div>Splitting enums into two types for better checking sounds like a good idea, and is probably better than the checks we do now.</div></div>