On Wed, Nov 18, 2015 at 09:14:46PM +0000, Joseph Myers wrote:
> remove_c_maybe_const_expr doesn't seem to be quite what you want. Apart
> from this not being a case covered by the comment on the function, you're
> ignoring the possibility of the side effects in the
> C_MAYBE_CONST_EXPR_PRE. So I think you want something else: if either
> argument is a C_MAYBE_CONST_EXPR whose C_MAYBE_CONST_EXPR_PRE is non-NULL
> and has side-effects, don't run the comparison, and otherwise it's OK to
> go down to the C_MAYBE_CONST_EXPR_EXPR.
Indeed, thanks. The following variant does what you suggest. Perhaps
I should've introduced a new helper function, but I couldn't find a
proper name.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-11-19 Marek Polacek <pola...@redhat.com>
PR c/68412
* c-typeck.c (parser_build_binary_op): Properly handle
C_MAYBE_CONST_EXPR before calling warn_tautological_cmp.
* gcc.dg/pr68412-2.c: New test.
* gcc.dg/pr68412.c: New test.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index c18c307..5cb0f7e 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3512,7 +3512,28 @@ parser_build_binary_op (location_t location, enum
tree_code code,
code1, arg1.value, code2, arg2.value);
if (warn_tautological_compare)
- warn_tautological_cmp (location, code, arg1.value, arg2.value);
+ {
+ tree lhs = arg1.value;
+ tree rhs = arg2.value;
+ if (TREE_CODE (lhs) == C_MAYBE_CONST_EXPR)
+ {
+ if (C_MAYBE_CONST_EXPR_PRE (lhs) != NULL_TREE
+ && TREE_SIDE_EFFECTS (C_MAYBE_CONST_EXPR_PRE (lhs)))
+ lhs = NULL_TREE;
+ else
+ lhs = C_MAYBE_CONST_EXPR_EXPR (lhs);
+ }
+ if (TREE_CODE (rhs) == C_MAYBE_CONST_EXPR)
+ {
+ if (C_MAYBE_CONST_EXPR_PRE (rhs) != NULL_TREE
+ && TREE_SIDE_EFFECTS (C_MAYBE_CONST_EXPR_PRE (rhs)))
+ rhs = NULL_TREE;
+ else
+ rhs = C_MAYBE_CONST_EXPR_EXPR (rhs);
+ }
+ if (lhs != NULL_TREE && rhs != NULL_TREE)
+ warn_tautological_cmp (location, code, lhs, rhs);
+ }
if (warn_logical_not_paren
&& TREE_CODE_CLASS (code) == tcc_comparison
diff --git gcc/testsuite/gcc.dg/pr68412-2.c gcc/testsuite/gcc.dg/pr68412-2.c
index e69de29..be1dcfa 100644
--- gcc/testsuite/gcc.dg/pr68412-2.c
+++ gcc/testsuite/gcc.dg/pr68412-2.c
@@ -0,0 +1,15 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+int
+fn1 (int i)
+{
+ return ({ i; }) == ({ i; }); /* { dg-warning "self-comparison always
evaluates to true" } */
+}
+
+int
+fn2 (int i)
+{
+ return ({ i + 1; }) != ({ i + 1; }); /* { dg-warning "self-comparison always
evaluates to false" } */
+}
diff --git gcc/testsuite/gcc.dg/pr68412.c gcc/testsuite/gcc.dg/pr68412.c
index e69de29..825eb63 100644
--- gcc/testsuite/gcc.dg/pr68412.c
+++ gcc/testsuite/gcc.dg/pr68412.c
@@ -0,0 +1,41 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+#define M (sizeof (int) * __CHAR_BIT__)
+
+int
+fn1 (int i)
+{
+ return i == (-1 << 8); /* { dg-warning "left shift of negative value" } */
+}
+
+int
+fn2 (int i)
+{
+ return i == (1 << M); /* { dg-warning "left shift count" } */
+}
+
+int
+fn3 (int i)
+{
+ return i == 10 << (M - 1); /* { dg-warning "requires" } */
+}
+
+int
+fn4 (int i)
+{
+ return i == 1 << -1; /* { dg-warning "left shift count" } */
+}
+
+int
+fn5 (int i)
+{
+ return i == 1 >> M; /* { dg-warning "right shift count" } */
+}
+
+int
+fn6 (int i)
+{
+ return i == 1 >> -1; /* { dg-warning "right shift count" } */
+}
Marek
