Message ID | 20210115162654.GI1034503@tucnak |
---|---|
State | New |
Headers | show |
Series | c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling [PR98672] | expand |
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 >
--- 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, "");