diff mbox

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

Message ID 20160826134211.GU11131@redhat.com
State New
Headers show

Commit Message

Marek Polacek Aug. 26, 2016, 1:42 p.m. UTC 
On Fri, Aug 26, 2016 at 11:18:46AM +0200, Richard Biener wrote:
> On Fri, Aug 26, 2016 at 10:57 AM, Marek Polacek <polacek@redhat.com> wrote:
> > On Fri, Aug 26, 2016 at 10:53:01AM +0200, Richard Biener wrote:
> >> 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)?
> >
> > We don't want to warn about !a == (a && b), I think.  But truth_value_p returns
> > true for both TRUTH_AND_EXPR and TRUTH_ANDIF_EXPR.  I find that confusing, and
> > I can't use it in the current form because of that.
> 
> Note that a & b would be BIT_AND_EXPR, TRUTH_AND_EXPR is just
> non-short-circuiting
> &&

Ugh, not sure what went wrong.  Let's pretend this never happened.

And now the patch for real:

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-08-26  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.


	Marek

Comments

Joseph Myers Aug. 29, 2016, 3:43 p.m. UTC | #1 
On Fri, 26 Aug 2016, Marek Polacek wrote:

> 2016-08-26  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.

OK.
diff mbox

Patch

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 001070d..f9b7c3c 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,8 @@  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
+      || truth_value_p (TREE_CODE (rhs)))
     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..b6b4e9d 100644
--- gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c
+++ gcc/testsuite/c-c++-common/Wlogical-not-parentheses-1.c
@@ -0,0 +1,26 @@ 
+/* 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;
+}