On Sat, Jun 6, 2026 at 6:31 PM Avi Kivity <[email protected]> wrote:
> From: Avi Kivity <[email protected]> > > The CWG 2867 implementation introduced conditional cleanup guards for > structured binding base variables, using internal TARGET_EXPRs. These > guard variables are not part of any BIND_EXPR_VARS list, so the > coroutine frame promotion in register_local_var_uses missed them. > After a suspend/resume cycle, the guard's stack location is dead, > causing the conditional destructor to be skipped. > > Fix this by introducing TARGET_EXPR_DECOMP_GUARD_P to label decomposition > cleanup guard TARGET_EXPRs at creation time in cp_finish_decl, then > extending both register_local_var_uses and transform_local_var_uses to > detect them and promote their slots to the coroutine frame. > > Ping on this. Is everyone frightened by the A-word? > gcc/cp/ChangeLog: > > PR c++/124584 > * cp-tree.h (TARGET_EXPR_DECOMP_GUARD_P): New macro using > TREE_LANG_FLAG_5 on TARGET_EXPR. > * decl.cc (cp_finish_decl): Set TARGET_EXPR_DECOMP_GUARD_P on > cleanup guard TARGET_EXPRs for structured bindings. > * coroutines.cc (register_local_var_uses): Also register > TARGET_EXPR_SLOT of TARGET_EXPR_DECOMP_GUARD_P nodes for frame > promotion. > (transform_local_var_uses): Also transform > TARGET_EXPR_DECOMP_GUARD_P slots by setting DECL_VALUE_EXPR to > the frame access expression. > > gcc/testsuite/ChangeLog: > > PR c++/124584 > * g++.dg/coroutines/pr124584.C: New test. > --- > > This patch was prepared with extensive AI help (Claude Opus 4.6 medium). > I reviewed it to the best of my ability and I don't think it is AI slop. > Of course, my ability in this regard is limited as I don't deeply > understand gcc code. My contribution was to insist on labeling the > cleanup guard so we don't promote any internal expression to the coroutine > frame, just the structured binding guard. > > I hope that in the case the patch is not accepted, it at least inspires > someone to fix the bug, as it makes gcc useless with coroutines (as no > one[1] will agree to avoid structured bindings). > > [1] No one who has access to a compiler that does work with structured > bindings > > gcc/cp/coroutines.cc | 58 ++++++++++++++++++++ > gcc/cp/cp-tree.h | 8 +++ > gcc/cp/decl.cc | 2 + > gcc/testsuite/g++.dg/coroutines/pr124584.C | 61 ++++++++++++++++++++++ > 4 files changed, 129 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr124584.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 46ec8c0a953..55d9dbfca56 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2489,10 +2489,32 @@ struct local_vars_transform > static tree > transform_local_var_uses (tree *stmt, int *do_subtree, void *d) > { > local_vars_transform *lvd = (local_vars_transform *) d; > > + /* PR c++/124584: Transform internal TARGET_EXPR slots that were > promoted > + to the coroutine frame (e.g. cleanup guard variables for structured > + bindings). Set DECL_VALUE_EXPR so all uses resolve to the frame. */ > + if (TREE_CODE (*stmt) == TARGET_EXPR > + && TARGET_EXPR_DECOMP_GUARD_P (*stmt)) > + { > + tree slot = TARGET_EXPR_SLOT (*stmt); > + local_var_info *local_var = lvd->local_var_uses->get (slot); > + if (local_var && local_var->field_id) > + { > + DECL_CONTEXT (slot) = lvd->context; > + tree fld_idx > + = coro_build_frame_access_expr (lvd->actor_frame, > + local_var->field_id, true, > + tf_warning_or_error); > + local_var->field_idx = fld_idx; > + SET_DECL_VALUE_EXPR (slot, fld_idx); > + DECL_HAS_VALUE_EXPR_P (slot) = true; > + } > + return NULL_TREE; > + } > + > /* For each var in this bind expr (that has a frame id, which means it > was > accessed), build a frame reference and add it as the > DECL_VALUE_EXPR. */ > > if (TREE_CODE (*stmt) == BIND_EXPR) > { > @@ -4339,10 +4361,46 @@ coro_make_frame_entry (tree *field_list, const > char *name, tree fld_type, > need it, so that they can be 'promoted' across suspension points. */ > > static tree > register_local_var_uses (tree *stmt, int *do_subtree, void *d) > { > + /* PR c++/124584: Internal TARGET_EXPRs (such as cleanup guard variables > + created for structured bindings by CWG 2867) need to be promoted to > the > + coroutine frame so their values survive across suspension points. > + Without this, the conditional cleanup guard is lost after resume and > + the destructor of the binding base variable is never called. */ > + if (TREE_CODE (*stmt) == TARGET_EXPR > + && TARGET_EXPR_DECOMP_GUARD_P (*stmt)) > + { > + local_vars_frame_data *lvd = (local_vars_frame_data *) d; > + tree slot = TARGET_EXPR_SLOT (*stmt); > + if (TREE_CODE (slot) == VAR_DECL > + && !TREE_STATIC (slot) > + && !lvd->local_var_uses->get (slot)) > + { > + bool existed; > + local_var_info &local_var > + = lvd->local_var_uses->get_or_insert (slot, &existed); > + gcc_checking_assert (!existed); > + local_var.def_loc = DECL_SOURCE_LOCATION (slot); > + tree lvtype = TREE_TYPE (slot); > + local_var.frame_type = lvtype; > + local_var.field_idx = local_var.field_id = NULL_TREE; > + local_var.is_static = false; > + local_var.is_lambda_capture = false; > + local_var.has_value_expr_p = false; > + > + lvd->local_var_seen = true; > + unsigned serial = lvd->bind_indx; > + char *buf = xasprintf ("_Guard%u_%u", lvd->nest_depth, serial); > + local_var.field_id > + = coro_make_frame_entry (lvd->field_list, buf, lvtype, > lvd->loc); > + free (buf); > + } > + return NULL_TREE; > + } > + > if (TREE_CODE (*stmt) != BIND_EXPR) > return NULL_TREE; > > local_vars_frame_data *lvd = (local_vars_frame_data *) d; > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 8807d80a21a..58385e3a577 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -540,10 +540,11 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; > FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE) > CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR) > CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR) > OVL_EXPORT_P (in OVERLOAD) > DECL_NTTP_OBJECT_P (in VAR_DECL) > + TARGET_EXPR_DECOMP_GUARD_P (in TARGET_EXPR) > 6: TYPE_MARKED_P (in _TYPE) > DECL_NONTRIVIALLY_INITIALIZED_P (in VAR_DECL) > RANGE_FOR_IVDEP (in RANGE_FOR_STMT) > CALL_EXPR_OPERATOR_SYNTAX (in CALL_EXPR, AGGR_INIT_EXPR) > CONSTRUCTOR_IS_DESIGNATED_INIT (in CONSTRUCTOR) > @@ -6031,10 +6032,17 @@ decl_template_parm_check (const_tree t, const char > *f, int l, const char *fn) > /* True if this TARGET_EXPR is for holding an implementation detail like a > cleanup flag or loop index, and should be ignored by > extend_all_temps. */ > #define TARGET_EXPR_INTERNAL_P(NODE) \ > TREE_LANG_FLAG_4 (TARGET_EXPR_CHECK (NODE)) > > +/* True if NODE is an internal TARGET_EXPR used as a cleanup guard > variable > + for structured binding (decomposition) conditional destruction. Such > + guards need to be promoted to the coroutine frame so their values > survive > + across suspension points. */ > +#define TARGET_EXPR_DECOMP_GUARD_P(NODE) \ > + TREE_LANG_FLAG_5 (TARGET_EXPR_CHECK (NODE)) > + > /* True if NODE is a TARGET_EXPR that just expresses a copy of its > INITIAL; if > the initializer has void type, it's doing something more complicated. > */ > #define SIMPLE_TARGET_EXPR_P(NODE) \ > (TREE_CODE (NODE) == TARGET_EXPR \ > && TARGET_EXPR_INITIAL (NODE) \ > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 84aa9a72fab..03af6d52fb5 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -10118,10 +10118,11 @@ cp_finish_decl (tree decl, tree init, bool > init_const_expr_p, > emitted code. */ > tree guard = NULL_TREE; > if (cleanups || cleanup) > { > guard = get_internal_target_expr (boolean_false_node); > + TARGET_EXPR_DECOMP_GUARD_P (guard) = true; > add_stmt (guard); > guard = TARGET_EXPR_SLOT (guard); > } > tree sl = push_stmt_list (); > initialize_local_var (decl, init, true); > @@ -10147,10 +10148,11 @@ cp_finish_decl (tree decl, tree init, bool > init_const_expr_p, > extend_ref_init_temps created vars, pop_stmt_list > popped that all, so push those extra cleanups around > the whole sequence with a guard variable. */ > gcc_assert (TREE_CODE (sl) == STATEMENT_LIST); > guard = get_internal_target_expr (integer_zero_node); > + TARGET_EXPR_DECOMP_GUARD_P (guard) = true; > add_stmt (guard); > guard = TARGET_EXPR_SLOT (guard); > for (unsigned i = 0; i < n_extra_cleanups; ++i) > { > tree_stmt_iterator tsi = tsi_last (sl); > diff --git a/gcc/testsuite/g++.dg/coroutines/pr124584.C > b/gcc/testsuite/g++.dg/coroutines/pr124584.C > new file mode 100644 > index 00000000000..116dc8aed88 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/coroutines/pr124584.C > @@ -0,0 +1,61 @@ > +// PR c++/124584 > +// { dg-do run { target c++20 } } > +// { dg-additional-options "-O1" } > +// Verify that the destructor of a structured binding base variable > +// (using the tuple protocol) is called after a coroutine resumes, > +// when the binding's scope spans a suspension point. > + > +#include <coroutine> > +#include <tuple> > +#include <utility> > + > +static int g_dtor_count = 0; > + > +struct guard { > + bool live_ = true; > + ~guard () { if (live_) ++g_dtor_count; } > + guard () = default; > + guard (guard&& o) noexcept : live_(std::exchange (o.live_, false)) {} > +}; > + > +static std::coroutine_handle<> g_pending; > + > +struct yield_aw { > + bool await_ready () const noexcept { return false; } > + void await_suspend (std::coroutine_handle<> h) noexcept { g_pending = > h; } > + void await_resume () const noexcept {} > +}; > + > +struct task { > + struct promise_type { > + task get_return_object () noexcept > + { return {std::coroutine_handle<promise_type>::from_promise (*this)}; > } > + std::suspend_never initial_suspend () noexcept { return {}; } > + std::suspend_always final_suspend () noexcept { return {}; } > + void return_void () noexcept {} > + void unhandled_exception () { __builtin_abort (); } > + }; > + std::coroutine_handle<promise_type> h_; > + ~task () { if (h_) h_.destroy (); } > +}; > + > +std::tuple<guard> make_guarded_tuple () { return {guard{}}; } > + > +task > +test_fn () > +{ > + { > + auto [g] = make_guarded_tuple (); > + co_await yield_aw{}; > + } // guard should be destroyed here > +} > + > +int > +main () > +{ > + auto t = test_fn (); > + std::exchange (g_pending, nullptr).resume (); > + > + if (g_dtor_count != 1) > + __builtin_abort (); > +} > -- > 2.54.0 > >
