re PR c++/62153 (warn for bool expression compared with integer different from 0/1)

	PR c++/62153
	* doc/invoke.texi: Document -Wbool-compare.
c-family/
	* c-common.c (maybe_warn_bool_compare): New function.
	* c-common.h (maybe_warn_bool_compare): Declare.
	* c.opt (Wbool-compare): New option.
c/
	* c-typeck.c (build_binary_op): If either operand of a comparison
	is a boolean expression, call maybe_warn_bool_compare.
cp/
	* call.c (build_new_op_1): Remember the type of arguments for
	a comparison.  If either operand of a comparison is a boolean
	expression, call maybe_warn_bool_compare.
testsuite/
	* c-c++-common/Wbool-compare-1.c: New test.

From-SVN: r214183

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f47a25fd7a415a010f6c397f5ca5fa46617bdb4a..900d616ac92d1d8712d6cbd7478864fec4a93fe6 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2014-08-19  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/62153
+       * doc/invoke.texi: Document -Wbool-compare.
+
 2014-08-19  David Malcolm  <dmalcolm@redhat.com>
 
        * rtl.h (entry_of_function): Strengthen return type from rtx to

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index e25f1d8516443996873ad1f563fdbbd4a5c42cba..e458f5e2250e8c7126e7bcda5d2660bc8051da9f 100644 (file)
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,10 @@
+2014-08-19  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/62153
+       * c-common.c (maybe_warn_bool_compare): New function.
+       * c-common.h (maybe_warn_bool_compare): Declare.
+       * c.opt (Wbool-compare): New option.
+
 2014-08-19  Marek Polacek  <polacek@redhat.com>
 
        * c.opt (Wc99-c11-compat): New option.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index acc9a203d557507bbf783b512fa562d0f4f7e844..901a5edd707939e80d60b51ac18d946372cb6c4e 100644 (file)
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -11612,6 +11612,39 @@ maybe_warn_unused_local_typedefs (void)
   vec_free (l->local_typedefs);
 }
 
+/* Warn about boolean expression compared with an integer value different
+   from true/false.  Warns also e.g. about "(i1 == i2) == 2".
+   LOC is the location of the comparison, CODE is its code, OP0 and OP1
+   are the operands of the comparison.  The caller must ensure that
+   either operand is a boolean expression.  */
+
+void
+maybe_warn_bool_compare (location_t loc, enum tree_code code, tree op0,
+                        tree op1)
+{
+  if (TREE_CODE_CLASS (code) != tcc_comparison)
+    return;
+
+  tree cst = (TREE_CODE (op0) == INTEGER_CST)
+            ? op0 : (TREE_CODE (op1) == INTEGER_CST) ? op1 : NULL_TREE;
+  if (!cst)
+    return;
+
+  if (!integer_zerop (cst) && !integer_onep (cst))
+    {
+      int sign = (TREE_CODE (op0) == INTEGER_CST)
+                ? tree_int_cst_sgn (cst) : -tree_int_cst_sgn (cst);
+      if (code == EQ_EXPR
+         || ((code == GT_EXPR || code == GE_EXPR) && sign < 0)
+         || ((code == LT_EXPR || code == LE_EXPR) && sign > 0))
+       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
+                   "with boolean expression is always false", cst);
+      else
+       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
+                   "with boolean expression is always true", cst);
+    }
+}
+
 /* The C and C++ parsers both use vectors to hold function arguments.
    For efficiency, we keep a cache of unused vectors.  This is the
    cache.  */

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 26aaee26cddda516df36e7d90ed4bf24a1cfd29d..995bc8ca50ed9f04a4d08859e842fbd1203b25b1 100644 (file)
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1015,6 +1015,7 @@ extern void record_types_used_by_current_var_decl (tree);
 extern void record_locally_defined_typedef (tree);
 extern void maybe_record_typedef_use (tree);
 extern void maybe_warn_unused_local_typedefs (void);
