* [PATCH] c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672]
@ 2021-01-15 16:26 Jakub Jelinek
2021-01-19 21:01 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2021-01-15 16:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
The following testcase is rejected even when it is valid.
The problem is that potential_constant_expression_1 doesn't have the
accurate *jump_target tracking cxx_eval_* has, and when the loop has
a condition that isn't guaranteed to be always true, the body isn't walked
at all. That is mostly a correct conservative behavior, except that it
doesn't detect if there are any return statements in the body, which means
the loop might return instead of falling through to the next statement.
We already have code for return stmt discovery in code snippets we don't
try to evaluate for switches, so this patch reuses that for FOR_STMT
and WHILE_STMT bodies.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Note, I haven't touched FOR_EXPR, with statement expressions it could
have return stmts in it too, or it could have break or continue statements
that wouldn't bind to the current loop but to something outer. That
case is clearly mishandled by potential_constant_expression_1 even
when the condition is missing or is always true, and it wouldn't surprise me
if cxx_eval_* didn't handle it right either, so I'm deferring that to
separate PR for later. We'd need proper test coverage for all of that.
2021-01-15 Jakub Jelinek <jakub@redhat.com>
PR c++/98672
* constexpr.c (potential_constant_expression_1) <case FOR_STMT>,
<case WHILE_STMT>: If the condition isn't constant true, check if
the loop body can contain a return stmt.
* g++.dg/cpp1y/constexpr-98672.C: New test.
--- gcc/cp/constexpr.c.jj 2021-01-13 19:19:44.368469462 +0100
+++ gcc/cp/constexpr.c 2021-01-14 12:02:27.347042704 +0100
@@ -8190,7 +8190,17 @@ potential_constant_expression_1 (tree t,
/* If we couldn't evaluate the condition, it might not ever be
true. */
if (!integer_onep (tmp))
- return true;
+ {
+ /* Before returning true, check if the for body can contain
+ a return. */
+ hash_set<tree> pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&FOR_BODY (t), check_for_return_continue,
+ &data, &pset))
+ *jump_target = ret_expr;
+ return true;
+ }
}
if (!RECUR (FOR_EXPR (t), any))
return false;
@@ -8219,7 +8229,17 @@ potential_constant_expression_1 (tree t,
tmp = cxx_eval_outermost_constant_expr (tmp, true);
/* If we couldn't evaluate the condition, it might not ever be true. */
if (!integer_onep (tmp))
- return true;
+ {
+ /* Before returning true, check if the while body can contain
+ a return. */
+ hash_set<tree> pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&WHILE_BODY (t), check_for_return_continue,
+ &data, &pset))
+ *jump_target = ret_expr;
+ return true;
+ }
if (!RECUR (WHILE_BODY (t), any))
return false;
if (breaks (jump_target) || continues (jump_target))
--- gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C.jj 2021-01-14 12:19:24.842438847 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C 2021-01-14 12:07:33.935551155 +0100
@@ -0,0 +1,35 @@
+// PR c++/98672
+// { dg-do compile { target c++14 } }
+
+void
+foo ()
+{
+}
+
+constexpr int
+bar ()
+{
+ for (int i = 0; i < 5; ++i)
+ return i;
+ foo ();
+ return 0;
+}
+
+constexpr int
+baz ()
+{
+ int i = 0;
+ while (i < 5)
+ {
+ if (i == 3)
+ return i;
+ else
+ ++i;
+ }
+ foo ();
+ return 0;
+}
+
+constexpr int i = bar ();
+constexpr int j = baz ();
+static_assert (i == 0 && j == 3, "");
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672]
2021-01-15 16:26 [PATCH] c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672] Jakub Jelinek
@ 2021-01-19 21:01 ` Jason Merrill
2021-01-19 21:54 ` [PATCH] c++, v2: " Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2021-01-19 21:01 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/15/21 11:26 AM, Jakub Jelinek wrote:
> The following testcase is rejected even when it is valid.
> The problem is that potential_constant_expression_1 doesn't have the
> accurate *jump_target tracking cxx_eval_* has, and when the loop has
> a condition that isn't guaranteed to be always true, the body isn't walked
> at all. That is mostly a correct conservative behavior, except that it
> doesn't detect if there are any return statements in the body, which means
> the loop might return instead of falling through to the next statement.
> We already have code for return stmt discovery in code snippets we don't
> try to evaluate for switches, so this patch reuses that for FOR_STMT
> and WHILE_STMT bodies.
Hmm, IF_STMT probably also needs to check the else clause, if the
condition isn't a known constant.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Note, I haven't touched FOR_EXPR, with statement expressions it could
> have return stmts in it too, or it could have break or continue statements
> that wouldn't bind to the current loop but to something outer. That
> case is clearly mishandled by potential_constant_expression_1 even
> when the condition is missing or is always true, and it wouldn't surprise me
> if cxx_eval_* didn't handle it right either, so I'm deferring that to
> separate PR for later. We'd need proper test coverage for all of that.
Agreed.
> 2021-01-15 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/98672
> * constexpr.c (potential_constant_expression_1) <case FOR_STMT>,
> <case WHILE_STMT>: If the condition isn't constant true, check if
> the loop body can contain a return stmt.
>
> * g++.dg/cpp1y/constexpr-98672.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2021-01-13 19:19:44.368469462 +0100
> +++ gcc/cp/constexpr.c 2021-01-14 12:02:27.347042704 +0100
> @@ -8190,7 +8190,17 @@ potential_constant_expression_1 (tree t,
> /* If we couldn't evaluate the condition, it might not ever be
> true. */
> if (!integer_onep (tmp))
> - return true;
> + {
> + /* Before returning true, check if the for body can contain
> + a return. */
> + hash_set<tree> pset;
> + check_for_return_continue_data data = { &pset, NULL_TREE };
> + if (tree ret_expr
> + = cp_walk_tree (&FOR_BODY (t), check_for_return_continue,
> + &data, &pset))
> + *jump_target = ret_expr;
> + return true;
> + }
> }
> if (!RECUR (FOR_EXPR (t), any))
> return false;
> @@ -8219,7 +8229,17 @@ potential_constant_expression_1 (tree t,
> tmp = cxx_eval_outermost_constant_expr (tmp, true);
> /* If we couldn't evaluate the condition, it might not ever be true. */
> if (!integer_onep (tmp))
> - return true;
> + {
> + /* Before returning true, check if the while body can contain
> + a return. */
> + hash_set<tree> pset;
> + check_for_return_continue_data data = { &pset, NULL_TREE };
> + if (tree ret_expr
> + = cp_walk_tree (&WHILE_BODY (t), check_for_return_continue,
> + &data, &pset))
> + *jump_target = ret_expr;
> + return true;
> + }
> if (!RECUR (WHILE_BODY (t), any))
> return false;
> if (breaks (jump_target) || continues (jump_target))
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C.jj 2021-01-14 12:19:24.842438847 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C 2021-01-14 12:07:33.935551155 +0100
> @@ -0,0 +1,35 @@
> +// PR c++/98672
> +// { dg-do compile { target c++14 } }
> +
> +void
> +foo ()
> +{
> +}
> +
> +constexpr int
> +bar ()
> +{
> + for (int i = 0; i < 5; ++i)
> + return i;
> + foo ();
> + return 0;
> +}
> +
> +constexpr int
> +baz ()
> +{
> + int i = 0;
> + while (i < 5)
> + {
> + if (i == 3)
> + return i;
> + else
> + ++i;
> + }
> + foo ();
> + return 0;
> +}
> +
> +constexpr int i = bar ();
> +constexpr int j = baz ();
> +static_assert (i == 0 && j == 3, "");
>
> Jakub
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] c++, v2: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672]
2021-01-19 21:01 ` Jason Merrill
@ 2021-01-19 21:54 ` Jakub Jelinek
2021-01-20 7:31 ` Jakub Jelinek
2021-01-21 15:27 ` Jason Merrill
0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2021-01-19 21:54 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Tue, Jan 19, 2021 at 04:01:49PM -0500, Jason Merrill wrote:
> Hmm, IF_STMT probably also needs to check the else clause, if the condition
> isn't a known constant.
You're right, I thought it was ok because it recurses with tf_none, but
if the then branch is potentially constant and only else returns, continues
or breaks, then as the enhanced testcase shows we were mishandling it too.
So like this then if it passes bootstrap/regtest?
2021-01-19 Jakub Jelinek <jakub@redhat.com>
PR c++/98672
* constexpr.c (check_for_return_continue_data): Add break_stmt member.
(check_for_return_continue): Also look for BREAK_STMT. Handle SWITCH_STMT
by ignoring break_stmt from its body.
(potential_constant_expression_1) <case FOR_STMT>,
<case WHILE_STMT>: If the condition isn't constant true, check if
the loop body can contain a return stmt.
<case SWITCH_STMT>: Adjust check_for_return_continue_data initializer.
<case IF_STMT>: If recursion with tf_none is successful, merge
*jump_target from the branches - returns with highest priority, breaks
or continues lower. If then branch is potentially constant and
doesn't return, check the else branch if it could return, break or
continue.
* g++.dg/cpp1y/constexpr-98672.C: New test.
--- gcc/cp/constexpr.c.jj 2021-01-14 12:49:50.500644142 +0100
+++ gcc/cp/constexpr.c 2021-01-19 22:44:17.845322567 +0100
@@ -7649,15 +7649,16 @@ check_automatic_or_tls (tree ref)
struct check_for_return_continue_data {
hash_set<tree> *pset;
tree continue_stmt;
+ tree break_stmt;
};
/* Helper function for potential_constant_expression_1 SWITCH_STMT handling,
called through cp_walk_tree. Return the first RETURN_EXPR found, or note
- the first CONTINUE_STMT if RETURN_EXPR is not found. */
+ the first CONTINUE_STMT and/or BREAK_STMT if RETURN_EXPR is not found. */
static tree
check_for_return_continue (tree *tp, int *walk_subtrees, void *data)
{
- tree t = *tp, s;
+ tree t = *tp, s, b;
check_for_return_continue_data *d = (check_for_return_continue_data *) data;
switch (TREE_CODE (t))
{
@@ -7669,6 +7670,11 @@ check_for_return_continue (tree *tp, int
d->continue_stmt = t;
break;
+ case BREAK_STMT:
+ if (d->break_stmt == NULL_TREE)
+ d->break_stmt = t;
+ break;
+
#define RECUR(x) \
if (tree r = cp_walk_tree (&x, check_for_return_continue, data, \
d->pset)) \
@@ -7680,16 +7686,20 @@ check_for_return_continue (tree *tp, int
*walk_subtrees = 0;
RECUR (DO_COND (t));
s = d->continue_stmt;
+ b = d->break_stmt;
RECUR (DO_BODY (t));
d->continue_stmt = s;
+ d->break_stmt = b;
break;
case WHILE_STMT:
*walk_subtrees = 0;
RECUR (WHILE_COND (t));
s = d->continue_stmt;
+ b = d->break_stmt;
RECUR (WHILE_BODY (t));
d->continue_stmt = s;
+ d->break_stmt = b;
break;
case FOR_STMT:
@@ -7698,16 +7708,28 @@ check_for_return_continue (tree *tp, int
RECUR (FOR_COND (t));
RECUR (FOR_EXPR (t));
s = d->continue_stmt;
+ b = d->break_stmt;
RECUR (FOR_BODY (t));
d->continue_stmt = s;
+ d->break_stmt = b;
break;
case RANGE_FOR_STMT:
*walk_subtrees = 0;
RECUR (RANGE_FOR_EXPR (t));
s = d->continue_stmt;
+ b = d->break_stmt;
RECUR (RANGE_FOR_BODY (t));
d->continue_stmt = s;
+ d->break_stmt = b;
+ break;
+
+ case SWITCH_STMT:
+ *walk_subtrees = 0;
+ RECUR (SWITCH_STMT_COND (t));
+ b = d->break_stmt;
+ RECUR (SWITCH_STMT_BODY (t));
+ d->break_stmt = b;
break;
#undef RECUR
@@ -8190,7 +8212,18 @@ potential_constant_expression_1 (tree t,
/* If we couldn't evaluate the condition, it might not ever be
true. */
if (!integer_onep (tmp))
- return true;
+ {
+ /* Before returning true, check if the for body can contain
+ a return. */
+ hash_set<tree> pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE,
+ NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&FOR_BODY (t), check_for_return_continue,
+ &data, &pset))
+ *jump_target = ret_expr;
+ return true;
+ }
}
if (!RECUR (FOR_EXPR (t), any))
return false;
@@ -8219,7 +8252,18 @@ potential_constant_expression_1 (tree t,
tmp = cxx_eval_outermost_constant_expr (tmp, true);
/* If we couldn't evaluate the condition, it might not ever be true. */
if (!integer_onep (tmp))
- return true;
+ {
+ /* Before returning true, check if the while body can contain
+ a return. */
+ hash_set<tree> pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE,
+ NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&WHILE_BODY (t), check_for_return_continue,
+ &data, &pset))
+ *jump_target = ret_expr;
+ return true;
+ }
if (!RECUR (WHILE_BODY (t), any))
return false;
if (breaks (jump_target) || continues (jump_target))
@@ -8238,7 +8282,8 @@ potential_constant_expression_1 (tree t,
else
{
hash_set<tree> pset;
- check_for_return_continue_data data = { &pset, NULL_TREE };
+ check_for_return_continue_data data = { &pset, NULL_TREE,
+ NULL_TREE };
if (tree ret_expr
= cp_walk_tree (&SWITCH_STMT_BODY (t), check_for_return_continue,
&data, &pset))
@@ -8648,11 +8693,46 @@ potential_constant_expression_1 (tree t,
return RECUR (TREE_OPERAND (t, 2), want_rval);
else if (TREE_CODE (tmp) == INTEGER_CST)
return RECUR (TREE_OPERAND (t, 1), want_rval);
+ tmp = *jump_target;
for (i = 1; i < 3; ++i)
- if (potential_constant_expression_1 (TREE_OPERAND (t, i),
- want_rval, strict, now,
- tf_none, jump_target))
- return true;
+ {
+ tree this_jump_target = tmp;
+ if (potential_constant_expression_1 (TREE_OPERAND (t, i),
+ want_rval, strict, now,
+ tf_none, &this_jump_target))
+ {
+ if (returns (&this_jump_target))
+ *jump_target = this_jump_target;
+ else if (!returns (jump_target))
+ {
+ if (breaks (&this_jump_target)
+ || continues (&this_jump_target))
+ *jump_target = this_jump_target;
+ if (i == 1)
+ {
+ /* If the then branch is potentially constant, but
+ does not return, check if the else branch
+ couldn't return, break or continue. */
+ hash_set<tree> pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE,
+ NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&TREE_OPERAND (t, 2),
+ check_for_return_continue, &data,
+ &pset))
+ *jump_target = ret_expr;
+ else if (*jump_target == NULL_TREE)
+ {
+ if (data.continue_stmt)
+ *jump_target = data.continue_stmt;
+ else if (data.break_stmt)
+ *jump_target = data.break_stmt;
+ }
+ }
+ }
+ return true;
+ }
+ }
if (flags & tf_error)
error_at (loc, "expression %qE is not a constant expression", t);
return false;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C.jj 2021-01-19 22:13:17.031454887 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C 2021-01-19 22:46:48.989605151 +0100
@@ -0,0 +1,92 @@
+// PR c++/98672
+// { dg-do compile { target c++14 } }
+
+void
+foo ()
+{
+}
+
+constexpr int
+bar ()
+{
+ for (int i = 0; i < 5; ++i)
+ return i;
+ foo ();
+ return 0;
+}
+
+constexpr int
+baz ()
+{
+ int i = 0;
+ while (i < 5)
+ {
+ if (i == 3)
+ return i;
+ else
+ ++i;
+ }
+ foo ();
+ return 0;
+}
+
+constexpr int
+qux (int x)
+{
+ if (x > 10)
+ ++x;
+ else
+ return 7;
+ foo ();
+ return 0;
+}
+
+constexpr int
+corge (int x)
+{
+ for (int a = 1; ; a++)
+ {
+ if (x > 10)
+ ++x;
+ else
+ return 4;
+ foo ();
+ }
+}
+
+constexpr int
+garply (int x)
+{
+ for (int a = 1; ; a++)
+ {
+ if (x > 10)
+ ++x;
+ else
+ break;
+ foo ();
+ }
+ return x;
+}
+
+constexpr int
+waldo (int x)
+{
+ for (int a = 1; ; a++)
+ {
+ if (x > 10)
+ break;
+ else
+ return 5;
+ foo ();
+ }
+ foo ();
+ return x;
+}
+
+constexpr int i = bar ();
+constexpr int j = baz ();
+constexpr int k = qux (4);
+constexpr int l = corge (5);
+constexpr int m = garply (2);
+constexpr int n = waldo (-2);
+static_assert (i == 0 && j == 3 && k == 7 && l == 4 && m == 2 && n == 5, "");
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++, v2: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672]
2021-01-19 21:54 ` [PATCH] c++, v2: " Jakub Jelinek
@ 2021-01-20 7:31 ` Jakub Jelinek
2021-01-21 15:27 ` Jason Merrill
1 sibling, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2021-01-20 7:31 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
On Tue, Jan 19, 2021 at 10:54:08PM +0100, Jakub Jelinek via Gcc-patches wrote:
> So like this then if it passes bootstrap/regtest?
Successfully bootstrapped/regtested on x86_64-linux and i686-linux.
>
> 2021-01-19 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/98672
> * constexpr.c (check_for_return_continue_data): Add break_stmt member.
> (check_for_return_continue): Also look for BREAK_STMT. Handle SWITCH_STMT
> by ignoring break_stmt from its body.
> (potential_constant_expression_1) <case FOR_STMT>,
> <case WHILE_STMT>: If the condition isn't constant true, check if
> the loop body can contain a return stmt.
> <case SWITCH_STMT>: Adjust check_for_return_continue_data initializer.
> <case IF_STMT>: If recursion with tf_none is successful, merge
> *jump_target from the branches - returns with highest priority, breaks
> or continues lower. If then branch is potentially constant and
> doesn't return, check the else branch if it could return, break or
> continue.
>
> * g++.dg/cpp1y/constexpr-98672.C: New test.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++, v2: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672]
2021-01-19 21:54 ` [PATCH] c++, v2: " Jakub Jelinek
2021-01-20 7:31 ` Jakub Jelinek
@ 2021-01-21 15:27 ` Jason Merrill
1 sibling, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2021-01-21 15:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/19/21 4:54 PM, Jakub Jelinek wrote:
> On Tue, Jan 19, 2021 at 04:01:49PM -0500, Jason Merrill wrote:
>> Hmm, IF_STMT probably also needs to check the else clause, if the condition
>> isn't a known constant.
>
> You're right, I thought it was ok because it recurses with tf_none, but
> if the then branch is potentially constant and only else returns, continues
> or breaks, then as the enhanced testcase shows we were mishandling it too.
>
> So like this then if it passes bootstrap/regtest?
OK.
> 2021-01-19 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/98672
> * constexpr.c (check_for_return_continue_data): Add break_stmt member.
> (check_for_return_continue): Also look for BREAK_STMT. Handle SWITCH_STMT
> by ignoring break_stmt from its body.
> (potential_constant_expression_1) <case FOR_STMT>,
> <case WHILE_STMT>: If the condition isn't constant true, check if
> the loop body can contain a return stmt.
> <case SWITCH_STMT>: Adjust check_for_return_continue_data initializer.
> <case IF_STMT>: If recursion with tf_none is successful, merge
> *jump_target from the branches - returns with highest priority, breaks
> or continues lower. If then branch is potentially constant and
> doesn't return, check the else branch if it could return, break or
> continue.
>
> * g++.dg/cpp1y/constexpr-98672.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2021-01-14 12:49:50.500644142 +0100
> +++ gcc/cp/constexpr.c 2021-01-19 22:44:17.845322567 +0100
> @@ -7649,15 +7649,16 @@ check_automatic_or_tls (tree ref)
> struct check_for_return_continue_data {
> hash_set<tree> *pset;
> tree continue_stmt;
> + tree break_stmt;
> };
>
> /* Helper function for potential_constant_expression_1 SWITCH_STMT handling,
> called through cp_walk_tree. Return the first RETURN_EXPR found, or note
> - the first CONTINUE_STMT if RETURN_EXPR is not found. */
> + the first CONTINUE_STMT and/or BREAK_STMT if RETURN_EXPR is not found. */
> static tree
> check_for_return_continue (tree *tp, int *walk_subtrees, void *data)
> {
> - tree t = *tp, s;
> + tree t = *tp, s, b;
> check_for_return_continue_data *d = (check_for_return_continue_data *) data;
> switch (TREE_CODE (t))
> {
> @@ -7669,6 +7670,11 @@ check_for_return_continue (tree *tp, int
> d->continue_stmt = t;
> break;
>
> + case BREAK_STMT:
> + if (d->break_stmt == NULL_TREE)
> + d->break_stmt = t;
> + break;
> +
> #define RECUR(x) \
> if (tree r = cp_walk_tree (&x, check_for_return_continue, data, \
> d->pset)) \
> @@ -7680,16 +7686,20 @@ check_for_return_continue (tree *tp, int
> *walk_subtrees = 0;
> RECUR (DO_COND (t));
> s = d->continue_stmt;
> + b = d->break_stmt;
> RECUR (DO_BODY (t));
> d->continue_stmt = s;
> + d->break_stmt = b;
> break;
>
> case WHILE_STMT:
> *walk_subtrees = 0;
> RECUR (WHILE_COND (t));
> s = d->continue_stmt;
> + b = d->break_stmt;
> RECUR (WHILE_BODY (t));
> d->continue_stmt = s;
> + d->break_stmt = b;
> break;
>
> case FOR_STMT:
> @@ -7698,16 +7708,28 @@ check_for_return_continue (tree *tp, int
> RECUR (FOR_COND (t));
> RECUR (FOR_EXPR (t));
> s = d->continue_stmt;
> + b = d->break_stmt;
> RECUR (FOR_BODY (t));
> d->continue_stmt = s;
> + d->break_stmt = b;
> break;
>
> case RANGE_FOR_STMT:
> *walk_subtrees = 0;
> RECUR (RANGE_FOR_EXPR (t));
> s = d->continue_stmt;
> + b = d->break_stmt;
> RECUR (RANGE_FOR_BODY (t));
> d->continue_stmt = s;
> + d->break_stmt = b;
> + break;
> +
> + case SWITCH_STMT:
> + *walk_subtrees = 0;
> + RECUR (SWITCH_STMT_COND (t));
> + b = d->break_stmt;
> + RECUR (SWITCH_STMT_BODY (t));
> + d->break_stmt = b;
> break;
> #undef RECUR
>
> @@ -8190,7 +8212,18 @@ potential_constant_expression_1 (tree t,
> /* If we couldn't evaluate the condition, it might not ever be
> true. */
> if (!integer_onep (tmp))
> - return true;
> + {
> + /* Before returning true, check if the for body can contain
> + a return. */
> + hash_set<tree> pset;
> + check_for_return_continue_data data = { &pset, NULL_TREE,
> + NULL_TREE };
> + if (tree ret_expr
> + = cp_walk_tree (&FOR_BODY (t), check_for_return_continue,
> + &data, &pset))
> + *jump_target = ret_expr;
> + return true;
> + }
> }
> if (!RECUR (FOR_EXPR (t), any))
> return false;
> @@ -8219,7 +8252,18 @@ potential_constant_expression_1 (tree t,
> tmp = cxx_eval_outermost_constant_expr (tmp, true);
> /* If we couldn't evaluate the condition, it might not ever be true. */
> if (!integer_onep (tmp))
> - return true;
> + {
> + /* Before returning true, check if the while body can contain
> + a return. */
> + hash_set<tree> pset;
> + check_for_return_continue_data data = { &pset, NULL_TREE,
> + NULL_TREE };
> + if (tree ret_expr
> + = cp_walk_tree (&WHILE_BODY (t), check_for_return_continue,
> + &data, &pset))
> + *jump_target = ret_expr;
> + return true;
> + }
> if (!RECUR (WHILE_BODY (t), any))
> return false;
> if (breaks (jump_target) || continues (jump_target))
> @@ -8238,7 +8282,8 @@ potential_constant_expression_1 (tree t,
> else
> {
> hash_set<tree> pset;
> - check_for_return_continue_data data = { &pset, NULL_TREE };
> + check_for_return_continue_data data = { &pset, NULL_TREE,
> + NULL_TREE };
> if (tree ret_expr
> = cp_walk_tree (&SWITCH_STMT_BODY (t), check_for_return_continue,
> &data, &pset))
> @@ -8648,11 +8693,46 @@ potential_constant_expression_1 (tree t,
> return RECUR (TREE_OPERAND (t, 2), want_rval);
> else if (TREE_CODE (tmp) == INTEGER_CST)
> return RECUR (TREE_OPERAND (t, 1), want_rval);
> + tmp = *jump_target;
> for (i = 1; i < 3; ++i)
> - if (potential_constant_expression_1 (TREE_OPERAND (t, i),
> - want_rval, strict, now,
> - tf_none, jump_target))
> - return true;
> + {
> + tree this_jump_target = tmp;
> + if (potential_constant_expression_1 (TREE_OPERAND (t, i),
> + want_rval, strict, now,
> + tf_none, &this_jump_target))
> + {
> + if (returns (&this_jump_target))
> + *jump_target = this_jump_target;
> + else if (!returns (jump_target))
> + {
> + if (breaks (&this_jump_target)
> + || continues (&this_jump_target))
> + *jump_target = this_jump_target;
> + if (i == 1)
> + {
> + /* If the then branch is potentially constant, but
> + does not return, check if the else branch
> + couldn't return, break or continue. */
> + hash_set<tree> pset;
> + check_for_return_continue_data data = { &pset, NULL_TREE,
> + NULL_TREE };
> + if (tree ret_expr
> + = cp_walk_tree (&TREE_OPERAND (t, 2),
> + check_for_return_continue, &data,
> + &pset))
> + *jump_target = ret_expr;
> + else if (*jump_target == NULL_TREE)
> + {
> + if (data.continue_stmt)
> + *jump_target = data.continue_stmt;
> + else if (data.break_stmt)
> + *jump_target = data.break_stmt;
> + }
> + }
> + }
> + return true;
> + }
> + }
> if (flags & tf_error)
> error_at (loc, "expression %qE is not a constant expression", t);
> return false;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C.jj 2021-01-19 22:13:17.031454887 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C 2021-01-19 22:46:48.989605151 +0100
> @@ -0,0 +1,92 @@
> +// PR c++/98672
> +// { dg-do compile { target c++14 } }
> +
> +void
> +foo ()
> +{
> +}
> +
> +constexpr int
> +bar ()
> +{
> + for (int i = 0; i < 5; ++i)
> + return i;
> + foo ();
> + return 0;
> +}
> +
> +constexpr int
> +baz ()
> +{
> + int i = 0;
> + while (i < 5)
> + {
> + if (i == 3)
> + return i;
> + else
> + ++i;
> + }
> + foo ();
> + return 0;
> +}
> +
> +constexpr int
> +qux (int x)
> +{
> + if (x > 10)
> + ++x;
> + else
> + return 7;
> + foo ();
> + return 0;
> +}
> +
> +constexpr int
> +corge (int x)
> +{
> + for (int a = 1; ; a++)
> + {
> + if (x > 10)
> + ++x;
> + else
> + return 4;
> + foo ();
> + }
> +}
> +
> +constexpr int
> +garply (int x)
> +{
> + for (int a = 1; ; a++)
> + {
> + if (x > 10)
> + ++x;
> + else
> + break;
> + foo ();
> + }
> + return x;
> +}
> +
> +constexpr int
> +waldo (int x)
> +{
> + for (int a = 1; ; a++)
> + {
> + if (x > 10)
> + break;
> + else
> + return 5;
> + foo ();
> + }
> + foo ();
> + return x;
> +}
> +
> +constexpr int i = bar ();
> +constexpr int j = baz ();
> +constexpr int k = qux (4);
> +constexpr int l = corge (5);
> +constexpr int m = garply (2);
> +constexpr int n = waldo (-2);
> +static_assert (i == 0 && j == 3 && k == 7 && l == 4 && m == 2 && n == 5, "");
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-21 15:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15 16:26 [PATCH] c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672] Jakub Jelinek
2021-01-19 21:01 ` Jason Merrill
2021-01-19 21:54 ` [PATCH] c++, v2: " Jakub Jelinek
2021-01-20 7:31 ` Jakub Jelinek
2021-01-21 15:27 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).