re PR c++/66555 (Fails to warn for if (j == 0 && i == i))
authorMarek Polacek <polacek@redhat.com>
Mon, 27 Jul 2015 12:40:45 +0000 (12:40 +0000)
committerMarek Polacek <mpolacek@gcc.gnu.org>
Mon, 27 Jul 2015 12:40:45 +0000 (12:40 +0000)
PR c++/66555
PR c/54979
* c-common.c (find_array_ref_with_const_idx_r): New function.
(warn_tautological_cmp): New function.
* c-common.h (warn_tautological_cmp): Declare.
* c.opt (Wtautological-compare): New option.

* c-typeck.c (parser_build_binary_op): Call warn_tautological_cmp.

* call.c (build_new_op_1): Call warn_tautological_cmp.
* pt.c (tsubst_copy_and_build): Use sentinel to suppress tautological
compare warnings.

* doc/invoke.texi: Document -Wtautological-compare.

* c-c++-common/Wtautological-compare-1.c: New test.

From-SVN: r226242

15 files changed:
gcc/ChangeLog
gcc/c-family/ChangeLog
gcc/c-family/c-common.c
gcc/c-family/c-common.h
gcc/c-family/c.opt
gcc/c/ChangeLog
gcc/c/c-typeck.c
gcc/cp/ChangeLog
gcc/cp/call.c
gcc/cp/pt.c
gcc/doc/invoke.texi
gcc/testsuite/ChangeLog
gcc/testsuite/c-c++-common/Wtautological-compare-1.c [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
gcc/testsuite/g++.dg/other/vector-compare.C

index e0eb3a2ee6a65e523db7ed64edffede4badd892e..177f71c87386fa9a4a53e94e0466737d3d7c4f46 100644 (file)
@@ -1,3 +1,9 @@
+2015-07-27  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/66555
+       PR c/54979
+       * doc/invoke.texi: Document -Wtautological-compare.
+
 2015-07-27  Richard Biener  <rguenther@suse.de>
 
        * genmatch.c (decision_tree::gen_gimple): Split out large
index caedf8e22672a8a18910141fc96b487d0049c36e..4d2ed59ea7a28fc627262aacb873f35ace5e9cbb 100644 (file)
@@ -1,3 +1,12 @@
+2015-07-27  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/66555
+       PR c/54979
+       * c-common.c (find_array_ref_with_const_idx_r): New function.
+       (warn_tautological_cmp): New function.
+       * c-common.h (warn_tautological_cmp): Declare.
+       * c.opt (Wtautological-compare): New option.
+
 2015-07-23  Marek Polacek  <polacek@redhat.com>
 
        * c-ubsan.c (ubsan_instrument_division): Use unshare_expr throughout.
index c94596f89bac57d216cf666c70e2c9db0463108c..6a79b95c27cecd662961e56f438da64716a8a8e2 100644 (file)
@@ -1861,6 +1861,70 @@ warn_logical_operator (location_t location, enum tree_code code, tree type,
     }
 }
 
+/* Helper function for warn_tautological_cmp.  Look for ARRAY_REFs
+   with constant indices.  */
+
+static tree
+find_array_ref_with_const_idx_r (tree *expr_p, int *walk_subtrees, void *data)
+{
+  tree expr = *expr_p;
+
+  if ((TREE_CODE (expr) == ARRAY_REF
+       || TREE_CODE (expr) == ARRAY_RANGE_REF)
+      && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST)
+    {
+      *(bool *) data = true;
+      *walk_subtrees = 0;
+    }
+
+  return NULL_TREE;
+}
+
+/* Warn if a self-comparison always evaluates to true or false.  LOC
+   is the location of the comparison with code CODE, LHS and RHS are
+   operands of the comparison.  */
+
+void
+warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs)
+{
+  if (TREE_CODE_CLASS (code) != tcc_comparison)
+    return;
+
+  /* We do not warn for constants because they are typical of macro
+     expansions that test for features, sizeof, and similar.  */
+  if (CONSTANT_CLASS_P (lhs) || CONSTANT_CLASS_P (rhs))
+    return;
+
+  /* Don't warn for e.g.
+     HOST_WIDE_INT n;
+     ...
+     if (n == (long) n) ...
+   */
+  if ((CONVERT_EXPR_P (lhs) || TREE_CODE (lhs) == NON_LVALUE_EXPR)
+      || (CONVERT_EXPR_P (rhs) || TREE_CODE (rhs) == NON_LVALUE_EXPR))
+    return;
+
+  if (operand_equal_p (lhs, rhs, 0))
+    {
+      /* Don't warn about array references with constant indices;
+        these are likely to come from a macro.  */
+      bool found = false;
+      walk_tree_without_duplicates (&lhs, find_array_ref_with_const_idx_r,
+                                   &found);
+      if (found)
+       return;
+      const bool always_true = (code == EQ_EXPR || code == LE_EXPR
+                               || code == GE_EXPR || code == UNLE_EXPR
+                               || code == UNGE_EXPR || code == UNEQ_EXPR);
+      if (always_true)
+       warning_at (loc, OPT_Wtautological_compare,
+                   "self-comparison always evaluates to true");
+      else
+       warning_at (loc, OPT_Wtautological_compare,
+                   "self-comparison always evaluates to false");
+    }
+}
+
 /* 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.  */