+extern void maybe_warn_bool_compare (location_t, enum tree_code, tree, tree);
 extern vec<tree, va_gc> *make_tree_vector (void);
 extern void release_tree_vector (vec<tree, va_gc> *);
 extern vec<tree, va_gc> *make_tree_vector_single (tree);

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 484839904ce80295a79d48b8f2ca48d5d53f0bcb..f97a11a19cc773fcb610cef1fc57624c5a0440e8 100644 (file)
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -287,6 +287,10 @@ Wbad-function-cast
 C ObjC Var(warn_bad_function_cast) Warning
 Warn about casting functions to incompatible types
 
+Wbool-compare
+C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about boolean expression compared with an integer value different from true/false
+
 Wbuiltin-macro-redefined
 C ObjC C++ ObjC++ Warning
 Warn when a built-in preprocessor macro is undefined or redefined

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 391cfeda95ea756139c05625d96b217ecb5c6235..e5429ac617cfeaa6bc3de8cfb777327cfd431a85 100644 (file)
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,9 @@
+2014-08-19  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/62153
+       * c-typeck.c (build_binary_op): If either operand of a comparison
+       is a boolean expression, call maybe_warn_bool_compare.
+
 2014-08-19  Patrick Palka  <ppalka@gcc.gnu.org>
 
        PR c/45584

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 3210f1ab4f7dacea7196a447a2eada490bfe6ba4..d6d96cf3550f1adc29f3fce917034a049e2195dc 100644 (file)
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -10679,6 +10679,11 @@ build_binary_op (location_t location, enum tree_code code,
          result_type = type1;
          pedwarn (location, 0, "comparison between pointer and integer");
        }
+      if ((TREE_CODE (TREE_TYPE (orig_op0)) == BOOLEAN_TYPE
+          || truth_value_p (TREE_CODE (orig_op0)))
+         ^ (TREE_CODE (TREE_TYPE (orig_op1)) == BOOLEAN_TYPE
+            || truth_value_p (TREE_CODE (orig_op1))))
+       maybe_warn_bool_compare (location, code, orig_op0, orig_op1);
       break;
 
     case LE_EXPR:
@@ -10783,6 +10788,11 @@ build_binary_op (location_t location, enum tree_code code,
          result_type = type1;
          pedwarn (location, 0, "comparison between pointer and integer");
        }
+      if ((TREE_CODE (TREE_TYPE (orig_op0)) == BOOLEAN_TYPE
+          || truth_value_p (TREE_CODE (orig_op0)))
+         ^ (TREE_CODE (TREE_TYPE (orig_op1)) == BOOLEAN_TYPE
+            || truth_value_p (TREE_CODE (orig_op1))))
+       maybe_warn_bool_compare (location, code, orig_op0, orig_op1);
       break;
 
     default:

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 26599c24c3f11ba39cf191a81fadcdd9e5aa8a18..6d61f75b804b9ef7d7bed07dee36dff68cfa639e 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@
+2014-08-19  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/62153
+       * call.c (build_new_op_1): Remember the type of arguments for
+       a comparison.  If either operand of a comparison is a boolean
+       expression, call maybe_warn_bool_compare.
+
 2014-08-19  Jason Merrill  <jason@redhat.com>
 
        PR tree-optimization/62091

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7044e1491156ab1f1e6e65648abf0ba89488ebba..161235b64cb05070007219f362ea2662249b09ac 100644 (file)
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5318,7 +5318,17 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
       /* These are saved for the sake of warn_logical_operator.  */
       code_orig_arg1 = TREE_CODE (arg1);
       code_orig_arg2 = TREE_CODE (arg2);
-
+      break;
+    case GT_EXPR:
+    case LT_EXPR:
+    case GE_EXPR:
+    case LE_EXPR:
+    case EQ_EXPR:
+    case NE_EXPR:
+      /* These are saved for the sake of maybe_warn_bool_compare.  */
+      code_orig_arg1 = TREE_CODE (TREE_TYPE (arg1));
+      code_orig_arg2 = TREE_CODE (TREE_TYPE (arg2));
+      break;
     default:
       break;
     }
