r210372 - Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings

[Reid Kleckner]

On Tue, Aug 5, 2014 at 3:43 PM, Chandler Carruth <chandlerc at google.com>
wrote:

>
> On Tue, Aug 5, 2014 at 3:36 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> This is a very useful warning, thanks.
>>
>> One issue I have with it is that it's disabled
>> by Wno-tautological-compare. From the warning name that makes sense –
>> but Wtautological-compare used to be a fairly harmless warning, so we
>> (chromium) disabled it for a bunch of third-party libraries. When
>> Wtautological-undefined-compare arrived, we fixed all instances of this in
>> our code and updated our compiler to a clang that optimizes away
>> comparisons of references with NULL (since we had fixed all of these
>> comparisons, we thought!)
>>
>> I recently realized that we didn't see Wtautological-undefined-compare
>> warnings for all the third-party libraries that we're building with
>> Wno-tautological-compare.
>>
>> Do you think Wtautological-undefined-compare should be in its own warning
>> group (and maybe have a different name to make that clear), so that folks
>> who disabled Wtautological-compare still get this warning? The reasoning is
>> that this warning is much more serious that what Wtautological-compare used
>> to warn about.
>>
>
> I don't really understand why. There is no actual undefined behavior here,
> just a comparison that can never, ever be go the other way. It seems almost
> exactly as severe as comparing an unsigned number for >= 0?
>

IMO there is a big difference.  All compilers have always returned true for
unsigned >= 0, but other compilers may return true for &reference ==
nullptr.

So, we can ignore the unsigned >= 0 warning so long as we are reasonably
confident in past test coverage.

However, with the &reference == nullptr behavior change, we don't have that
confidence.  Therefore it is more severe.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140805/2ab4f91d/attachment.html>