On Fri, Jan 09, 2026 at 08:19:33AM +0000, Iain Sandoe wrote:
> Hi Nathaniel.
>
> thanks for looking at this (both language features were in flux at the same
> time and I don’t think the interaction was especially well-considered).
>
> > On 9 Jan 2026, at 04:39, Nathaniel Shead <[email protected]> wrote:
> >
> > 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 were always considered
> > non-inline, even for an inline ramp function, which meant that modules
> > didn't realise it needed to stream a definition.
>
> I am somewhat concerned about the proposed change here (it looks like
> an ABI change - albeit an addition so, presumably, not breaking).
>
> The principle is that the three functions are all considered to be part of the
> same entity, where the user-visible interface is only the ramp and the
> coroutine
> handle.
>
> That is, I don’t think that this is an ‘exposure’ in the sense of P1815 since
> the split into ramp/actor/destroyer is an internal detail invisible to the end
> user.
>
I think perhaps my wording wasn't clear; this wasn't so much about
internal linkage (though that of course is also related) but about vague
linkage. Consider the following TU:
#include <coroutine>
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() {}
};
inline simple_coroutine foo() {
co_return;
}
We do not emit foo as it is unused, but we do emit the transform
functions (e.g. _Z3fooP13_Z3foov.Frame.actor). These functions are
also not marked as '.globl' (they are not TREE_PUBLIC and so are
considered internal to the TU), so a different TU calling a
forward-declared 'foo' would get undefined references to the actor
function etc.
My assumption was that the intention is that they are all meant to come
along for the ride with each other wrt linkage; that is, in this case
foo has vague linkage, so foo.actor and foo.destroy should be as well,
and will be emitted iff foo is. Rather than every TU having copies of
the transform functions.
> Would it be possible to make the actor and destroyer fns dependencies
> of the ramp (they are) and have them streamed and restored in that way?
That said, it should be possible to special-case the actor and destroyer
functions and always stream their bodies in a modules context if we're
streaming the body of the ramp function, which would maintain the
current ABI, if that is indeed intentional.
Nathaniel
>
> Iain
>
> > - 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 of proxies.
> > (coro_set_ramp_function): New function.
> > (coro_set_transform_functions): New function.
> > (coro_build_actor_or_destroy_function): Use
> > coro_set_ramp_function; copy linkage from original function.
> > * cp-tree.h (coro_set_transform_functions): Declare.
> > (coro_set_ramp_function): Declare.
> > * decl2.cc (mark_used): Mark transform functions as used if we
> > use the ramp function.
> > * module.cc (struct merge_key): New field coro_disc.
> > (struct post_process_data): New field coroutine_component.
> > (get_coroutine_discriminator): New function.
> > (trees_out::key_mergeable): Write coroutine discriminator.
> > (check_mergeable_decl): Adjust comment, check for matching
> > coroutine discriminator.
> > (trees_in::key_mergeable): Read coroutine discriminator.
> > (trees_out::write_function_def): Write coroutine_component and
> > ramp/actor/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.
> >
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> > gcc/cp/coroutines.cc | 64 ++++++++++++++++----
> > gcc/cp/cp-tree.h | 4 ++
> > gcc/cp/decl2.cc | 13 +++++
> > gcc/cp/module.cc | 78 ++++++++++++++++++++++---
> > gcc/testsuite/g++.dg/modules/coro-1_a.C | 28 +++++++++
> > gcc/testsuite/g++.dg/modules/coro-1_b.C | 19 ++++++
> > 6 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..930d453dde2 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 ());
> > +
> > + /* 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,15 +4433,19 @@ 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;
> >
> > + /* Copy linkage from the original function. */
> > + TREE_PUBLIC (fn) = TREE_PUBLIC (orig);
> > + DECL_DECLARED_INLINE_P (fn) = DECL_DECLARED_INLINE_P (orig);
> > + DECL_NOT_REALLY_EXTERN (fn) = DECL_NOT_REALLY_EXTERN (orig);
> > + DECL_INTERFACE_KNOWN (fn) = DECL_INTERFACE_KNOWN (orig);
> > +
> > tree id = get_identifier ("frame_ptr");
> > tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
> > DECL_ARTIFICIAL (fp) = true;
> > 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/decl2.cc b/gcc/cp/decl2.cc
> > index e807eab1b8a..d50864b8f75 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -6480,6 +6480,19 @@ mark_used (tree decl, tsubst_flags_t complain /* =
> > tf_warning_or_error */)
> > return false;
> > }
> >
> > + /* For coroutines, we need to mark the transform functions as used,
> > + if they exist yet. */
> > + if (flag_coroutines
> > + && TREE_CODE (decl) == FUNCTION_DECL
> > + && DECL_COROUTINE_P (decl)
> > + && DECL_RAMP_P (decl))
> > + {
> > + if (tree actor = DECL_ACTOR_FN (decl))
> > + mark_used (actor);
> > + if (tree destroy = DECL_DESTROY_FN (decl))
> > + mark_used (destroy);
> > + }
> > +
> > /* If DECL has a deduced return type, we need to instantiate it now to
> > find out its type. For OpenMP user defined reductions, we need them
> > instantiated for reduction clauses which inline them by hand directly.
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index af0730ba974..67fb1e4a22d 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)
> > {
> > @@ -2999,6 +3000,7 @@ struct post_process_data {
> > bool returns_null;
> > bool returns_abnormally;
> > bool infinite_loop;
> > + bool coroutine_component;
> > };
> >
> > /* Tree stream reader. Note that reading a stream doesn't mark the
> > @@ -11696,6 +11698,24 @@ 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 (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. */
> > @@ -11787,6 +11807,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))
> > @@ -11923,7 +11944,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);
> > }
> >
> > @@ -11947,8 +11973,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.
> >
> > @@ -12008,6 +12034,9 @@ check_mergeable_decl (merge_kind mk, tree decl,
> > tree ovl, merge_key const &key)
> > && (!DECL_IS_UNDECLARED_BUILTIN (m_inner)
> > || !DECL_EXTERN_C_P (m_inner)
> > || DECL_EXTERN_C_P (d_inner))
> > + /* Reject if we're not the same kind of coroutine function. */
> > + && (!flag_coroutines
> > + || key.coro_disc == get_coroutine_discriminator (m_inner))
> > /* Reject if one is a different member of a
> > guarded/pre/post fn set. */
> > && (!flag_contracts
> > @@ -12125,7 +12154,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 ();
> > @@ -13031,6 +13061,8 @@ trees_out::write_function_def (tree decl)
> > flags |= 8 * f->language->returns_null;
> > flags |= 16 * f->language->returns_abnormally;
> > flags |= 32 * f->language->infinite_loop;
> > + /* Set for coroutines. */
> > + flags |= 64 * f->coroutine_component;
> > }
> >
> > u (flags);
> > @@ -13041,6 +13073,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 (f && f->coroutine_component)
> > + {
> > + tree ramp = DECL_RAMP_FN (decl);
> > + tree_node (ramp);
> > + if (!ramp)
> > + {
> > + tree_node (DECL_ACTOR_FN (decl));
> > + tree_node (DECL_DESTROY_FN (decl));
> > + }
> > + }
> > }
> >
> > void
> > @@ -13056,13 +13099,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 ();
> > @@ -13074,7 +13117,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);
> > @@ -13083,6 +13125,22 @@ trees_in::read_function_def (tree decl, tree
> > maybe_template)
> > pdata.returns_null = flags & 8;
> > pdata.returns_abnormally = flags & 16;
> > pdata.infinite_loop = flags & 32;
> > + pdata.coroutine_component = flags & 64;
> > + }
> > +
> > + tree coro_actor = NULL_TREE;
> > + tree coro_destroy = NULL_TREE;
> > + tree coro_ramp = NULL_TREE;
> > + if (pdata.coroutine_component)
> > + {
> > + 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 ())
> > @@ -13100,6 +13158,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. */
> > @@ -17556,6 +17619,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 = pdata.coroutine_component;
> >
> > /* 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..ec2c8300a80
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/coro-1_a.C
> > @@ -0,0 +1,28 @@
> > +// { dg-do compile { target c++20 } }
> > +// { dg-additional-options "-fmodules" }
> > +// { 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;
> > +}
> > 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
> >
>