@@ -5625,16 +5635,20 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
       warn_logical_operator (loc, code, boolean_type_node,
                             code_orig_arg1, arg1, code_orig_arg2, arg2);
       /* Fall through.  */
-    case PLUS_EXPR:
-    case MINUS_EXPR:
-    case MULT_EXPR:
-    case TRUNC_DIV_EXPR:
     case GT_EXPR:
     case LT_EXPR:
     case GE_EXPR:
     case LE_EXPR:
     case EQ_EXPR:
     case NE_EXPR:
+      if ((code_orig_arg1 == BOOLEAN_TYPE)
+         ^ (code_orig_arg2 == BOOLEAN_TYPE))
+       maybe_warn_bool_compare (loc, code, arg1, arg2);
+      /* Fall through.  */
+    case PLUS_EXPR:
+    case MINUS_EXPR:
+    case MULT_EXPR:
+    case TRUNC_DIV_EXPR:
     case MAX_EXPR:
     case MIN_EXPR:
     case LSHIFT_EXPR:

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8ea368984ef37744ef1d262aa7e3f26b3eb23045..b8e42949f21951df5dcec96adda4f5b001ccb484 100644 (file)
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -240,6 +240,7 @@ Objective-C and Objective-C++ Dialects}.
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
 -Waggressive-loop-optimizations -Warray-bounds @gol
+-Wbool-compare @gol
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
@@ -4221,6 +4222,19 @@ This option is only active when @option{-ftree-vrp} is active
 (default for @option{-O2} and above). It warns about subscripts to arrays
 that are always out of bounds. This warning is enabled by @option{-Wall}.
 
+@item -Wbool-compare
+@opindex Wno-bool-compare
+@opindex Wbool-compare
+Warn about boolean expression compared with an integer value different from
+@code{true}/@code{false}.  For instance, the following comparison is
+always false:
+@smallexample
+int n = 5;
+@dots{}
+if ((n > 1) == 2) @{ @dots{} @}
+@end smallexample
+This warning is enabled by @option{-Wall}.
+
 @item -Wno-discarded-qualifiers @r{(C and Objective-C only)}
 @opindex Wno-discarded-qualifiers
 @opindex Wdiscarded-qualifiers

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a783217fe7d28973870d5a9b746ceafc353a4e0f..b1703fa1e148317a074026756e70c5d66d94399e 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-08-19  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/62153
+       * c-c++-common/Wbool-compare-1.c: New test.
+
 2014-08-19  Patrick Palka  <ppalka@gcc.gnu.org>
 
        PR c/45584

