On Thu, May 04, 2017 at 02:13:24PM +0200, Richard Biener wrote:
> On Thu, May 4, 2017 at 2:11 PM, Marek Polacek <pola...@redhat.com> wrote:
> > On Thu, May 04, 2017 at 12:42:03PM +0200, Richard Biener wrote:
> >> > +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;
> >>
> >> This changes trees in-place -- do you need to operate on a copy?
> >
> > Ugh, yes. But I can't simply copy_node, because that creates new VAR_DECLs,
> > and operand_equal_p would consider them unequal. Hmm... We need something
> > else.
>
> unshare_expr?
Yeah, so:
2017-05-04 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..aa0cfa9 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. If not see
#include "intl.h"
#include "asan.h"
#include "gcc-rich-location.h"
+#include "gimplify.h"
/* Print a warning if a constant expression had overflow in folding.
Invoke this function on every expression that the language
@@ -112,6 +113,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 +205,11 @@ 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. */
+ op_left = unshare_expr (op_left);
+ 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 +220,11 @@ warn_logical_operator (location_t location, enum
tree_code code, tree type,
if (tem && integer_zerop (tem))
return;
+ op_right = unshare_expr (op_right);
+ 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
