* [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
@ 2015-07-14 15:19 Marek Polacek
2015-07-14 22:04 ` Richard Sandiford
2015-07-22 19:48 ` Martin Sebor
0 siblings, 2 replies; 17+ messages in thread
From: Marek Polacek @ 2015-07-14 15:19 UTC (permalink / raw)
To: GCC Patches, Joseph Myers, Jason Merrill
Code such as "if (i == i)" is hardly ever desirable, so we should be able
to warn about this to prevent dumb mistakes.
This ought to be easy in principle: when we see a comparison, just check
for operand_equal_p of operands, and if they're the same, warn.
In reality, it of course ain't that simple.
Doing just this resulted in a spate of warnings when crunching the sse.md
file, where we have a code like mode_nunits[V16SImode] == mode_nunits[V32HImode].
Both V16SImode and V32HImode happen to be defined to 76 so we issued a bogus
warning. To mollify these bogus warnings I had to use walk_tree and bail out if
we find an ARRAY_REF with a constant index in the comparison.
(We're still able to warn for a[i] == a[i].)
I also had to jump through the hoops to not warn on several other cases.
This warning is enabled by -Wall.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-07-14 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.
* 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.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 84e7242..6ceed36 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1849,6 +1849,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. */
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index a2a4621..b891bbd 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -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);
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 285952e..2f6369b 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -840,6 +840,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()
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 247a7bf..6bbc1ba 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -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
diff --git gcc/cp/call.c gcc/cp/call.c
index fce42da..5d3524e 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -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:
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 2097963..a21ee8f 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -14893,6 +14893,7 @@ tsubst_copy_and_build (tree t,
{
warning_sentinel s1(warn_type_limits);
warning_sentinel s2(warn_div_by_zero);
+ warning_sentinel s3(warn_tautological_compare);
tree op0 = RECUR (TREE_OPERAND (t, 0));
tree op1 = RECUR (TREE_OPERAND (t, 1));
tree r = build_x_binary_op
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 22ab269..9771aca 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -282,7 +282,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
@@ -3450,6 +3451,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
@@ -3466,6 +3468,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
@@ -4491,6 +4494,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
diff --git gcc/testsuite/c-c++-common/Wtautological-compare-1.c gcc/testsuite/c-c++-common/Wtautological-compare-1.c
index e69de29..576877b 100644
--- gcc/testsuite/c-c++-common/Wtautological-compare-1.c
+++ gcc/testsuite/c-c++-common/Wtautological-compare-1.c
@@ -0,0 +1,66 @@
+/* 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); /* { dg-warning "self-comparison always evaluates to false" } */
+ 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" } */
+}
+
+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" } */
+}
diff --git gcc/testsuite/g++.dg/cpp0x/decltype-54581.C gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
index 4b81b5a..6322730 100644
--- gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
+++ gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
@@ -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))));
diff --git gcc/testsuite/g++.dg/other/vector-compare.C gcc/testsuite/g++.dg/other/vector-compare.C
index 03ff5fd..77b0f51 100644
--- gcc/testsuite/g++.dg/other/vector-compare.C
+++ gcc/testsuite/g++.dg/other/vector-compare.C
@@ -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.
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-14 15:19 [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979) Marek Polacek
@ 2015-07-14 22:04 ` Richard Sandiford
2015-07-14 22:14 ` Marek Polacek
2015-07-22 19:48 ` Martin Sebor
1 sibling, 1 reply; 17+ messages in thread
From: Richard Sandiford @ 2015-07-14 22:04 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, Jason Merrill
Marek Polacek <polacek@redhat.com> writes:
> + /* 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;
I might be misreading it, sorry, but it looks like the XOR means that
we'd still warn for:
if ((HOST_WIDE_INT) n == (long) n) ...
in cases where HOST_WIDE_INT and long have the same precision.
Thanks,
Richard
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-14 22:04 ` Richard Sandiford
@ 2015-07-14 22:14 ` Marek Polacek
2015-07-22 18:03 ` Marek Polacek
0 siblings, 1 reply; 17+ messages in thread
From: Marek Polacek @ 2015-07-14 22:14 UTC (permalink / raw)
To: GCC Patches, Joseph Myers, Jason Merrill, rdsandiford
On Tue, Jul 14, 2015 at 10:30:06PM +0100, Richard Sandiford wrote:
> Marek Polacek <polacek@redhat.com> writes:
> > + /* 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;
>
> I might be misreading it, sorry, but it looks like the XOR means that
> we'd still warn for:
>
> if ((HOST_WIDE_INT) n == (long) n) ...
>
> in cases where HOST_WIDE_INT and long have the same precision.
Yes, that's true. Maybe we want to warn in that case as well,
I didn't know. If we do, just changing ^ into || would probably
help. It's somewhat hazy to me what to do in this case.
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-14 22:14 ` Marek Polacek
@ 2015-07-22 18:03 ` Marek Polacek
0 siblings, 0 replies; 17+ messages in thread
From: Marek Polacek @ 2015-07-22 18:03 UTC (permalink / raw)
To: GCC Patches, Joseph Myers, Jason Merrill, rdsandiford
On Tue, Jul 14, 2015 at 11:42:21PM +0200, Marek Polacek wrote:
> On Tue, Jul 14, 2015 at 10:30:06PM +0100, Richard Sandiford wrote:
> > Marek Polacek <polacek@redhat.com> writes:
> > > + /* 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;
> >
> > I might be misreading it, sorry, but it looks like the XOR means that
> > we'd still warn for:
> >
> > if ((HOST_WIDE_INT) n == (long) n) ...
> >
> > in cases where HOST_WIDE_INT and long have the same precision.
>
> Yes, that's true. Maybe we want to warn in that case as well,
> I didn't know. If we do, just changing ^ into || would probably
> help. It's somewhat hazy to me what to do in this case.
This is version with || rather than ^. Pick whichever you prefer.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-07-22 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.
* 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.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 84e7242..6ceed36 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1849,6 +1849,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. */
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index a2a4621..b891bbd 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -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);
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 285952e..2f6369b 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -840,6 +840,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()
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 247a7bf..6bbc1ba 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -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
diff --git gcc/cp/call.c gcc/cp/call.c
index fce42da..5d3524e 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -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:
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 2097963..a21ee8f 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -14893,6 +14893,7 @@ tsubst_copy_and_build (tree t,
{
warning_sentinel s1(warn_type_limits);
warning_sentinel s2(warn_div_by_zero);
+ warning_sentinel s3(warn_tautological_compare);
tree op0 = RECUR (TREE_OPERAND (t, 0));
tree op1 = RECUR (TREE_OPERAND (t, 1));
tree r = build_x_binary_op
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 22ab269..9771aca 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -282,7 +282,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
@@ -3450,6 +3451,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
@@ -3466,6 +3468,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
@@ -4491,6 +4494,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
diff --git gcc/testsuite/c-c++-common/Wtautological-compare-1.c gcc/testsuite/c-c++-common/Wtautological-compare-1.c
index e69de29..576877b 100644
--- gcc/testsuite/c-c++-common/Wtautological-compare-1.c
+++ gcc/testsuite/c-c++-common/Wtautological-compare-1.c
@@ -0,0 +1,66 @@
+/* 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" } */
+}
+
+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" } */
+}
diff --git gcc/testsuite/g++.dg/cpp0x/decltype-54581.C gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
index 4b81b5a..6322730 100644
--- gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
+++ gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
@@ -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))));
diff --git gcc/testsuite/g++.dg/other/vector-compare.C gcc/testsuite/g++.dg/other/vector-compare.C
index 03ff5fd..77b0f51 100644
--- gcc/testsuite/g++.dg/other/vector-compare.C
+++ gcc/testsuite/g++.dg/other/vector-compare.C
@@ -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.
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-14 15:19 [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979) Marek Polacek
2015-07-14 22:04 ` Richard Sandiford
@ 2015-07-22 19:48 ` Martin Sebor
2015-07-22 20:02 ` Marek Polacek
2015-07-29 10:24 ` Richard Earnshaw
1 sibling, 2 replies; 17+ messages in thread
From: Martin Sebor @ 2015-07-22 19:48 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers, Jason Merrill
On 07/14/2015 09:18 AM, Marek Polacek wrote:
> Code such as "if (i == i)" is hardly ever desirable, so we should be able
> to warn about this to prevent dumb mistakes.
I haven't tried the patch or even studied it very carefully but
I wonder if this is also the case when i is declared volatile.
I.e., do we want to issue a warning there? (If we do, the text
of the warning would need to be adjusted in those cases since
the expression need not evaluate to true.)
Martin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-22 19:48 ` Martin Sebor
@ 2015-07-22 20:02 ` Marek Polacek
2015-07-22 20:56 ` Martin Sebor
2015-07-29 10:24 ` Richard Earnshaw
1 sibling, 1 reply; 17+ messages in thread
From: Marek Polacek @ 2015-07-22 20:02 UTC (permalink / raw)
To: Martin Sebor; +Cc: GCC Patches, Joseph Myers, Jason Merrill
On Wed, Jul 22, 2015 at 12:43:53PM -0600, Martin Sebor wrote:
> On 07/14/2015 09:18 AM, Marek Polacek wrote:
> >Code such as "if (i == i)" is hardly ever desirable, so we should be able
> >to warn about this to prevent dumb mistakes.
>
> I haven't tried the patch or even studied it very carefully but
> I wonder if this is also the case when i is declared volatile.
> I.e., do we want to issue a warning there? (If we do, the text
> of the warning would need to be adjusted in those cases since
> the expression need not evaluate to true.)
We don't warn for volatiles because operand_equal_p doesn't consider
decls with side effects as same. Admittedly the test doesn't test
that...
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-22 20:02 ` Marek Polacek
@ 2015-07-22 20:56 ` Martin Sebor
2015-07-22 20:58 ` Marek Polacek
0 siblings, 1 reply; 17+ messages in thread
From: Martin Sebor @ 2015-07-22 20:56 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, Jason Merrill
On 07/22/2015 01:06 PM, Marek Polacek wrote:
> On Wed, Jul 22, 2015 at 12:43:53PM -0600, Martin Sebor wrote:
>> On 07/14/2015 09:18 AM, Marek Polacek wrote:
>>> Code such as "if (i == i)" is hardly ever desirable, so we should be able
>>> to warn about this to prevent dumb mistakes.
>>
>> I haven't tried the patch or even studied it very carefully but
>> I wonder if this is also the case when i is declared volatile.
>> I.e., do we want to issue a warning there? (If we do, the text
>> of the warning would need to be adjusted in those cases since
>> the expression need not evaluate to true.)
>
> We don't warn for volatiles because operand_equal_p doesn't consider
> decls with side effects as same. Admittedly the test doesn't test
> that...
I see. Thanks for clarifying that. Not warning makes sense. I would
suggest to add a test case for it then to make sure it's deliberate.
Martin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-22 20:56 ` Martin Sebor
@ 2015-07-22 20:58 ` Marek Polacek
2015-07-24 21:56 ` Jeff Law
0 siblings, 1 reply; 17+ messages in thread
From: Marek Polacek @ 2015-07-22 20:58 UTC (permalink / raw)
To: Martin Sebor; +Cc: GCC Patches, Joseph Myers, Jason Merrill
On Wed, Jul 22, 2015 at 01:48:03PM -0600, Martin Sebor wrote:
> On 07/22/2015 01:06 PM, Marek Polacek wrote:
> >On Wed, Jul 22, 2015 at 12:43:53PM -0600, Martin Sebor wrote:
> >>On 07/14/2015 09:18 AM, Marek Polacek wrote:
> >>>Code such as "if (i == i)" is hardly ever desirable, so we should be able
> >>>to warn about this to prevent dumb mistakes.
> >>
> >>I haven't tried the patch or even studied it very carefully but
> >>I wonder if this is also the case when i is declared volatile.
> >>I.e., do we want to issue a warning there? (If we do, the text
> >>of the warning would need to be adjusted in those cases since
> >>the expression need not evaluate to true.)
> >
> >We don't warn for volatiles because operand_equal_p doesn't consider
> >decls with side effects as same. Admittedly the test doesn't test
> >that...
>
> I see. Thanks for clarifying that. Not warning makes sense. I would
> suggest to add a test case for it then to make sure it's deliberate.
Here:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-07-22 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.
* 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.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index c94596f..6a79b95 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -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. */
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index a198e79..f0640c7 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -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);
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index dc760d7..cb3af48 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -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()
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index d3d0abd..e8c8189 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -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
diff --git gcc/cp/call.c gcc/cp/call.c
index 8dda1de..1be2527 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -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:
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 95ec376..1538711 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -14917,6 +14917,7 @@ tsubst_copy_and_build (tree t,
{
warning_sentinel s1(warn_type_limits);
warning_sentinel s2(warn_div_by_zero);
+ warning_sentinel s3(warn_tautological_compare);
tree op0 = RECUR (TREE_OPERAND (t, 0));
tree op1 = RECUR (TREE_OPERAND (t, 1));
tree r = build_x_binary_op
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 413ac16..f21c66b 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -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
diff --git gcc/testsuite/c-c++-common/Wtautological-compare-1.c gcc/testsuite/c-c++-common/Wtautological-compare-1.c
index e69de29..71ba4f8 100644
--- gcc/testsuite/c-c++-common/Wtautological-compare-1.c
+++ gcc/testsuite/c-c++-common/Wtautological-compare-1.c
@@ -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" } */
+}
diff --git gcc/testsuite/g++.dg/cpp0x/decltype-54581.C gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
index 4b81b5a..6322730 100644
--- gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
+++ gcc/testsuite/g++.dg/cpp0x/decltype-54581.C
@@ -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))));
diff --git gcc/testsuite/g++.dg/other/vector-compare.C gcc/testsuite/g++.dg/other/vector-compare.C
index 03ff5fd..77b0f51 100644
--- gcc/testsuite/g++.dg/other/vector-compare.C
+++ gcc/testsuite/g++.dg/other/vector-compare.C
@@ -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.
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-22 20:58 ` Marek Polacek
@ 2015-07-24 21:56 ` Jeff Law
2015-07-27 12:46 ` Marek Polacek
0 siblings, 1 reply; 17+ messages in thread
From: Jeff Law @ 2015-07-24 21:56 UTC (permalink / raw)
To: Marek Polacek, Martin Sebor; +Cc: GCC Patches, Joseph Myers, Jason Merrill
On 07/22/2015 02:02 PM, Marek Polacek wrote:
> On Wed, Jul 22, 2015 at 01:48:03PM -0600, Martin Sebor wrote:
>> On 07/22/2015 01:06 PM, Marek Polacek wrote:
>>> On Wed, Jul 22, 2015 at 12:43:53PM -0600, Martin Sebor wrote:
>>>> On 07/14/2015 09:18 AM, Marek Polacek wrote:
>>>>> Code such as "if (i == i)" is hardly ever desirable, so we should be able
>>>>> to warn about this to prevent dumb mistakes.
>>>>
>>>> I haven't tried the patch or even studied it very carefully but
>>>> I wonder if this is also the case when i is declared volatile.
>>>> I.e., do we want to issue a warning there? (If we do, the text
>>>> of the warning would need to be adjusted in those cases since
>>>> the expression need not evaluate to true.)
>>>
>>> We don't warn for volatiles because operand_equal_p doesn't consider
>>> decls with side effects as same. Admittedly the test doesn't test
>>> that...
>>
>> I see. Thanks for clarifying that. Not warning makes sense. I would
>> suggest to add a test case for it then to make sure it's deliberate.
>
> Here:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2015-07-22 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.
>
> * 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.
OK. Note I think the pt.c change will need trivial updating as it
conflicts with another change of yours that I approved recently.
jeff
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-24 21:56 ` Jeff Law
@ 2015-07-27 12:46 ` Marek Polacek
2015-07-27 16:09 ` Kyrill Tkachov
0 siblings, 1 reply; 17+ messages in thread
From: Marek Polacek @ 2015-07-27 12:46 UTC (permalink / raw)
To: Jeff Law; +Cc: Martin Sebor, GCC Patches, Joseph Myers, Jason Merrill
On Fri, Jul 24, 2015 at 03:10:17PM -0600, Jeff Law wrote:
> OK. Note I think the pt.c change will need trivial updating as it conflicts
> with another change of yours that I approved recently.
Committed after resolving trivial merge conflict.
Thanks for review,
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-27 12:46 ` Marek Polacek
@ 2015-07-27 16:09 ` Kyrill Tkachov
2015-07-27 16:10 ` Kyrill Tkachov
0 siblings, 1 reply; 17+ messages in thread
From: Kyrill Tkachov @ 2015-07-27 16:09 UTC (permalink / raw)
To: Marek Polacek, Jeff Law
Cc: Martin Sebor, GCC Patches, Joseph Myers, Jason Merrill
Hi Marek,
On 27/07/15 13:41, Marek Polacek wrote:
> On Fri, Jul 24, 2015 at 03:10:17PM -0600, Jeff Law wrote:
>> OK. Note I think the pt.c change will need trivial updating as it conflicts
>> with another change of yours that I approved recently.
> Committed after resolving trivial merge conflict.
This caused an arm bootstrap failure with -Werror.
https://gcc.gnu.org/bugzilla/process_bug.cgi
Thanks,
Kyrill
> Thanks for review,
>
> Marek
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-27 16:09 ` Kyrill Tkachov
@ 2015-07-27 16:10 ` Kyrill Tkachov
2015-07-27 16:19 ` Marek Polacek
0 siblings, 1 reply; 17+ messages in thread
From: Kyrill Tkachov @ 2015-07-27 16:10 UTC (permalink / raw)
To: Marek Polacek, Jeff Law
Cc: Martin Sebor, GCC Patches, Joseph Myers, Jason Merrill
On 27/07/15 16:59, Kyrill Tkachov wrote:
> Hi Marek,
>
> On 27/07/15 13:41, Marek Polacek wrote:
>> On Fri, Jul 24, 2015 at 03:10:17PM -0600, Jeff Law wrote:
>>> OK. Note I think the pt.c change will need trivial updating as it conflicts
>>> with another change of yours that I approved recently.
>> Committed after resolving trivial merge conflict.
> This caused an arm bootstrap failure with -Werror.
> https://gcc.gnu.org/bugzilla/process_bug.cgi
Err, the correct link is:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67030
>
> Thanks,
> Kyrill
>
>> Thanks for review,
>>
>> Marek
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-27 16:10 ` Kyrill Tkachov
@ 2015-07-27 16:19 ` Marek Polacek
2015-07-28 16:52 ` Steve Ellcey
0 siblings, 1 reply; 17+ messages in thread
From: Marek Polacek @ 2015-07-27 16:19 UTC (permalink / raw)
To: Kyrill Tkachov
Cc: Jeff Law, Martin Sebor, GCC Patches, Joseph Myers, Jason Merrill
On Mon, Jul 27, 2015 at 05:01:30PM +0100, Kyrill Tkachov wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67030
I hope the patch I've posted in the PR helps. Can you please check?
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-27 16:19 ` Marek Polacek
@ 2015-07-28 16:52 ` Steve Ellcey
2015-07-28 20:01 ` Marek Polacek
0 siblings, 1 reply; 17+ messages in thread
From: Steve Ellcey @ 2015-07-28 16:52 UTC (permalink / raw)
To: Marek Polacek
Cc: Kyrill Tkachov, Jeff Law, Martin Sebor, GCC Patches,
Joseph Myers, Jason Merrill
Marek,
I have run into a problem with this warning and building glibc.
sysdeps/ieee754/s_matherr.c has:
int
weak_function
__matherr(struct exception *x)
{
int n=0;
if(x->arg1!=x->arg1) return 0;
return n;
}
And arg1 is a floating point type. I think that if the value of
x->xarg1 is a NaN then the if statement should return TRUE because a NaN
never compares equal to anything, even another NaN (check with your
local IEEE expert). I believe this method of checking for a NaN is
fairly common and I am not sure if GCC should be emitting a warning for
it.
Steve Ellcey
sellcey@imgtec.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-28 16:52 ` Steve Ellcey
@ 2015-07-28 20:01 ` Marek Polacek
0 siblings, 0 replies; 17+ messages in thread
From: Marek Polacek @ 2015-07-28 20:01 UTC (permalink / raw)
To: Steve Ellcey
Cc: Kyrill Tkachov, Jeff Law, Martin Sebor, GCC Patches,
Joseph Myers, Jason Merrill
On Tue, Jul 28, 2015 at 09:02:51AM -0700, Steve Ellcey wrote:
> Marek,
>
> I have run into a problem with this warning and building glibc.
>
> sysdeps/ieee754/s_matherr.c has:
>
> int
> weak_function
> __matherr(struct exception *x)
> {
> int n=0;
> if(x->arg1!=x->arg1) return 0;
> return n;
> }
>
>
> And arg1 is a floating point type. I think that if the value of
> x->xarg1 is a NaN then the if statement should return TRUE because a NaN
> never compares equal to anything, even another NaN (check with your
> local IEEE expert). I believe this method of checking for a NaN is
> fairly common and I am not sure if GCC should be emitting a warning for
> it.
Oh, you're right. In IEEE-754, NaN != NaN. So I need to adjust the
warning and the documentation a bit. I suppose this is just about
using get_inner_reference and punting for FLOAT_TYPE_P (I'll try to fix
this tomorrow).
This certainly didn't occur to me when I was writing the warning...
Thanks for bringing this up.
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-22 19:48 ` Martin Sebor
2015-07-22 20:02 ` Marek Polacek
@ 2015-07-29 10:24 ` Richard Earnshaw
2015-07-29 11:22 ` Marek Polacek
1 sibling, 1 reply; 17+ messages in thread
From: Richard Earnshaw @ 2015-07-29 10:24 UTC (permalink / raw)
To: Martin Sebor, Marek Polacek, GCC Patches, Joseph Myers, Jason Merrill
On 22/07/15 19:43, Martin Sebor wrote:
> On 07/14/2015 09:18 AM, Marek Polacek wrote:
>> Code such as "if (i == i)" is hardly ever desirable, so we should be able
>> to warn about this to prevent dumb mistakes.
>
> I haven't tried the patch or even studied it very carefully but
> I wonder if this is also the case when i is declared volatile.
> I.e., do we want to issue a warning there? (If we do, the text
> of the warning would need to be adjusted in those cases since
> the expression need not evaluate to true.)
>
> Martin
>
It's also not true if i is an IEEE floating point type with a NaN value.
In that case this is a standard idiom for testing for a NaN.
R.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979)
2015-07-29 10:24 ` Richard Earnshaw
@ 2015-07-29 11:22 ` Marek Polacek
0 siblings, 0 replies; 17+ messages in thread
From: Marek Polacek @ 2015-07-29 11:22 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: Martin Sebor, GCC Patches, Joseph Myers, Jason Merrill
On Wed, Jul 29, 2015 at 10:59:43AM +0100, Richard Earnshaw wrote:
> On 22/07/15 19:43, Martin Sebor wrote:
> > On 07/14/2015 09:18 AM, Marek Polacek wrote:
> >> Code such as "if (i == i)" is hardly ever desirable, so we should be able
> >> to warn about this to prevent dumb mistakes.
> >
> > I haven't tried the patch or even studied it very carefully but
> > I wonder if this is also the case when i is declared volatile.
> > I.e., do we want to issue a warning there? (If we do, the text
> > of the warning would need to be adjusted in those cases since
> > the expression need not evaluate to true.)
> >
> > Martin
> >
>
> It's also not true if i is an IEEE floating point type with a NaN value.
> In that case this is a standard idiom for testing for a NaN.
Yep. Steve already raised this yesterday:
<https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02388.html>
I'm going to fix it.
Thanks,
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-07-29 10:40 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-14 15:19 [C/C++ PATCH] Implement -Wtautological-compare (PR c++/66555, c/54979) Marek Polacek
2015-07-14 22:04 ` Richard Sandiford
2015-07-14 22:14 ` Marek Polacek
2015-07-22 18:03 ` Marek Polacek
2015-07-22 19:48 ` Martin Sebor
2015-07-22 20:02 ` Marek Polacek
2015-07-22 20:56 ` Martin Sebor
2015-07-22 20:58 ` Marek Polacek
2015-07-24 21:56 ` Jeff Law
2015-07-27 12:46 ` Marek Polacek
2015-07-27 16:09 ` Kyrill Tkachov
2015-07-27 16:10 ` Kyrill Tkachov
2015-07-27 16:19 ` Marek Polacek
2015-07-28 16:52 ` Steve Ellcey
2015-07-28 20:01 ` Marek Polacek
2015-07-29 10:24 ` Richard Earnshaw
2015-07-29 11:22 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).