On Thu, Jun 10, 2021 at 10:09:26AM -0400, Jason Merrill wrote: > > --- gcc/cp/parser.c.jj 2021-06-09 21:54:39.482193853 +0200 > > +++ gcc/cp/parser.c 2021-06-10 10:09:23.753052980 +0200 > > @@ -10902,6 +10902,11 @@ cp_parser_lambda_expression (cp_parser* > > bool discarded = in_discarded_stmt; > > in_discarded_stmt = 0; > > + /* Similarly the body of a lambda in immediate function context is not > > + in immediate function context. */ > > + bool immediate_fn_ctx_p = in_immediate_fn_ctx_p; > > It's hard to distinguish between these two names when reading the code; > let's give the local variable a name including "saved".
Will do. > > + if (cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)) > > + error ("%<if consteval%> requires compound statement"); > > + > > + in_immediate_fn_ctx_p |= ce > 0; > > + cp_parser_implicitly_scoped_statement (parser, NULL, guard_tinfo); > > Maybe use cp_parser_compound_statement directly instead of this and checking > CPP_OPEN_BRACE above? Either way is fine. I thought doing it this way will provide better diagnostics for what I think can be a common bug - people so used to normal if not requiring compound statements forgetting those {}s from time to time. > > + if (TREE_CODE (t) == IF_STMT && IF_STMT_CONSTEVAL_P (t)) > > + { > > + /* Evaluate the condition as if it was > > + if (__builtin_is_constant_evaluated ()). */ > > + if (ctx->manifestly_const_eval) > > + val = boolean_true_node; > > + else > > + { > > + *non_constant_p = true; > > + return t; > > + } > > Why set *non_constant_p? Shouldn't this just be > > val = boolean_false_node; > > so we constant-evaluate the else clause when we are trying to > constant-evaluate in a non-manifestly-constant-evaluated context? It matches what we do for CP_BUILT_IN_IS_CONSTANT_EVALUATED calls: /* For __builtin_is_constant_evaluated, defer it if not ctx->manifestly_const_eval, otherwise fold it to true. */ if (fndecl_built_in_p (fun, CP_BUILT_IN_IS_CONSTANT_EVALUATED, BUILT_IN_FRONTEND)) { if (!ctx->manifestly_const_eval) { *non_constant_p = true; return t; } return boolean_true_node; } I believe we sometimes try to constexpr evaluate something without having manifestly_const_eval = true even on expressions that are in manifestly constant evaluated contexts and am worried if we just folded it to boolean_false_node we could get a constant expression and replace the expression with that, even before we actually try to constant evaluate it with manifestly_const_eval = true. If I do (for the CP_BUILT_IN_IS_CONSTANT_EVALUATED): --- gcc/cp/constexpr.c.jj 2021-06-10 15:27:31.123353594 +0200 +++ gcc/cp/constexpr.c 2021-06-10 16:26:38.368168281 +0200 @@ -1320,10 +1320,7 @@ cxx_eval_builtin_function_call (const co BUILT_IN_FRONTEND)) { if (!ctx->manifestly_const_eval) - { - *non_constant_p = true; - return t; - } + return boolean_false_node; return boolean_true_node; } then FAIL: g++.dg/cpp2a/is-constant-evaluated1.C -std=c++14 execution test FAIL: g++.dg/cpp2a/is-constant-evaluated1.C -std=c++17 execution test FAIL: g++.dg/cpp2a/is-constant-evaluated1.C -std=c++2a execution test FAIL: g++.dg/cpp2a/is-constant-evaluated1.C -std=c++17 -fconcepts execution test FAIL: g++.dg/cpp2a/is-constant-evaluated2.C -std=c++14 execution test FAIL: g++.dg/cpp2a/is-constant-evaluated2.C -std=c++17 execution test FAIL: g++.dg/cpp2a/is-constant-evaluated2.C -std=c++2a execution test FAIL: g++.dg/cpp2a/is-constant-evaluated2.C -std=c++17 -fconcepts execution test FAIL: g++.dg/cpp2a/is-constant-evaluated9.C -std=gnu++2a (test for excess errors) at least regresses. > > + if (TREE_CODE (t) != IF_STMT || !IF_STMT_CONSTEVAL_P (t)) > > + { > > + if (integer_zerop (tmp)) > > + return RECUR (TREE_OPERAND (t, 2), want_rval); > > + else if (TREE_CODE (tmp) == INTEGER_CST) > > + return RECUR (TREE_OPERAND (t, 1), want_rval); > > + } > > Don't we still want to shortcut consideration of one of the arms for if > consteval? potential_constant_expression_p{,_1} doesn't get passed whether it is manifestly_const_eval or not. > > --- gcc/cp/cp-gimplify.c.jj 2021-06-09 21:54:39.473193977 +0200 > > +++ gcc/cp/cp-gimplify.c 2021-06-10 09:49:35.898557178 +0200 > > @@ -161,7 +161,9 @@ genericize_if_stmt (tree *stmt_p) > > if (!else_) > > else_ = build_empty_stmt (locus); > > - if (integer_nonzerop (cond) && !TREE_SIDE_EFFECTS (else_)) > > + if (IF_STMT_CONSTEVAL_P (stmt)) > > + stmt = else_; > > This seems redundant, since you're using boolean_false_node for the > condition. It is only when !TREE_SIDE_EFFECTS (else_). I think that is about having labels in the then_/else_ blocks and gotos jumping into those from outside. But IF_STMT_CONSTEVAL_P guarantees that is not the case (and we verify earlier), so we can do that (and in fact, for IF_STMT_CONSTEVAL_P have to, because then_ could contain consteval calls not constant expression evaluated). I guess we could do that for IF_STMT_CONSTEXPR_P too, that also doesn't allow gotos/switch into the branches. > > --- gcc/cp/call.c.jj 2021-06-09 21:54:39.436194489 +0200 > > +++ gcc/cp/call.c 2021-06-10 09:49:35.949556470 +0200 > > @@ -8840,6 +8840,7 @@ immediate_invocation_p (tree fn, int nar > > || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)) > > && (current_binding_level->kind != sk_function_parms > > || !current_binding_level->immediate_fn_ctx_p) > > + && !in_immediate_fn_ctx_p > > Now that we have this flag, shouldn't we set it in actual immediate function > context? Or rename the flag to match when it's actually set. I guess I can try that. Though, I'm not sure if we could also get rid of the current_binding_level->immediate_fn_ctx_p for sk_function_parms case. Jakub