This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] correct "comparisons do not have their mathematical meaning" warning


Hi, 

I'd like to separate the "comparisons like X<= Y <= Z do not have their 
mathematical meaning" warning a bit more. Right now, it warns about 
constructs like: 

  if (a < b < c)
   ..

  if (a >= b >= c)

as well as about constructs like

  if ( a > b == true)

Triggering the "do not have their mathematical meaning" warning for the 3rd is 
surprising and not very helpful (given that the evaluation order is 
well-defined and the programmer likely knew what he wanted to do, much unlike 
in the first two cases). 

The trouble is that the existing testsuite already tested for a warning for 
the 3rd case. In order to not "regress", I've added a separate warning, much 
in the spirit for the other -Wparentheses warnings ("suggest parentheses 
around comparison in operand of =="), and limited the "mathematical meaning" 
warning to sequences of <,<=,>=,> operators. 

Below is bootstrapped and regtested against trunk on i686-suse-linux. Ok?


2006-12-20  Dirk Mueller  <dmueller@suse.de>

        * c-common.c (warn_about_parentheses): Separate warning about
        un-parenthized sequence of comparison operators from the one
        which is supposed to warn about x <= y <= z.

        * testsuite/gcc.dg/Wparentheses-2.c: Add new tests.


--- c-common.c	(revision 119972)
+++ c-common.c	(working copy)
@@ -6680,12 +6680,24 @@ warn_about_parentheses (enum tree_code c
 		 "suggest parentheses around comparison in operand of &");
     }
 
-  /* Similarly, check for cases like 1<=i<=10 that are probably errors.  */
-  if (TREE_CODE_CLASS (code) == tcc_comparison
-      && (TREE_CODE_CLASS (code_left) == tcc_comparison
-	  || TREE_CODE_CLASS (code_right) == tcc_comparison))
-    warning (OPT_Wparentheses, "comparisons like X<=Y<=Z do not "
-	     "have their mathematical meaning");
+  if (code == EQ_EXPR || code == NE_EXPR)
+    {
+      if (TREE_CODE_CLASS (code_left) == tcc_comparison
+          || TREE_CODE_CLASS (code_right) == tcc_comparison)
+	warning (OPT_Wparentheses,
+		 "suggest parentheses around comparison in operand of %s",
+                 code == EQ_EXPR ? "==" : "!=");
+    }
+
+  if (TREE_CODE_CLASS (code) == tcc_comparison)
+  {
+    if (TREE_CODE_CLASS (code) == tcc_comparison
+	&& TREE_CODE_CLASS (code_left) == tcc_comparison
+	&& code != NE_EXPR && code != EQ_EXPR
+	&& code_left != NE_EXPR && code_left != EQ_EXPR)
+      warning (OPT_Wparentheses, "comparisons like X<=Y<=Z do not "
+	       "have their mathematical meaning");
+  }
 }
 
 
Index: testsuite/gcc.dg/Wparentheses-2.c
===================================================================
--- testsuite/gcc.dg/Wparentheses-2.c	(revision 119972)
+++ testsuite/gcc.dg/Wparentheses-2.c	(working copy)
@@ -64,4 +64,52 @@ bar (int a, int b, int c)
   foo (1 != 2 != 3); /* { dg-warning "comparison" "correct warning" } */
   foo ((1 != 2) != 3);
   foo (1 != (2 != 3));
+  foo (a < b == c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a < b) == c);
+  foo (a < (b == c));
+  foo (a > b == c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a > b) == c);
+  foo (a > (b == c));
+  foo (a == b < c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a == b) < c);
+  foo (a == (b < c));
+  foo (a == b > c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a == b) > c);
+  foo (a == (b > c));
+  foo (1 < 2 == 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 < 2) == 3);
+  foo (1 < (2 == 3));
+  foo (1 > 2 == 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 > 2) == 3);
+  foo (1 > (2 == 3));
+  foo (1 == 2 < 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 == 2) < 3);
+  foo (1 == (2 < 3));
+  foo (1 == 2 > 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 == 2) > 3);
+  foo (1 == (2 > 3));
+  foo (a < b != c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a < b) != c);
+  foo (a < (b != c));
+  foo (a > b != c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a > b) != c);
+  foo (a > (b != c));
+  foo (a != b < c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a != b) < c);
+  foo (a != (b < c));
+  foo (a != b > c);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((a != b) > c);
+  foo (a != (b > c));
+  foo (1 < 2 != 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 < 2) != 3);
+  foo (1 < (2 != 3));
+  foo (1 > 2 != 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 > 2) != 3);
+  foo (1 > (2 != 3));
+  foo (1 != 2 < 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 != 2) < 3);
+  foo (1 != (2 < 3));
+  foo (1 != 2 > 3);  /* { dg-warning "comparison" "correct warning" } */
+  foo ((1 != 2) > 3);
+  foo (1 != (2 > 3));
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]