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

[David Blaikie]

On Fri, Oct 12, 2012 at 9:38 AM, 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).
>
> Also, a second note will be offer parenthesis around the LHS to silence this
> warning.
>
>      if ((!x) < 5)

Should we suggest a suppression? (what're the false positive rates
like? have you had need to use this suppression syntax & if so, why?
(perhaps we can narrow the diagnostic down further if there are common
false positives))

This particular example looks completely tautological so if there are
false positives it'd be interesting to understand them.

Should this be a subgroup of -Wparentheses or -Wtautological-compare?
(I suppose not all of these are cases of the latter, but this does
have the same feel as many of the subgroups of the former (it
basically amounts to "precedence doesn't do what you expect here" -
but it's higher confidence than the usual -Wparentheses, it's not just
confusing precedence that we're detecting but a stronger signal that
this comparison is actually wrong/meaningless))

Perhaps it might even be worth splitting out the
trivial/obvious/tautological cases from the rest & grouping them in
different ways.

>
> This will not warn.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>