Re: Fix bogus warning with -Wlogical-not-parentheses (PR c/77292)

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Aug 25, 2016 at 3:39 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Thu, Aug 25, 2016 at 11:17:37AM +0200, Richard Biener wrote:
>> On Wed, Aug 24, 2016 at 7:43 PM, Marek Polacek <polacek@redhat.com> wrote:
>> > The -Wlogical-not-parentheses deliberately doesn't warn when the RHS has
>> > boolean type.  But since in C the type of a comparison is int, we need
>> > to check for tcc_comparison, too.
>> >
>> > Bootstrapped/regtested on x86_64-linux and ppc64le-redhat-linux, ok for trunk?
>>
>> What about using truth_value_p ()?  That also includes && and || (just
>> in case those
>> do not have boolean type).
>
> You're right that I should also handle || and && (done in this patch).  But I
> can't use truth_value_p because we want to warn for e.g. "!a == (a & b)".

But a & b isn't truth_value_p.  a && b is.  Do we want to warn about
!a == (a && b)?

Richard.

> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-08-25  Marek Polacek  <polacek@redhat.com>
>
>         PR c/77292
>         * c-common.c (warn_logical_not_parentheses): Don't warn for
>         a comparison or a logical operator.
>
>         * c-c++-common/Wlogical-not-parentheses-1.c: New test.
>
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 3feb910..3656c57 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -1481,7 +1481,8 @@ warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs)
>
>  /* Warn about logical not used on the left hand side operand of a comparison.
>     This function assumes that the LHS is inside of TRUTH_NOT_EXPR.
> -   Do not warn if RHS is of a boolean type.  */
> +   Do not warn if RHS is of a boolean type, a logical operator, or
> +   a comparison.  */
>
>  void
>  warn_logical_not_parentheses (location_t location, enum tree_code code,
> @@ -1489,7 +1490,10 @@ warn_logical_not_parentheses (location_t location, enum tree_code code,
>  {
>    if (TREE_CODE_CLASS (code) != tcc_comparison
>        || TREE_TYPE (rhs) == NULL_TREE
> -      || TREE_CODE (TREE_TYPE (rhs)) == BOOLEAN_TYPE)
> +      || TREE_CODE (TREE_TYPE (rhs)) == BOOLEAN_TYPE
> +      || COMPARISON_CLASS_P (rhs)
> +      || TREE_CODE (rhs) == TRUTH_ANDIF_EXPR
> +      || TREE_CODE (rhs) == TRUTH_ORIF_EXPR)
>      return;
>
>    /* Don't warn for !x == 0 or !y != 0, those are equivalent to
> diff --git gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c
> index e69de29..ba57437 100644
> --- gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c
> +++ gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c
> @@ -0,0 +1,25 @@
> +/* PR c/77292 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wlogical-not-parentheses" } */
> +
> + /* Test that we don't warn if rhs is a comparison or a logical op.  */
> +
> +int
> +foo (int a, int b)
> +{
> +  int r = 0;
> +  r += !a == (a < b);
> +  r += !a == (a > b);
> +  r += !a == (a >= b);
> +  r += !a == (a <= b);
> +  r += !a == (a != b);
> +  r += !a == (a == b);
> +  r += !a == (a || b);
> +  r += !a == (a && b);
> +  r += !a == (!b);
> +
> +  r += !a == (a ^ b); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */
> +  r += !a == (a | b); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */
> +  r += !a == (a & b); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */
> +  return r;
> +}
>
>         Marek