In this testcase, we weren't getting the benefits of fold's cleverness
in handling COND_EXPR because we were only calling fold_for_warn on the
condition itself. This patch changes check_function_arguments_recurse
to fold the entire COND_EXPR, and also fixes cp_fold to actually fold
COND_EXPR.
Along with this, I've cleaned up some other bits I noticed in cp_fold:
there was various unnecessary special-casing for unary and binary ops as
well, and we were clobbering an input CONSTRUCTOR rather than returning
a new folded one.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 0a3081a56bcd9b98d66ddc5eadf8c3fece9152dd
Author: Jason Merrill <ja...@redhat.com>
Date: Fri Jan 15 12:39:28 2016 -0500
PR c++/68767
gcc/c-family/
* c-common.c (check_function_arguments_recurse): Fold the whole
COND_EXPR, not just the condition.
gcc/cp/
* cp-gimplify.c (cp_fold) [COND_EXPR]: Simplify. Do fold COND_EXPR.
(contains_label_1, contains_label_p): Remove.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 0bfa1f6..1a2c21b 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -9765,15 +9765,19 @@ check_function_arguments_recurse (void (*callback)
if (TREE_CODE (param) == COND_EXPR)
{
- tree cond = fold_for_warn (TREE_OPERAND (param, 0));
- /* Check both halves of the conditional expression. */
- if (!integer_zerop (cond))
- check_function_arguments_recurse (callback, ctx,
- TREE_OPERAND (param, 1), param_num);
- if (!integer_nonzerop (cond))
- check_function_arguments_recurse (callback, ctx,
- TREE_OPERAND (param, 2), param_num);
- return;
+ /* Simplify to avoid warning for an impossible case. */
+ param = fold_for_warn (param);
+ if (TREE_CODE (param) == COND_EXPR)
+ {
+ /* Check both halves of the conditional expression. */
+ check_function_arguments_recurse (callback, ctx,
+ TREE_OPERAND (param, 1),
+ param_num);
+ check_function_arguments_recurse (callback, ctx,
+ TREE_OPERAND (param, 2),
+ param_num);
+ return;
+ }
}
(*callback) (ctx, param, param_num);
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index c0ee8e4..e151753 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1851,38 +1851,6 @@ cxx_omp_disregard_value_expr (tree decl, bool shared)
&& DECL_OMP_PRIVATIZED_MEMBER (decl);
}
-/* Callback for walk_tree, looking for LABEL_EXPR. Return *TP if it is
- a LABEL_EXPR; otherwise return NULL_TREE. Do not check the subtrees
- of GOTO_EXPR. */
-
-static tree
-contains_label_1 (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
-{
- switch (TREE_CODE (*tp))
- {
- case LABEL_EXPR:
- return *tp;
-
- case GOTO_EXPR:
- *walk_subtrees = 0;
-
- /* ... fall through ... */
-
- default:
- return NULL_TREE;
- }
-}
-
-/* Return whether the sub-tree ST contains a label which is accessible from
- outside the sub-tree. */
-
-static bool
-contains_label_p (tree st)
-{
- return
- walk_tree_without_duplicates (&st, contains_label_1 , NULL) != NULL_TREE;
-}
-
/* Perform folding on expression X. */
tree
@@ -2110,54 +2078,22 @@ cp_fold (tree x)
case VEC_COND_EXPR:
case COND_EXPR:
+ /* Don't bother folding a void condition, since it can't produce a
+ constant value. Also, some statement-level uses of COND_EXPR leave
+ one of the branches NULL, so folding would crash. */
+ if (VOID_TYPE_P (TREE_TYPE (x)))
+ return x;
+
loc = EXPR_LOCATION (x);
op0 = cp_fold_rvalue (TREE_OPERAND (x, 0));
-
- if (TREE_SIDE_EFFECTS (op0))
- break;
-
op1 = cp_fold (TREE_OPERAND (x, 1));
op2 = cp_fold (TREE_OPERAND (x, 2));
- if (TREE_CODE (op0) == INTEGER_CST)
- {
- tree un;
-
- if (integer_zerop (op0))
- {
- un = op1;
- r = op2;
- }
- else
- {
- un = op2;
- r = op1;
- }
-
- if ((!TREE_SIDE_EFFECTS (un) || !contains_label_p (un))
- && (! VOID_TYPE_P (TREE_TYPE (r)) || VOID_TYPE_P (x)))
- {
- if (CAN_HAVE_LOCATION_P (r)
- && EXPR_LOCATION (r) != loc
- && !(TREE_CODE (r) == SAVE_EXPR
- || TREE_CODE (r) == TARGET_EXPR
- || TREE_CODE (r) == BIND_EXPR))
- {
- r = copy_node (r);
- SET_EXPR_LOCATION (r, loc);
- }
- x = r;
- }
-
- break;
- }
-
- if (VOID_TYPE_P (TREE_TYPE (x)))
- break;
-
- x = build3_loc (loc, code, TREE_TYPE (x), op0, op1, op2);
-
- if (code != COND_EXPR)
+ if (op0 != TREE_OPERAND (x, 0)
+ || op1 != TREE_OPERAND (x, 1)
+ || op2 != TREE_OPERAND (x, 2))
+ x = fold_build3_loc (loc, code, TREE_TYPE (x), op0, op1, op2);
+ else
x = fold (x);
break;
diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull2.C b/gcc/testsuite/g++.dg/warn/Wnonnull2.C
new file mode 100644
index 0000000..6757437
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wnonnull2.C
@@ -0,0 +1,9 @@
+// PR c++/68767
+// { dg-options "-Wnonnull" }
+
+extern int len (const char*) __attribute__ ((__nonnull__ (1)));
+
+int f (int x)
+{
+ return len ((x ? "x" : 0) ? (x ? "x" : 0) : "x");
+}
commit 40540c93f27e92ee04c62d691809446b8f4b090d
Author: Jason Merrill <ja...@redhat.com>
Date: Fri Jan 15 13:00:19 2016 -0500
* cp-gimplify.c (cp_fold): Remove unnecessary special cases.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index e151753..2dc53ae 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1942,19 +1942,8 @@ cp_fold (tree x)
if (VOID_TYPE_P (TREE_TYPE (x)))
return x;
- if (!TREE_OPERAND (x, 0)
- || TREE_CODE (TREE_OPERAND (x, 0)) == NON_LVALUE_EXPR)
- return x;
-
loc = EXPR_LOCATION (x);
- op0 = TREE_OPERAND (x, 0);
-
- if (TREE_CODE (x) == NOP_EXPR
- && TREE_OVERFLOW_P (op0)
- && TREE_TYPE (x) == TREE_TYPE (op0))
- return x;
-
- op0 = cp_fold_maybe_rvalue (op0, rval_ops);
+ op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);
if (op0 != TREE_OPERAND (x, 0))
x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
@@ -2000,16 +1989,6 @@ cp_fold (tree x)
case POSTDECREMENT_EXPR:
case POSTINCREMENT_EXPR:
case INIT_EXPR:
-
- loc = EXPR_LOCATION (x);
- op0 = cp_fold (TREE_OPERAND (x, 0));
- op1 = cp_fold_rvalue (TREE_OPERAND (x, 1));
-
- if (TREE_OPERAND (x, 0) != op0 || TREE_OPERAND (x, 1) != op1)
- x = build2_loc (loc, code, TREE_TYPE (x), op0, op1);
-
- break;
-
case PREDECREMENT_EXPR:
case PREINCREMENT_EXPR:
case COMPOUND_EXPR:
@@ -2054,25 +2033,12 @@ cp_fold (tree x)
loc = EXPR_LOCATION (x);
op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);
op1 = cp_fold_rvalue (TREE_OPERAND (x, 1));
- if ((code == COMPOUND_EXPR || code == MODIFY_EXPR)
- && ((op1 && TREE_SIDE_EFFECTS (op1))
- || (op0 && TREE_SIDE_EFFECTS (op0))))
- {
- if (op0 != TREE_OPERAND (x, 0) || op1 != TREE_OPERAND (x, 1))
- x = build2_loc (loc, code, TREE_TYPE (x), op0, op1);
- break;
- }
- if (TREE_CODE (x) == COMPOUND_EXPR && !op0)
- op0 = build_empty_stmt (loc);
if (op0 != TREE_OPERAND (x, 0) || op1 != TREE_OPERAND (x, 1))
x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1);
else
x = fold (x);
- if (TREE_CODE (x) == COMPOUND_EXPR && TREE_OPERAND (x, 0) == NULL_TREE
- && TREE_OPERAND (x, 1))
- return TREE_OPERAND (x, 1);
break;
case VEC_COND_EXPR:
commit 4e480f0e4eede58f60caf8e05c94709e5a5f3fc0
Author: Jason Merrill <ja...@redhat.com>
Date: Fri Jan 15 13:00:10 2016 -0500
* cp-gimplify.c (cp_fold) [CONSTRUCTOR]: Don't clobber the input.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 2dc53ae..5c4d3c1 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2125,9 +2125,22 @@ cp_fold (tree x)
{
unsigned i;
constructor_elt *p;
+ bool changed = false;
vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (x);
+ vec<constructor_elt, va_gc> *nelts = NULL;
+ vec_safe_reserve (nelts, vec_safe_length (elts));
FOR_EACH_VEC_SAFE_ELT (elts, i, p)
- p->value = cp_fold (p->value);
+ {
+ tree op = cp_fold (p->value);
+ constructor_elt e = { p->index, op };
+ nelts->quick_push (e);
+ if (op != p->value)
+ changed = true;
+ }
+ if (changed)
+ x = build_constructor (TREE_TYPE (x), nelts);
+ else
+ vec_free (nelts);
break;
}
case TREE_VEC:
