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.
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
