On Sat, Jan 10, 2026 at 11:32:17AM +0000, Iain Sandoe wrote:
> Hi Nathaniel.
>
> > On 10 Jan 2026, at 03:06, Nathaniel Shead <[email protected]> wrote:
> >
> > And here's a v2 of [1] that should avoid any potential ABI issues with
> > coroutines, by just special-casing the transform functions during
> > modules streaming in has_definition.
>
> Thanks; I think that this is the right mental model for the coroutine - it is
> a single entity that happens to be implemented in three pieces. Anything
> else is likely to open an ODR-flavoured can of worms.
>
> A couple of small additional points below, otherwise this looks good to me
> (but I cannot approve it),
> thanks
> Iain
>
> > [1]: https://gcc.gnu.org/pipermail/gcc-patches/2026-January/705304.html
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > -- >8 --
> >
> > While working on another issue I found that currently modules do not
> > work with coroutines at all. This patch fixes a number of issues in
> > both the coroutines logic and modules logic to ensure that they play
> > well together. To summarize:
> >
> > - The coroutine proxy objects did not have a DECL_CONTEXT set (required
> > for modules to merge declarations).
> >
> > - The coroutine transformation functions are always non-inline, even
> > for an inline ramp function, which means that modules need an override
> > to ensure the definitions are available where needed.
> >
> > - Coroutine transformation functions were not marked DECL_COROUTINE_P,
> > despite accessors implying that they were.
> >
> > - In an importing TU we had lost the connection between the ramp
> > functions and the transform functions, as they were kept in a pair
> > of global maps.
> >
> > - Modules streaming couldn't discriminate between the actor or destroy
> > functions when merging.
> >
> > - Modules streaming wasn't setting the cfun->coroutine_component flag,
> > needed to activate the middle-end coroutine lowering pass.
> >
> > This patch also separates the coroutine_info_table initialization from
> > the ensure_coro_initialized function. If the first time we see a
> > coroutine is from a module import, we need to register the
> > transformation functions now but calling ensure_coro_initialized would
> > lookup e.g. std::coroutine_traits, which may only be visible from this
> > module that we're currently reading, causing a recursive load.
> > Separating the concerns allows this to work correctly.
> >
> > gcc/cp/ChangeLog:
> >
> > * coroutines.cc (create_coroutine_info_table): New function.
> > (get_or_insert_coroutine_info): Mark static.
> > (ensure_coro_initialized): Likewise; use
> > create_coroutine_info_table.
> > (coro_promise_type_found_p): Set DECL_CONTEXT for proxies.
> > (coro_set_ramp_function): New function.
> > (coro_set_transform_functions): New function.
> > (coro_build_actor_or_destroy_function): Use
> > coro_set_ramp_function, mark as DECL_COROUTINE_P.
> > * cp-tree.h (coro_set_transform_functions): Declare.
> > (coro_set_ramp_function): Declare.
> > * module.cc (struct merge_key): New field coro_disc.
> > (dumper::impl::nested_name): Distinguish coroutine transform
> > functions.
> > (get_coroutine_discriminator): New function.
> > (trees_out::key_mergeable): Stream coroutine discriminator.
> > (check_mergeable_decl): Adjust comment, check for matching
> > coroutine discriminator.
> > (trees_in::key_mergeable): Read coroutine discriminator.
> > (has_definition): Override for coroutine transform functions.
> > (trees_out::write_function_def): Stream linked ramp, actor, and
> > destroy functions for coroutines.
> > (trees_in::read_function_def): Read them.
> > (module_state::read_cluster): Set cfun->coroutine_component.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/coro-1_a.C: New test.
> > * g++.dg/modules/coro-1_b.C: New test.
> >
> > Reviewed-by: Iain Sandoe <[email protected]>
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> > gcc/cp/coroutines.cc | 59 +++++++++++++---
> > gcc/cp/cp-tree.h | 4 ++
> > gcc/cp/module.cc | 90 +++++++++++++++++++++++--
> > gcc/testsuite/g++.dg/modules/coro-1_a.C | 34 ++++++++++
> > gcc/testsuite/g++.dg/modules/coro-1_b.C | 19 ++++++
> > 5 files changed, 189 insertions(+), 17 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/coro-1_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/coro-1_b.C
> >
> > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> > index f0485a95073..2056b184d99 100644
> > --- a/gcc/cp/coroutines.cc
> > +++ b/gcc/cp/coroutines.cc
> > @@ -353,10 +353,20 @@ coroutine_info_hasher::equal (coroutine_info *lhs,
> > const compare_type& rhs)
> > return lhs->function_decl == rhs;
> > }
> >
> > +/* Initialize the coroutine info table, to hold state per coroutine decl,
> > + if not already created. */
> > +
> > +static void
> > +create_coroutine_info_table ()
> > +{
> > + if (!coroutine_info_table)
> > + coroutine_info_table = hash_table<coroutine_info_hasher>::create_ggc
> > (11);
> > +}
> > +
> > /* Get the existing coroutine_info for FN_DECL, or insert a new one if the
> > entry does not yet exist. */
> >
> > -coroutine_info *
> > +static coroutine_info *
> > get_or_insert_coroutine_info (tree fn_decl)
> > {
> > gcc_checking_assert (coroutine_info_table != NULL);
> > @@ -375,7 +385,7 @@ get_or_insert_coroutine_info (tree fn_decl)
> >
> > /* Get the existing coroutine_info for FN_DECL, fail if it doesn't exist.
> > */
> >
> > -coroutine_info *
> > +static coroutine_info *
> > get_coroutine_info (tree fn_decl)
> > {
> > if (coroutine_info_table == NULL)
> > @@ -757,11 +767,7 @@ ensure_coro_initialized (location_t loc)
> > if (!void_coro_handle_address)
> > return false;
> >
> > - /* A table to hold the state, per coroutine decl. */
> > - gcc_checking_assert (coroutine_info_table == NULL);
> > - coroutine_info_table =
> > - hash_table<coroutine_info_hasher>::create_ggc (11);
> > -
> > + create_coroutine_info_table ();
> > if (coroutine_info_table == NULL)
> > return false;
> >
> > @@ -873,11 +879,13 @@ coro_promise_type_found_p (tree fndecl, location_t
> > loc)
> > coro_info->self_h_proxy
> > = build_lang_decl (VAR_DECL, coro_self_handle_id,
> > coro_info->handle_type);
> > + DECL_CONTEXT (coro_info->self_h_proxy) = fndecl;
> >
> > /* Build a proxy for the promise so that we can perform lookups. */
> > coro_info->promise_proxy
> > = build_lang_decl (VAR_DECL, coro_promise_id,
> > coro_info->promise_type);
> > + DECL_CONTEXT (coro_info->promise_proxy) = fndecl;
> >
> > /* Note where we first saw a coroutine keyword. */
> > coro_info->first_coro_keyword = loc;
> > @@ -902,6 +910,17 @@ coro_get_ramp_function (tree decl)
> > return NULL_TREE;
> > }
> >
> > +/* Given a DECL, an actor or destroyer, build a link from that to the ramp
> > + function. Used by modules streaming. */
> > +
> > +void
> > +coro_set_ramp_function (tree decl, tree ramp)
> > +{
> > + if (!to_ramp)
> > + to_ramp = hash_map<tree, tree>::create_ggc (10);
> > + to_ramp->put (decl, ramp);
> > +}
> > +
> > /* Given the DECL for a ramp function (the user's original declaration)
> > return
> > the actor function if it has been defined. */
> >
> > @@ -926,6 +945,27 @@ coro_get_destroy_function (tree decl)
> > return NULL_TREE;
> > }
> >
> > +/* For a given ramp function DECL, set the actor and destroy functions.
> > + This is only used by modules streaming. */
> > +
> > +void
> > +coro_set_transform_functions (tree decl, tree actor, tree destroy)
> > +{
> > + /* Only relevant with modules. */
> > + gcc_assert (modules_p ());
>
> Could we make these asserts gcc_checking_asserts() ?
Updated locally.
> >
> > + /* This should only be called for newly streamed declarations. */
> > + gcc_assert (!get_coroutine_info (decl));
> > +
> > + /* This might be the first use of coroutine info in the TU, so
> > + create the coroutine info table if needed. */
> > + create_coroutine_info_table ();
> > +
> > + coroutine_info *coroutine = get_or_insert_coroutine_info (decl);
> > + coroutine->actor_decl = actor;
> > + coroutine->destroy_decl = destroy;
> > +}
> > +
> > /* Given a CO_AWAIT_EXPR AWAIT_EXPR, return its resume call. */
> >
> > tree
> > @@ -4393,14 +4433,13 @@ coro_build_actor_or_destroy_function (tree orig,
> > tree fn_type,
> > = build_lang_decl (FUNCTION_DECL, copy_node (DECL_NAME (orig)),
> > fn_type);
> >
> > /* Allow for locating the ramp (original) function from this one. */
> > - if (!to_ramp)
> > - to_ramp = hash_map<tree, tree>::create_ggc (10);
> > - to_ramp->put (fn, orig);
> > + coro_set_ramp_function (fn, orig);
> >
> > DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
> > DECL_SOURCE_LOCATION (fn) = loc;
> > DECL_ARTIFICIAL (fn) = true;
> > DECL_INITIAL (fn) = error_mark_node;
> > + DECL_COROUTINE_P (fn) = true;
> >
> > tree id = get_identifier ("frame_ptr");
> > tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index b8470fc256c..1d40b387d6e 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -9144,6 +9144,10 @@ extern tree coro_get_ramp_function (tree);
> >
> > extern tree co_await_get_resume_call (tree await_expr);
> >
> > +/* Only for use by modules. */
> > +extern void coro_set_transform_functions (tree, tree, tree);
> > +extern void coro_set_ramp_function (tree, tree);
> > +
> > /* Inline bodies. */
> >
> > inline tree
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 174bebff01c..e6b2b2f7858 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2969,6 +2969,7 @@ static char const *const merge_kind_name[MK_hwm] =
> > /* Mergeable entity location data. */
> > struct merge_key {
> > cp_ref_qualifier ref_q : 2;
> > + unsigned coro_disc : 2; /* Discriminator for coroutine transforms. */
> > unsigned index;
> >
> > tree ret; /* Return type, if appropriate. */
> > @@ -2977,7 +2978,7 @@ struct merge_key {
> > tree constraints; /* Constraints. */
> >
> > merge_key ()
> > - :ref_q (REF_QUAL_NONE), index (0),
> > + :ref_q (REF_QUAL_NONE), coro_disc (0), index (0),
> > ret (NULL_TREE), args (NULL_TREE),
> > constraints (NULL_TREE)
> > {
> > @@ -4590,6 +4591,17 @@ dumper::impl::nested_name (tree t)
> > else
> > fputs ("#null#", stream);
> >
> > + if (t && TREE_CODE (t) == FUNCTION_DECL && DECL_COROUTINE_P (t))
> > + if (tree ramp = DECL_RAMP_FN (t))
> > + {
> > + if (DECL_ACTOR_FN (ramp) == t)
> > + fputs (".actor", stream);
> > + else if (DECL_DESTROY_FN (ramp) == t)
> > + fputs (".destroy", stream);
> > + else
> > + gcc_unreachable ();
> > + }
> > +
> > if (origin >= 0)
> > {
> > const module_state *module = (*modules)[origin];
> > @@ -11712,6 +11724,25 @@ trees_in::decl_container ()
> > return container;
> > }
> >
> > +/* Gets a 2-bit discriminator to distinguish coroutine actor or destroy
> > + functions from a normal function. */
> > +
> > +static int
> > +get_coroutine_discriminator (tree inner)
> > +{
> > + if (DECL_COROUTINE_P (inner))
> > + if (tree ramp = DECL_RAMP_FN (inner))
> > + {
> > + if (DECL_ACTOR_FN (ramp) == inner)
> > + return 1;
> > + else if (DECL_DESTROY_FN (ramp) == inner)
> > + return 2;
> > + else
> > + gcc_unreachable ();
> > + }
> > + return 0;
> > +}
> > +
> > /* Write out key information about a mergeable DEP. Does not write
> > the contents of DEP itself. The context has already been
> > written. The container has already been streamed. */
> > @@ -11803,6 +11834,7 @@ trees_out::key_mergeable (int tag, merge_kind mk,
> > tree decl, tree inner,
> > tree fn_type = TREE_TYPE (inner);
> >
> > key.ref_q = type_memfn_rqual (fn_type);
> > + key.coro_disc = get_coroutine_discriminator (inner);
> > key.args = TYPE_ARG_TYPES (fn_type);
> >
> > if (tree reqs = get_constraints (inner))
> > @@ -11939,7 +11971,12 @@ trees_out::key_mergeable (int tag, merge_kind mk,
> > tree decl, tree inner,
> > tree_node (name);
> > if (streaming_p ())
> > {
> > - unsigned code = (key.ref_q << 0) | (key.index << 2);
> > + /* Check we have enough bits for the index. */
> > + gcc_checking_assert (key.index < (1u << (sizeof (unsigned) * 8 - 4)));
> > +
> > + unsigned code = ((key.ref_q << 0)
> > + | (key.coro_disc << 2)
> > + | (key.index << 4));
> > u (code);
> > }
> >
> > @@ -11963,8 +12000,8 @@ trees_out::key_mergeable (int tag, merge_kind mk,
> > tree decl, tree inner,
> > }
> > }
> >
> > -/* DECL is a new declaration that may be duplicated in OVL. Use RET &
> > - ARGS to find its clone, or NULL. If DECL's DECL_NAME is NULL, this
> > +/* DECL is a new declaration that may be duplicated in OVL. Use KEY
> > + to find its clone, or NULL. If DECL's DECL_NAME is NULL, this
> > has been found by a proxy. It will be an enum type located by its
> > first member.
> >
> > @@ -12019,6 +12056,7 @@ check_mergeable_decl (merge_kind mk, tree decl,
> > tree ovl, merge_key const &key)
> > || same_type_p (key.ret, fndecl_declared_return_type (m_inner)))
> > && type_memfn_rqual (m_type) == key.ref_q
> > && compparms (key.args, TYPE_ARG_TYPES (m_type))
> > + && get_coroutine_discriminator (m_inner) == key.coro_disc
> > /* Reject if old is a "C" builtin and new is not "C".
> > Matches decls_match behaviour. */
> > && (!DECL_IS_UNDECLARED_BUILTIN (m_inner)
> > @@ -12141,7 +12179,8 @@ trees_in::key_mergeable (int tag, merge_kind mk,
> > tree decl, tree inner,
> > merge_key key;
> > unsigned code = u ();
> > key.ref_q = cp_ref_qualifier ((code >> 0) & 3);
> > - key.index = code >> 2;
> > + key.coro_disc = (code >> 2) & 3;
> > + key.index = code >> 4;
> >
> > if (mk == MK_enum)
> > key.ret = tree_node ();
> > @@ -12874,6 +12913,12 @@ has_definition (tree decl)
> > if (use_tpl < 2)
> > return true;
> > }
> > +
> > + /* Coroutine transform functions always need to be emitted
> > + into the importing TU if the ramp function will be. */
> > + if (DECL_COROUTINE_P (decl))
> > + if (tree ramp = DECL_RAMP_FN (decl))
> > + return has_definition (ramp);
> > break;
> >
> > case TYPE_DECL:
> > @@ -13057,6 +13102,17 @@ trees_out::write_function_def (tree decl)
> > state->write_location (*this, f->function_start_locus);
> > state->write_location (*this, f->function_end_locus);
> > }
> > +
> > + if (DECL_COROUTINE_P (decl))
> > + {
> > + tree ramp = DECL_RAMP_FN (decl);
> > + tree_node (ramp);
>
> What does this ^ do when ramp is nullptr? (Is it a NOP?)
>
It streams a single 0 byte (a 'tt_null' tag), which is streamed back as
a NULL_TREE in the reader.
Thanks,
Nathaniel
> > + if (!ramp)
> > + {
> > + tree_node (DECL_ACTOR_FN (decl));
> > + tree_node (DECL_DESTROY_FN (decl));
> > + }
> > + }
> > }
> >
> > void
> > @@ -13072,13 +13128,13 @@ trees_in::read_function_def (tree decl, tree
> > maybe_template)
> > tree initial = tree_node ();
> > tree saved = tree_node ();
> > tree context = tree_node ();
> > - constexpr_fundef cexpr;
> > post_process_data pdata {};
> > pdata.decl = maybe_template;
> >
> > tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl));
> > bool installing = maybe_dup && !DECL_SAVED_TREE (decl);
> >
> > + constexpr_fundef cexpr;
> > if (u ())
> > {
> > cexpr.parms = chained_decls ();
> > @@ -13090,7 +13146,6 @@ trees_in::read_function_def (tree decl, tree
> > maybe_template)
> > cexpr.decl = NULL_TREE;
> >
> > unsigned flags = u ();
> > -
> > if (flags & 2)
> > {
> > pdata.start_locus = state->read_location (*this);
> > @@ -13101,6 +13156,21 @@ trees_in::read_function_def (tree decl, tree
> > maybe_template)
> > pdata.infinite_loop = flags & 32;
> > }
> >
> > + tree coro_actor = NULL_TREE;
> > + tree coro_destroy = NULL_TREE;
> > + tree coro_ramp = NULL_TREE;
> > + if (DECL_COROUTINE_P (decl))
> > + {
> > + coro_ramp = tree_node ();
> > + if (!coro_ramp)
> > + {
> > + coro_actor = tree_node ();
> > + coro_destroy = tree_node ();
> > + if ((coro_actor == NULL_TREE) != (coro_destroy == NULL_TREE))
> > + set_overrun ();
> > + }
> > + }
> > +
> > if (get_overrun ())
> > return NULL_TREE;
> >
> > @@ -13116,6 +13186,11 @@ trees_in::read_function_def (tree decl, tree
> > maybe_template)
> > if (cexpr.decl)
> > register_constexpr_fundef (cexpr);
> >
> > + if (coro_ramp)
> > + coro_set_ramp_function (decl, coro_ramp);
> > + else if (coro_actor && coro_destroy)
> > + coro_set_transform_functions (decl, coro_actor, coro_destroy);
> > +
> > if (DECL_LOCAL_DECL_P (decl))
> > /* Block-scope OMP UDRs aren't real functions, and don't need a
> > function structure to be allocated or to be expanded. */
> > @@ -17572,6 +17647,7 @@ module_state::read_cluster (unsigned snum)
> > cfun->language->returns_null = pdata.returns_null;
> > cfun->language->returns_abnormally = pdata.returns_abnormally;
> > cfun->language->infinite_loop = pdata.infinite_loop;
> > + cfun->coroutine_component = DECL_COROUTINE_P (decl);
> >
> > /* Make sure we emit explicit instantiations.
> > FIXME do we want to do this in expand_or_defer_fn instead? */
> > diff --git a/gcc/testsuite/g++.dg/modules/coro-1_a.C
> > b/gcc/testsuite/g++.dg/modules/coro-1_a.C
> > new file mode 100644
> > index 00000000000..165c643fd7f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/coro-1_a.C
> > @@ -0,0 +1,34 @@
> > +// { dg-do compile { target c++20 } }
> > +// { dg-additional-options "-fmodules -fdump-lang-module" }
> > +// { dg-module-cmi M }
> > +
> > +module;
> > +#include <coroutine>
> > +export module M;
> > +
> > +struct simple_promise;
> > +struct simple_coroutine : std::coroutine_handle<simple_promise> {
> > + using promise_type = ::simple_promise;
> > +};
> > +struct simple_promise {
> > + simple_coroutine get_return_object() { return {
> > simple_coroutine::from_promise(*this) }; }
> > + std::suspend_always initial_suspend() noexcept { return {}; }
> > + std::suspend_always final_suspend() noexcept { return {}; }
> > + void return_void() {}
> > + void unhandled_exception() {}
> > +};
> > +export simple_coroutine coroutine() {
> > + co_return;
> > +}
> > +export inline simple_coroutine inline_coroutine() {
> > + co_return;
> > +}
> > +export template <typename T> simple_coroutine template_coroutine() {
> > + co_return;
> > +}
> > +
> > +// The actor and destroy functions should have definitions streamed only
> > for inline coroutines
> > +// { dg-final { scan-lang-dump-not {Writing definition
> > function_decl:'::coroutine.actor'} module } }
> > +// { dg-final { scan-lang-dump-not {Writing definition
> > function_decl:'::coroutine.destroy'} module } }
> > +// { dg-final { scan-lang-dump {Writing definition
> > function_decl:'::inline_coroutine.actor'} module } }
> > +// { dg-final { scan-lang-dump {Writing definition
> > function_decl:'::inline_coroutine.destroy'} module } }
> > diff --git a/gcc/testsuite/g++.dg/modules/coro-1_b.C
> > b/gcc/testsuite/g++.dg/modules/coro-1_b.C
> > new file mode 100644
> > index 00000000000..e1384a7d639
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/coro-1_b.C
> > @@ -0,0 +1,19 @@
> > +// { dg-module-do run { target c++20 } }
> > +// { dg-additional-options "-fmodules" }
> > +
> > +#include <coroutine>
> > +import M;
> > +
> > +int main() {
> > + auto a = coroutine();
> > + a.resume();
> > + a.destroy();
> > +
> > + auto b = inline_coroutine();
> > + b.resume();
> > + b.destroy();
> > +
> > + auto c = template_coroutine<int>();
> > + c.resume();
> > + c.destroy();
> > +}
> > --
> > 2.51.0
> >
>
