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

[Sean Silva]

> 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).

-- Sean Silva

On Mon, Oct 15, 2012 at 1:02 PM, Stephen Canon <scanon at apple.com> wrote:
> On Oct 15, 2012, at 12:44 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
>> On Oct 14, 2012, at 8:19 , Stephen Canon <scanon at apple.com> wrote:
>>
>>> On Oct 12, 2012, at 12:38 PM, Richard Trieu <rtrieu at google.com> wrote:
>>>
>>>> This patch is for a new warning to Clang, called -Wlogical-not-compare.  It is designed to catch the case where the user is attempting to negate a comparison, but only manages to negate the LHS because of missing parens.  For instance, warn here:
>>>>
>>>>    if (!x < 5)
>>>>
>>>> The user probably meant:
>>>>
>>>>    if (!(x < 5))
>>>>
>>>> or
>>>>
>>>>    if (x >= 5)
>>>>
>>>> When emitted, the warning will have a note suggesting this as a fix-it (drop the not and inverting the comparison operator).
>>>
>>> If x has FP type, then !(x < 5) is not equivalent to (x >= 5); if x is NaN, the first is true, but the second is false.
>>
>> The fixit is on a note, so I personally think it's okay that it doesn't perfectly match the semantics of the existing expression. I think most people ignore NaNs when writing casual floating-point code.
>
> 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.
>
> - Steve
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits