From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2153) id 65610386F472; Fri, 29 Jan 2021 19:19:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 65610386F472 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jakub Jelinek To: gcc-cvs@gcc.gnu.org Subject: [gcc r10-9315] c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672] X-Act-Checkin: gcc X-Git-Author: Jakub Jelinek X-Git-Refname: refs/heads/releases/gcc-10 X-Git-Oldrev: 2127d2c3ee23bbd03f02d88fd82403408696ee4a X-Git-Newrev: 8182cbe3fb2c2d20e8dff9d2476fb94046e560b3 Message-Id: <20210129191929.65610386F472@sourceware.org> Date: Fri, 29 Jan 2021 19:19:29 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jan 2021 19:19:29 -0000 https://gcc.gnu.org/g:8182cbe3fb2c2d20e8dff9d2476fb94046e560b3 commit r10-9315-g8182cbe3fb2c2d20e8dff9d2476fb94046e560b3 Author: Jakub Jelinek Date: Thu Jan 21 17:20:24 2021 +0100 c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672] 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. 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. > 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. 2021-01-21 Jakub Jelinek 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) , : If the condition isn't constant true, check if the loop body can contain a return stmt. : Adjust check_for_return_continue_data initializer. : 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. Diff: --- gcc/cp/constexpr.c | 98 +++++++++++++++++++++++++--- gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C | 92 ++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 9 deletions(-) diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index ea432cc61bd..0382f347445 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -7261,15 +7261,16 @@ check_automatic_or_tls (tree ref) struct check_for_return_continue_data { hash_set *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)) { @@ -7281,6 +7282,11 @@ check_for_return_continue (tree *tp, int *walk_subtrees, void *data) 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)) \ @@ -7292,16 +7298,20 @@ check_for_return_continue (tree *tp, int *walk_subtrees, void *data) *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: @@ -7310,16 +7320,28 @@ check_for_return_continue (tree *tp, int *walk_subtrees, void *data) 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 @@ -7797,7 +7819,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, /* 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 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; @@ -7826,7 +7859,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, 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 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)) @@ -7845,7 +7889,8 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, else { hash_set 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)) @@ -8257,11 +8302,46 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now, 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 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; diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C new file mode 100644 index 00000000000..06f7e42ed24 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-98672.C @@ -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, "");