From 0d2a5f3cb715fd95f1fa4a13b5d67c7eea28f178 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Mon, 10 Feb 2025 15:44:13 +0100 Subject: [PATCH] c++: change implementation of -frange-for-ext-temps [PR118574] The implementation in r15-3840 used a novel technique of wrapping the entire range-for loop in a CLEANUP_POINT_EXPR, which confused the coroutines transformation. Instead let's use the existing extend_ref_init_temps mechanism. This does not revert all of r15-3840, only the parts that change how CLEANUP_POINT_EXPRs are applied to range-for declarations. PR c++/118574 PR c++/107637 gcc/cp/ChangeLog: * call.cc (struct extend_temps_data): New. (extend_temps_r, extend_all_temps): New. (set_up_extended_ref_temp): Handle tree walk case. (extend_ref_init_temps): Cal extend_all_temps. * decl.cc (initialize_local_var): Revert ext-temps change. * parser.cc (cp_convert_range_for): Likewise. (cp_parser_omp_loop_nest): Likewise. * pt.cc (tsubst_stmt): Likewise. * semantics.cc (finish_for_stmt): Likewise. gcc/testsuite/ChangeLog: * g++.dg/coroutines/range-for1.C: New test. --- gcc/cp/call.cc | 117 +++++++++++++++++-- gcc/cp/decl.cc | 5 - gcc/cp/parser.cc | 23 +--- gcc/cp/pt.cc | 22 ---- gcc/cp/semantics.cc | 13 --- gcc/testsuite/g++.dg/coroutines/range-for1.C | 69 +++++++++++ 6 files changed, 180 insertions(+), 69 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/range-for1.C diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index e440d58141b..2c77b4a4b68 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -14154,6 +14154,20 @@ make_temporary_var_for_ref_to_temp (tree decl, tree type) return pushdecl (var); } +/* Data for extend_temps_r, mostly matching the parameters of + extend_ref_init_temps. */ + +struct extend_temps_data +{ + tree decl; + tree init; + vec **cleanups; + tree* cond_guard; + hash_set *pset; +}; + +static tree extend_temps_r (tree *, int *, void *); + /* EXPR is the initializer for a variable DECL of reference or std::initializer_list type. Create, push and return a new VAR_DECL for the initializer so that it will live as long as DECL. Any @@ -14162,7 +14176,8 @@ make_temporary_var_for_ref_to_temp (tree decl, tree type) static tree set_up_extended_ref_temp (tree decl, tree expr, vec **cleanups, - tree *initp, tree *cond_guard) + tree *initp, tree *cond_guard, + extend_temps_data *walk_data) { tree init; tree type; @@ -14198,10 +14213,16 @@ set_up_extended_ref_temp (tree decl, tree expr, vec **cleanups, suppress_warning (decl); } - /* Recursively extend temps in this initializer. */ - TARGET_EXPR_INITIAL (expr) - = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups, - cond_guard); + /* Recursively extend temps in this initializer. The recursion needs to come + after creating the variable to conform to the mangling ABI, and before + maybe_constant_init because the extension might change its result. */ + if (walk_data) + cp_walk_tree (&TARGET_EXPR_INITIAL (expr), extend_temps_r, + walk_data, walk_data->pset); + else + TARGET_EXPR_INITIAL (expr) + = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups, + cond_guard); /* Any reference temp has a non-trivial initializer. */ DECL_NONTRIVIALLY_INITIALIZED_P (var) = true; @@ -14801,7 +14822,8 @@ extend_ref_init_temps_1 (tree decl, tree init, vec **cleanups, if (TREE_CODE (*p) == TARGET_EXPR) { tree subinit = NULL_TREE; - *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit, cond_guard); + *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit, + cond_guard, nullptr); recompute_tree_invariant_for_addr_expr (sub); if (init != sub) init = fold_convert (TREE_TYPE (init), sub); @@ -14811,6 +14833,81 @@ extend_ref_init_temps_1 (tree decl, tree init, vec **cleanups, return init; } +/* Tree walk function for extend_all_temps. Generally parallel to + extend_ref_init_temps_1, but adapted for walk_tree. */ + +tree +extend_temps_r (tree *tp, int *walk_subtrees, void *data) +{ + extend_temps_data *d = (extend_temps_data *)data; + + if (TYPE_P (*tp) || TREE_CODE (*tp) == CLEANUP_POINT_EXPR) + { + *walk_subtrees = 0; + return NULL_TREE; + } + + if (TREE_CODE (*tp) == COND_EXPR) + { + cp_walk_tree (&TREE_OPERAND (*tp, 0), extend_temps_r, d, d->pset); + + auto walk_arm = [d](tree &op) + { + tree cur_cond_guard = NULL_TREE; + auto ov = make_temp_override (d->cond_guard, &cur_cond_guard); + cp_walk_tree (&op, extend_temps_r, d, d->pset); + if (cur_cond_guard) + { + tree set = build2 (MODIFY_EXPR, boolean_type_node, + cur_cond_guard, boolean_true_node); + op = add_stmt_to_compound (set, op); + } + }; + walk_arm (TREE_OPERAND (*tp, 1)); + walk_arm (TREE_OPERAND (*tp, 2)); + + *walk_subtrees = 0; + return NULL_TREE; + } + + if (TREE_CODE (*tp) == ADDR_EXPR + /* A discarded-value temporary. */ + || (TREE_CODE (*tp) == CONVERT_EXPR + && VOID_TYPE_P (TREE_TYPE (*tp)))) + { + tree *p; + for (p = &TREE_OPERAND (*tp, 0); + TREE_CODE (*p) == COMPONENT_REF || TREE_CODE (*p) == ARRAY_REF; ) + p = &TREE_OPERAND (*p, 0); + if (TREE_CODE (*p) == TARGET_EXPR) + { + tree subinit = NULL_TREE; + *p = set_up_extended_ref_temp (d->decl, *p, d->cleanups, &subinit, + d->cond_guard, d); + if (TREE_CODE (*tp) == ADDR_EXPR) + recompute_tree_invariant_for_addr_expr (*tp); + if (subinit) + *tp = cp_build_compound_expr (subinit, *tp, tf_none); + } + } + + /* TARGET_EXPRs that aren't handled by the above are implementation details + that shouldn't be ref-extended, like the build_vec_init iterator. */ + + return NULL_TREE; +} + +/* Extend all the temporaries in a for-range-initializer. */ + +static tree +extend_all_temps (tree decl, tree init, vec **cleanups) +{ + hash_set pset; + extend_temps_data d = { decl, init, cleanups, nullptr, &pset }; + cp_walk_tree (&init, extend_temps_r, &d, &pset); + return init; +} + /* INIT is part of the initializer for DECL. If there are any reference or initializer lists being initialized, extend their lifetime to match that of DECL. */ @@ -14823,11 +14920,13 @@ extend_ref_init_temps (tree decl, tree init, vec **cleanups, if (processing_template_decl) return init; - /* P2718R0 - ignore temporaries in C++23 for-range-initializer, those - have all extended lifetime. */ + /* P2718R0 - in C++23 for-range-initializer, extend all temps. */ if (DECL_NAME (decl) == for_range__identifier && flag_range_for_ext_temps) - return init; + { + gcc_checking_assert (!cond_guard); + return extend_all_temps (decl, init, cleanups); + } maybe_warn_dangling_reference (decl, init); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 552a7a2ec54..7f7f4938f2c 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -8310,11 +8310,6 @@ initialize_local_var (tree decl, tree init, bool decomp) code emitted by cp_finish_decomp. */ if (decomp) current_stmt_tree ()->stmts_are_full_exprs_p = 0; - /* P2718R0 - avoid CLEANUP_POINT_EXPR for range-for-initializer, - temporaries from there should have lifetime extended. */ - else if (DECL_NAME (decl) == for_range__identifier - && flag_range_for_ext_temps) - current_stmt_tree ()->stmts_are_full_exprs_p = 0; else current_stmt_tree ()->stmts_are_full_exprs_p = 1; finish_expr_stmt (init); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 9d0970b4d83..8284d659787 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -14844,15 +14844,6 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr, { range_temp = build_range_temp (range_expr); pushdecl (range_temp); - if (flag_range_for_ext_temps) - { - /* P2718R0 - put the range_temp declaration and everything - until end of the range for body into an extra STATEMENT_LIST - which will have CLEANUP_POINT_EXPR around it, so that all - temporaries are destroyed at the end of it. */ - gcc_assert (FOR_INIT_STMT (statement) == NULL_TREE); - FOR_INIT_STMT (statement) = push_stmt_list (); - } cp_finish_decl (range_temp, range_expr, /*is_constant_init*/false, NULL_TREE, LOOKUP_ONLYCONVERTING); @@ -46719,20 +46710,12 @@ cp_parser_omp_loop_nest (cp_parser *parser, bool *if_p) /* Pop and remember the init block. */ if (sl) - { - sl = pop_stmt_list (sl); - /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in - for-range-initializer whose lifetime is extended are destructed - here. */ - if (flag_range_for_ext_temps - && is_range_for - && !processing_template_decl) - sl = maybe_cleanup_point_expr_void (sl); - add_stmt (sl); - } + add_stmt (pop_stmt_list (sl)); + tree range_for_decl[3] = { NULL_TREE, NULL_TREE, NULL_TREE }; if (is_range_for && !processing_template_decl) find_range_for_decls (range_for_decl); + finish_compound_stmt (init_scope); init_block = pop_stmt_list (init_block); omp_for_parse_state->init_blockv[depth] = init_block; diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index f857b3f1180..a2fc8813e9d 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -19478,18 +19478,6 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl) RECUR (OMP_FOR_PRE_BODY (t)); pre_body = pop_stmt_list (pre_body); - tree sl = NULL_TREE; - if (flag_range_for_ext_temps - && OMP_FOR_INIT (t) != NULL_TREE - && !processing_template_decl) - for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (t)); i++) - if (TREE_VEC_ELT (OMP_FOR_INIT (t), i) - && TREE_VEC_ELT (OMP_FOR_COND (t), i) == global_namespace) - { - sl = push_stmt_list (); - break; - } - if (OMP_FOR_INIT (t) != NULL_TREE) for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (t)); i++) { @@ -19545,16 +19533,6 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t complain, tree in_decl) add_stmt (t); } - if (sl) - { - /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in - for-range-initializer whose lifetime is extended are destructed - here. */ - sl = pop_stmt_list (sl); - sl = maybe_cleanup_point_expr_void (sl); - add_stmt (sl); - } - add_stmt (finish_omp_for_block (finish_omp_structured_block (stmt), t)); pop_omp_privatization_clauses (r); diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index a2ee3a34caa..8a2d86576fb 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -1736,19 +1736,6 @@ finish_for_stmt (tree for_stmt) tree range_for_decl[3] = { NULL_TREE, NULL_TREE, NULL_TREE }; find_range_for_decls (range_for_decl); - /* P2718R0 - Add CLEANUP_POINT_EXPR so that temporaries in - for-range-initializer whose lifetime is extended are destructed - here. */ - if (flag_range_for_ext_temps - && range_for_decl[0] - && FOR_INIT_STMT (for_stmt)) - { - tree stmt = pop_stmt_list (FOR_INIT_STMT (for_stmt)); - FOR_INIT_STMT (for_stmt) = NULL_TREE; - stmt = maybe_cleanup_point_expr_void (stmt); - add_stmt (stmt); - } - add_stmt (do_poplevel (scope)); /* If we're being called from build_vec_init, don't mess with the names of diff --git a/gcc/testsuite/g++.dg/coroutines/range-for1.C b/gcc/testsuite/g++.dg/coroutines/range-for1.C new file mode 100644 index 00000000000..8b4f830b452 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/range-for1.C @@ -0,0 +1,69 @@ +// PR c++/118574 +// { dg-do run { target c++20 } } +// { dg-additional-options -frange-for-ext-temps } + +#include + +struct A { + const char **a = nullptr; + int n = 0; + void push_back (const char *x) { if (!a) a = new const char *[2]; a[n++] = x; } + const char **begin () const { return a; } + const char **end () const { return a + n; } + ~A () { delete[] a; } +}; + +struct B { + long ns; + bool await_ready () const noexcept { return false; } + void await_suspend (std::coroutine_handle<> h) const noexcept { + volatile int v = 0; + while (v < ns) + v = v + 1; + h.resume (); + } + void await_resume () const noexcept {} +}; + +struct C { + struct promise_type { + const char *value; + std::suspend_never initial_suspend () { return {}; } + std::suspend_always final_suspend () noexcept { return {}; } + void return_value (const char *v) { value = v; } + void unhandled_exception () { __builtin_abort (); } + C get_return_object () { return C{this}; } + }; + promise_type *p; + explicit C (promise_type *p) : p(p) {} + const char *get () { return p->value; } +}; + +A +foo () +{ + A a; + a.push_back ("foo"); + a.push_back ("bar"); + return a; +} + +C +bar () +{ + A ret; + for (const auto &item : foo ()) + { + co_await B{200000}; + ret.push_back (item); + } + co_return "foobar"; +} + +int +main () +{ + auto task = bar (); + if (__builtin_strcmp (task.get (), "foobar")) + __builtin_abort (); +}