diff --git a/gcc/testsuite/c-c++-common/Wbool-compare-1.c b/gcc/testsuite/c-c++-common/Wbool-compare-1.c
new file mode 100644 (file)
index 0000000..5b03e06
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wbool-compare-1.c
@@ -0,0 +1,128 @@
+/* PR c++/62153 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+# define true 1
+# define false 0
+#endif
+
+extern bool foo (void);
+bool r;
+
+enum { E = 4 };
+
+void
+fn1 (bool b)
+{
+  r = b == 2; /* { dg-warning "with boolean expression is always false" } */
+  r = b != 2; /* { dg-warning "with boolean expression is always true" } */
+  r = b < 2; /* { dg-warning "with boolean expression is always true" } */
+  r = b > 2; /* { dg-warning "with boolean expression is always false" } */
+  r = b <= 2; /* { dg-warning "with boolean expression is always true" } */
+  r = b >= 2; /* { dg-warning "with boolean expression is always false" } */
+
+  r = b == -1; /* { dg-warning "with boolean expression is always false" } */
+  r = b != -1; /* { dg-warning "with boolean expression is always true" } */
+  r = b < -1; /* { dg-warning "with boolean expression is always false" } */
+  r = b > -1; /* { dg-warning "with boolean expression is always true" } */
+  r = b <= -1; /* { dg-warning "with boolean expression is always false" } */
+  r = b >= -1; /* { dg-warning "with boolean expression is always true" } */
+
+  r = foo () == 2; /* { dg-warning "with boolean expression is always false" } */
+  r = foo () != 2; /* { dg-warning "with boolean expression is always true" } */
+  r = foo () < 2; /* { dg-warning "with boolean expression is always true" } */
+  r = foo () > 2; /* { dg-warning "with boolean expression is always false" } */
+  r = foo () <= 2; /* { dg-warning "with boolean expression is always true" } */
+  r = foo () >= 2; /* { dg-warning "with boolean expression is always false" } */
+
+  r = b == E; /* { dg-warning "with boolean expression is always false" } */
+  r = b != E; /* { dg-warning "with boolean expression is always true" } */
+  r = b < E; /* { dg-warning "with boolean expression is always true" } */
+  r = b > E; /* { dg-warning "with boolean expression is always false" } */
+  r = b <= E; /* { dg-warning "with boolean expression is always true" } */
+  r = b >= E; /* { dg-warning "with boolean expression is always false" } */
+
+  /* Swap LHS and RHS.  */
+  r = 2 == b; /* { dg-warning "with boolean expression is always false" } */
+  r = 2 != b; /* { dg-warning "with boolean expression is always true" } */
+  r = 2 < b; /* { dg-warning "with boolean expression is always false" } */
+  r = 2 > b; /* { dg-warning "with boolean expression is always true" } */
+  r = 2 <= b; /* { dg-warning "with boolean expression is always false" } */
+  r = 2 >= b; /* { dg-warning "with boolean expression is always true" } */
+
+  r = -1 == b; /* { dg-warning "with boolean expression is always false" } */
+  r = -1 != b; /* { dg-warning "with boolean expression is always true" } */
+  r = -1 < b; /* { dg-warning "with boolean expression is always true" } */
+  r = -1 > b; /* { dg-warning "with boolean expression is always false" } */
+  r = -1 <= b; /* { dg-warning "with boolean expression is always true" } */
+  r = -1 >= b; /* { dg-warning "with boolean expression is always false" } */
+
+  r = E == b; /* { dg-warning "with boolean expression is always false" } */
+  r = E != b; /* { dg-warning "with boolean expression is always true" } */
+  r = E < b; /* { dg-warning "with boolean expression is always false" } */
+  r = E > b; /* { dg-warning "with boolean expression is always true" } */
+  r = E <= b; /* { dg-warning "with boolean expression is always false" } */
+  r = E >= b; /* { dg-warning "with boolean expression is always true" } */
+
+  /* These are of course fine.  */
+  r = b == false;
+  r = b != false;
+  r = b == true;
+  r = b != true;
+
+  /* Some of these don't make much sense, but we don't warn.  */
+  r = b < false;
+  r = b >= false;
+  r = b <= false;
+  r = b > false;
+  r = b < true;
+  r = b >= true;
+  r = b <= true;
+  r = b > true;
+}
+
+void
+fn2 (int i1, int i2)
+{
+  r = (i1 == i2) == 2; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) != 2; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) < 2; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) > 2; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) <= 2; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) >= 2; /* { dg-warning "with boolean expression is always false" } */
+
+  r = (i1 == i2) == -1; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) != -1; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) < -1; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) > -1; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) <= -1; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) >= -1; /* { dg-warning "with boolean expression is always true" } */
+
+  r = (i1 == i2) == E; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) != E; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) < E; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) > E; /* { dg-warning "with boolean expression is always false" } */
+  r = (i1 == i2) <= E; /* { dg-warning "with boolean expression is always true" } */
+  r = (i1 == i2) >= E; /* { dg-warning "with boolean expression is always false" } */
+}
+
+void
+fn3 (int n, bool b)
+{
+  /* Don't warn here.  */
+  r = b == n;
+  r = b != n;
+  r = b < n;
+  r = b > n;
+  r = b <= n;
+  r = b >= n;
+
+  r = n == E;
+  r = n != E;
+  r = n < E;
+  r = n > E;
+  r = n <= E;
+  r = n >= E;
+}