index a198e794f5baa30305ff10e63dc1683cfdd4349f..f0640c7f86cfb690bacb05304986bf5be0e474a0 100644 (file)
@@ -812,6 +812,7 @@ extern bool warn_if_unused_value (const_tree, location_t);
 extern void warn_logical_operator (location_t, enum tree_code, tree,
                                   enum tree_code, tree, enum tree_code, tree);
 extern void warn_logical_not_parentheses (location_t, enum tree_code, tree);
+extern void warn_tautological_cmp (location_t, enum tree_code, tree, tree);
 extern void check_main_parameter_types (tree decl);
 extern bool c_determine_visibility (tree);
 extern bool vector_types_compatible_elements_p (tree, tree);
index dc760d7179997cf72a79ace816865e3ef2505c6c..cb3af48dc4766b00a0c5d9ed012aaed917e82109 100644 (file)
@@ -848,6 +848,10 @@ Wsystem-headers
 C ObjC C++ ObjC++ Warning
 ; Documented in common.opt
 
+Wtautological-compare
+C ObjC C++ ObjC++ Var(warn_tautological_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn if a comparison always evaluates to true or false
+
 Wterminate
 C++ ObjC++ Warning Var(warn_terminate) Init(1)
 Warn if a throw expression will always result in a call to terminate()
index 18443aea3dba8f38686d88166850e28a32d1fe10..08d62460768770fe1eac64e18a65085dca48d9b9 100644 (file)
@@ -1,3 +1,9 @@
+2015-07-27  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/66555
+       PR c/54979
+       * c-typeck.c (parser_build_binary_op): Call warn_tautological_cmp.
+
 2015-07-20  Marek Polacek  <polacek@redhat.com>
 
        PR c++/55095
index d3d0abdb2cb137efa6cc3dd2bb83fa1282711f11..e8c818989ee9b7ca62f9eed3b1ba4ac2744c8d9c 100644 (file)
@@ -3430,6 +3430,9 @@ parser_build_binary_op (location_t location, enum tree_code code,
     warn_logical_operator (location, code, TREE_TYPE (result.value),
                           code1, arg1.value, code2, arg2.value);
 
+  if (warn_tautological_compare)
+    warn_tautological_cmp (location, code, arg1.value, arg2.value);
+
   if (warn_logical_not_paren
       && TREE_CODE_CLASS (code) == tcc_comparison
       && code1 == TRUTH_NOT_EXPR
index a9fa4457c6823050b03dafe020c08bbfc73eceb9..2c5fae1274d0c2106180e740cef6f1920d46bd91 100644 (file)
@@ -1,3 +1,11 @@
+2015-07-27  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/66555
+       PR c/54979
+       * call.c (build_new_op_1): Call warn_tautological_cmp.
+       * pt.c (tsubst_copy_and_build): Use sentinel to suppress tautological
+       compare warnings.
+
 2015-07-26  Patrick Palka  <ppalka@gcc.gnu.org>
 
        PR c++/18969
index 8dda1deeb8cc24baad1d8c1b7ea9f319f48c98b5..1be2527358a9339a6ac27c0319b0e59f07608b1e 100644 (file)
@@ -5651,6 +5651,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
          && ((code_orig_arg1 == BOOLEAN_TYPE)
              ^ (code_orig_arg2 == BOOLEAN_TYPE)))
        maybe_warn_bool_compare (loc, code, arg1, arg2);
+      if (complain & tf_warning && warn_tautological_compare)
+       warn_tautological_cmp (loc, code, arg1, arg2);
       /* Fall through.  */
     case PLUS_EXPR:
     case MINUS_EXPR:
index 971c98ee0d9639d30da89a738ac8f2bf230fb712..f15d16fe269606941f98aa2062e874a623100788 100644 (file)
@@ -14918,6 +14918,7 @@ tsubst_copy_and_build (tree t,
        warning_sentinel s1(warn_type_limits);
        warning_sentinel s2(warn_div_by_zero);
        warning_sentinel s3(warn_logical_op);
+       warning_sentinel s4(warn_tautological_compare);
        tree op0 = RECUR (TREE_OPERAND (t, 0));
        tree op1 = RECUR (TREE_OPERAND (t, 1));
        tree r = build_x_binary_op
index 54e7a679c2f71408cb867f233bf5ddd1abd3f6f8..e988444bdb71b6fcbd8332b8294af3a5dc4cb0d5 100644 (file)
@@ -283,7 +283,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol
 -Wmissing-format-attribute @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
--Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
+-Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
+-Wtype-limits  -Wundef @gol
 -Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
@@ -3452,6 +3453,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
 -Wimplicit-int @r{(C and Objective-C only)} @gol
 -Wimplicit-function-declaration @r{(C and Objective-C only)} @gol
+-Wbool-compare  @gol
 -Wcomment  @gol
 -Wformat   @gol
 -Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)}  @gol
@@ -3468,6 +3470,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wstrict-aliasing  @gol
 -Wstrict-overflow=1  @gol
 -Wswitch  @gol
+-Wtautological-compare  @gol
 -Wtrigraphs  @gol
 -Wuninitialized  @gol
 -Wunknown-pragmas  @gol
@@ -4513,6 +4516,18 @@ code.  However, note that using @option{-Wall} in conjunction with this
 option does @emph{not} warn about unknown pragmas in system
 headers---for that, @option{-Wunknown-pragmas} must also be used.
 
+@item -Wtautological-compare
+@opindex Wtautological-compare
+@opindex Wno-tautological-compare
+Warn if a self-comparison always evaluates to true or false.  This
+warning detects various mistakes such as:
+@smallexample
+int i = 1;
+@dots{}
+if (i > i) @{ @dots{} @}
+@end smallexample
+This warning is enabled by @option{-Wall}.
+
 @item -Wtrampolines
 @opindex Wtrampolines
 @opindex Wno-trampolines
index c711f8362d5c9d4c7f2b7d1a11a4c82a17d49ace..8b572c28fee43cc83177fd97e7018778fcc8f7d9 100644 (file)
@@ -1,3 +1,9 @@
+2015-07-27  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/66555
+       PR c/54979
+       * c-c++-common/Wtautological-compare-1.c: New test.
+
 2015-07-26  Patrick Palka  <ppalka@gcc.gnu.org>
 
        PR c++/18969
diff --git a/gcc/testsuite/c-c++-common/Wtautological-compare-1.c b/gcc/testsuite/c-c++-common/Wtautological-compare-1.c
new file mode 100644 (file)
index 0000000..71ba4f8
--- /dev/null
@@ -0,0 +1,70 @@
+/* PR c++/66555 */
+/* { dg-do compile } */
+/* { dg-options "-Wtautological-compare" } */
+
+#define X 5
+#define Y 5
+#define A a
+enum { U };
+
+void
+fn1 (int a, int *p)
+{
+  if (a > a); /* { dg-warning "self-comparison always evaluates to false" } */
+  if (a < a); /* { dg-warning "self-comparison always evaluates to false" } */
+  if (a >= a); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (a <= a); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (a == a); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (a != a); /* { dg-warning "self-comparison always evaluates to false" } */
+  if (A == A); /* { dg-warning "self-comparison always evaluates to true" } */
+  if ((unsigned) a != (unsigned) a);
+  if ((a + 1) <= (a + 1)); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (1 ? a == a : 0); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (fn1 == fn1); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (*p == *p); /* { dg-warning "self-comparison always evaluates to true" } */
+
+  volatile int v = 5;
+  if (v == v);
+  if (v != v);
+}
+
+void
+fn2 (int a)
+{
+  if (sizeof (int) >= 4);
+  if (sizeof (char) != 1);
+  if (sizeof (long) != sizeof (long long));
+  if (0 < sizeof (short));
+  if (5 != 5);
+  if (X > 5);
+  if (X == X);
+  if (3 + 4 == 6 + 1);
+  if ((unsigned) a != (unsigned long) a);
+  if (U == U);
+  if (U > 0);
+}
+
+void
+fn3 (int i, int j)
+{
+  static int a[16];
+  static int b[8][8];
+
+  if (a[5] == a[5]);
+  if (a[X] != a[Y]);
+  if (a[X] != a[X]);
+  if (a[i] == a[i]); /* { dg-warning "self-comparison always evaluates to true" } */
+  if (b[5][5] == b[5][5]);
+  if (b[X][Y] >= b[Y][X]);
+  if (b[X][X] == b[Y][Y]);
+  if (b[i][j] != b[i][j]); /* { dg-warning "self-comparison always evaluates to false" } */
+  if (b[i][Y] < b[i][X]);
+  if (b[X][j] < b[X][j]);
+  if ((a[i] + 4) == (4 + a[i])); /* { dg-warning "self-comparison always evaluates to true" } */
+}
+
+int
+fn4 (int x, int y)
+{
+  return x > x ? 1 : 0; /* { dg-warning "self-comparison always evaluates to false" } */
+}
index 4b81b5ab888b4220965ca57eea50a8bb86cf2118..63227303225b8189b5c77dabefb4372e2f1d85a1 100644 (file)
@@ -1,5 +1,5 @@
 /* { dg-do compile { target c++11 } } */
-/* { dg-options "-Wall" } */
+/* { dg-options "-Wall -Wno-tautological-compare" } */
 
 typedef float v4f __attribute__((vector_size(4*sizeof(float))));
 
index 03ff5fd7255a0c4d206192f9789b528fec9663aa..77b0f5153ca1231b506629ecec8b7be199749c85 100644 (file)
@@ -1,5 +1,5 @@
 /* { dg-do compile { target c++11 } } */
-/* { dg-options "-Wall" } */
+/* { dg-options "-Wall -Wno-tautological-compare" } */
 
 // Check that we can compare vector types that really are the same through
 // typedefs.