[cfe-commits] [Patch] Add new warning for logical not on LHS of comparison

[Stephen Canon]

On Oct 15, 2012, at 1:37 PM, Sean Silva <silvas at purdue.edu> wrote:

>> The compiler shouldn't be in the business of suggesting semantically incorrect transformations to the user, especially when there's a perfectly good alternative available.
>> 
>> We don't want to find bugs in users' code only to replace them with even more subtle bugs. For example, conver
> 
> While I agree with you, there is a certain balance to be had. Even
> converting it to !(x < 5) could cause issues if the conversion between
> integer and float causes undefined behavior by not being
> representable. I forget which direction the conversion for (f < 5)
> would go (int->float or float->int), but either operand could be
> outside the domain of the other (replacing 5 by INT_MAX, for example,
> or having f be 1e30).

int -> float, and conversions from int to float are always defined (assuming IEEE-754 floating point).  The conversion may be inexact, and may even overflow on platforms with 128-bit integers, but it is always defined.

More to the point, you're conflating two separate issues.

First, we have a fixit for (!expr1 < expr2).  I'm recommending suggesting that maybe the user intended to write !(expr1 < expr2) instead, which is always at least as correct as the other proposed fixit, (expr1 >= expr2), and sometimes significantly better (if either expr is floating-point).

Possibly warning the user of inexact conversions of integer constants to floating-point is an entirely separate issue (worth investigating; it would be neat if the compiler warned on (float)x < INT_MAX by pointing out that the comparison is actually being performed against 0x80000000.0 instead of 0x7ffffff.0, which isn't representable in single-precision, but that's an entirely separate can of worms).

- Steve