This PR points out a missing -Wlogical-op warning (unless you use -fwrapv).
We end up calling warn_logical_operator with op_left that is
C_M_C_E <a + 1> != 0
and op_right that is
a + 1
But make_range just cannot handle C_M_C_Es right; for exprs it simply picks the
first operand and that doesn't work with C_M_C_E, where we'd need to use
C_MAYBE_CONST_EXPR_EXPR. warn_logical_operator has code that strips C_M_C_E
but that is insufficient. I think we have to use walk_tree, because as this
testcase shows, those C_M_C_Es might not be the outermost expression.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-05-03 Marek Polacek <pola...@redhat.com>
PR c/80525
* c-warn.c (unwrap_c_maybe_const): New.
(warn_logical_operator): Call it.
* c-c++-common/Wlogical-op-1.c: Don't use -fwrapv anymore.
* c-c++-common/Wlogical-op-2.c: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 45dd583..0f17bc5 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -112,6 +112,21 @@ overflow_warning (location_t loc, tree value)
}
}
+/* Helper function for walk_tree. Unwrap C_MAYBE_CONST_EXPRs in an expression
+ pointed to by TP. */
+
+static tree
+unwrap_c_maybe_const (tree *tp, int *walk_subtrees, void *)
+{
+ if (TREE_CODE (*tp) == C_MAYBE_CONST_EXPR)
+ {
+ *tp = C_MAYBE_CONST_EXPR_EXPR (*tp);
+ /* C_MAYBE_CONST_EXPRs don't nest. */
+ *walk_subtrees = false;
+ }
+ return NULL_TREE;
+}
+
/* Warn about uses of logical || / && operator in a context where it
is likely that the bitwise equivalent was intended by the
programmer. We have seen an expression in which CODE is a binary
@@ -189,11 +204,10 @@ warn_logical_operator (location_t location, enum
tree_code code, tree type,
(with OR) or trivially false (with AND). If so, do not warn.
This is a common idiom for testing ranges of data types in
portable code. */
+ walk_tree_without_duplicates (&op_left, unwrap_c_maybe_const, NULL);
lhs = make_range (op_left, &in0_p, &low0, &high0, &strict_overflow_p);
if (!lhs)
return;
- if (TREE_CODE (lhs) == C_MAYBE_CONST_EXPR)
- lhs = C_MAYBE_CONST_EXPR_EXPR (lhs);
/* If this is an OR operation, invert both sides; now, the result
should be always false to get a warning. */
@@ -204,11 +218,10 @@ warn_logical_operator (location_t location, enum
tree_code code, tree type,
if (tem && integer_zerop (tem))
return;
+ walk_tree_without_duplicates (&op_right, unwrap_c_maybe_const, NULL);
rhs = make_range (op_right, &in1_p, &low1, &high1, &strict_overflow_p);
if (!rhs)
return;
- if (TREE_CODE (rhs) == C_MAYBE_CONST_EXPR)
- rhs = C_MAYBE_CONST_EXPR_EXPR (rhs);
/* If this is an OR operation, invert both sides; now, the result
should be always false to get a warning. */
diff --git gcc/testsuite/c-c++-common/Wlogical-op-1.c
gcc/testsuite/c-c++-common/Wlogical-op-1.c
index e89a35a..c5f992a 100644
--- gcc/testsuite/c-c++-common/Wlogical-op-1.c
+++ gcc/testsuite/c-c++-common/Wlogical-op-1.c
@@ -1,8 +1,6 @@
/* PR c/63357 */
/* { dg-do compile } */
-/* For -fwrapv see PR80525, xfailing the subtest isn't possible as it passes
- with the C++ FE which doesn't have maybe_const_expr. */
-/* { dg-options "-fwrapv -Wlogical-op" } */
+/* { dg-options "-Wlogical-op" } */
#ifndef __cplusplus
# define bool _Bool
diff --git gcc/testsuite/c-c++-common/Wlogical-op-2.c
gcc/testsuite/c-c++-common/Wlogical-op-2.c
index e69de29..6360ef9 100644
--- gcc/testsuite/c-c++-common/Wlogical-op-2.c
+++ gcc/testsuite/c-c++-common/Wlogical-op-2.c
@@ -0,0 +1,12 @@
+/* PR c/80525 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-op" } */
+
+int
+fn (int a, int b)
+{
+ if ((a + 1) && (a + 1)) /* { dg-warning "logical .and. of equal expressions"
} */
+ return a;
+ if ((a + 1) || (a + 1)) /* { dg-warning "logical .or. of equal expressions"
} */
+ return b;
+}
Marek
