From: Marek Polacek Date: Mon, 27 Jul 2015 12:40:45 +0000 (+0000) Subject: re PR c++/66555 (Fails to warn for if (j == 0 && i == i)) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=05b28fd6f91016151e758c2414d7245a54512c99;p=gcc.git re PR c++/66555 (Fails to warn for if (j == 0 && i == i)) 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 --- diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e0eb3a2ee6a..177f71c8738 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2015-07-27 Marek Polacek + + PR c++/66555 + PR c/54979 + * doc/invoke.texi: Document -Wtautological-compare. + 2015-07-27 Richard Biener * genmatch.c (decision_tree::gen_gimple): Split out large diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index caedf8e2267..4d2ed59ea7a 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,12 @@ +2015-07-27 Marek Polacek + + 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 * c-ubsan.c (ubsan_instrument_division): Use unshare_expr throughout. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index c94596f89ba..6a79b95c27c 100644 --- a/gcc/c-family/c-common.c +++ b/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 a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index a198e794f5b..f0640c7f86c 100644 --- a/gcc/c-family/c-common.h +++ b/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 a/gcc/c-family/c.opt b/gcc/c-family/c.opt index dc760d71799..cb3af48dc47 100644 --- a/gcc/c-family/c.opt +++ b/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 a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 18443aea3db..08d62460768 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,3 +1,9 @@ +2015-07-27 Marek Polacek + + PR c++/66555 + PR c/54979 + * c-typeck.c (parser_build_binary_op): Call warn_tautological_cmp. + 2015-07-20 Marek Polacek PR c++/55095 diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index d3d0abdb2cb..e8c818989ee 100644 --- a/gcc/c/c-typeck.c +++ b/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 a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index a9fa4457c68..2c5fae1274d 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,11 @@ +2015-07-27 Marek Polacek + + 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 PR c++/18969 diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 8dda1deeb8c..1be2527358a 100644 --- a/gcc/cp/call.c +++ b/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 a/gcc/cp/pt.c b/gcc/cp/pt.c index 971c98ee0d9..f15d16fe269 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -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 diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 54e7a679c2f..e988444bdb7 100644 --- a/gcc/doc/invoke.texi +++ b/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 a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index c711f8362d5..8b572c28fee 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2015-07-27 Marek Polacek + + PR c++/66555 + PR c/54979 + * c-c++-common/Wtautological-compare-1.c: New test. + 2015-07-26 Patrick Palka 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 index 00000000000..71ba4f80484 --- /dev/null +++ b/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 a/gcc/testsuite/g++.dg/cpp0x/decltype-54581.C b/gcc/testsuite/g++.dg/cpp0x/decltype-54581.C index 4b81b5ab888..63227303225 100644 --- a/gcc/testsuite/g++.dg/cpp0x/decltype-54581.C +++ b/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 a/gcc/testsuite/g++.dg/other/vector-compare.C b/gcc/testsuite/g++.dg/other/vector-compare.C index 03ff5fd7255..77b0f5153ca 100644 --- a/gcc/testsuite/g++.dg/other/vector-compare.C +++ b/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.