On Sat, Oct 18, 2025 at 06:41:15PM +0300, Jason Merrill wrote:
> On 10/18/25 4:13 PM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
> >
> > -- >8 --
> >
> > The ICE in the linked PR occurs because we first stream the lambda type
> > before its keyed decl has been streamed, but the key decl's type depends
> > on the lambda. And so when streaming the key decl to check for an
> > existing decl to merge with, merging the key decl itself crashes because
> > its type has only been partially streamed.
> >
> > This patch fixes the issue by generalising the existing FIELD_DECL
> > handling to any class member using the outermost containing TYPE_DECL as
> > its key type. This way we can guarantee that the key decl has been
> > streamed before the lambda type is otherwise needed.
>
> Why doesn't the existing FIELD_DECL handling already key the lambda to the
> class, since "do_nothing" is a data member?
>
Because 'do_nothing' is a static member it's a VAR_DECL, not a
FIELD_DECL.
> Why is the outermost class needed? It looks like the A/B case is testing
> that, but why doesn't it work to start streaming the lambda and check with
> MoreInner for a merge candidate?
>
A more minimal example of this case that also fails in GCC 15:
struct X {
struct Inner {
union MoreInner {
decltype([]{}) y;
};
};
using B = decltype(Inner::MoreInner::y);
};
Here we crash when streaming in the attached decls for 'MoreInner'.
The type of 'y' is attached to 'MoreInner' directly, but we currently
stream members in reverse order of declaration, so we see 'B' before
streaming 'Inner' or 'MoreInner'. So similarly to the case in the PR,
we end up reading 'MoreInner' first while finding the key entity for the
lambda, see that's it a duplicate, read its attached entities, and then
fail the check here:
if (DECL_LANG_SPECIFIC (inner)
&& DECL_MODULE_KEYED_DECLS_P (inner))
{
/* Read and maybe install the attached entities. */
bool existed;
auto &set = keyed_table->get_or_insert (STRIP_TEMPLATE (existing),
&existed);
unsigned num = u ();
if (is_new == existed)
set_overrun ();
if (is_new)
set.reserve (num);
for (unsigned ix = 0; !get_overrun () && ix != num; ix++)
{
tree attached = tree_node ();
dump (dumper::MERGE)
&& dump ("Read %d[%u] %s attached decl %N", tag, ix,
is_new ? "new" : "matched", attached);
if (is_new)
set.quick_push (attached);
else if (set[ix] != attached)
set_overrun (); // <--- fail here
}
}
because the partially-streamed, not-yet-deduplicated type of the lambda
isn't the same type as the existing lambda keyed to the existing
MoreInner type.
By keying to the outermost class we avoid any potential issues with
out-of-order member dependencies. These specific cases could possibly
also be fixed by reversing the order that we stream members, but I'm not
sure if that would expose other possible issues (with e.g. lazily
declared members or instantiations or whatnot). There's also the very
very slight benefit that we need to allocate less vecs in the
keyed_decls map if more decls reuse the same key here.
Nathaniel
> > PR c++/122310
> >
> > gcc/cp/ChangeLog:
> >
> > * module.cc (get_keyed_decl_scope): New function.
> > (trees_out::get_merge_kind): Use it.
> > (trees_out::key_mergeable): Use it.
> > (maybe_key_decl): Key to the containing type for all members.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/lambda-12.h: New test.
> > * g++.dg/modules/lambda-12_a.H: New test.
> > * g++.dg/modules/lambda-12_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> > gcc/cp/module.cc | 67 +++++++++++++---------
> > gcc/testsuite/g++.dg/modules/lambda-12.h | 27 +++++++++
> > gcc/testsuite/g++.dg/modules/lambda-12_a.H | 5 ++
> > gcc/testsuite/g++.dg/modules/lambda-12_b.C | 5 ++
> > 4 files changed, 77 insertions(+), 27 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-12.h
> > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-12_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-12_b.C
> >
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index bdc7e6af874..972ca36444f 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2798,6 +2798,8 @@ vec<tree, va_heap, vl_embed> *post_load_decls;
> > typedef hash_map<tree, auto_vec<tree>> keyed_map_t;
> > static keyed_map_t *keyed_table;
> > +static tree get_keyed_decl_scope (tree);
> > +
> > /* Instantiations of temploid friends imported from another module
> > need to be attached to the same module as the temploid. This maps
> > these decls to the temploid they are instantiated from, as there is
> > @@ -11464,20 +11466,12 @@ trees_out::get_merge_kind (tree decl, depset *dep)
> > if (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl))
> > && LAMBDA_TYPE_P (TREE_TYPE (decl)))
> > {
> > - if (tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl)))
> > - {
> > - /* Lambdas attached to fields are keyed to its class. */
> > - if (TREE_CODE (scope) == FIELD_DECL)
> > - scope = TYPE_NAME (DECL_CONTEXT (scope));
> > - if (DECL_LANG_SPECIFIC (scope)
> > - && DECL_MODULE_KEYED_DECLS_P (scope))
> > - {
> > - mk = MK_keyed;
> > - break;
> > - }
> > - }
> > - /* Lambdas not attached to any mangling scope are TU-local. */
> > - mk = MK_unique;
> > + if (get_keyed_decl_scope (decl))
> > + mk = MK_keyed;
> > + else
> > + /* Lambdas not attached to any mangling scope are TU-local
> > + and so cannot be deduplicated. */
> > + mk = MK_unique;
> > break;
> > }
> > @@ -11778,16 +11772,9 @@ trees_out::key_mergeable (int tag, merge_kind mk,
> > tree decl, tree inner,
> > case MK_keyed:
> > {
> > - gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner)));
> > - tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (inner));
> > - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL
> > - || TREE_CODE (scope) == FIELD_DECL
> > - || TREE_CODE (scope) == PARM_DECL
> > - || TREE_CODE (scope) == TYPE_DECL
> > - || TREE_CODE (scope) == CONCEPT_DECL);
> > - /* Lambdas attached to fields are keyed to the class. */
> > - if (TREE_CODE (scope) == FIELD_DECL)
> > - scope = TYPE_NAME (DECL_CONTEXT (scope));
> > + tree scope = get_keyed_decl_scope (inner);
> > + gcc_checking_assert (scope);
> > +
> > auto *root = keyed_table->get (scope);
> > unsigned ix = root->length ();
> > /* If we don't find it, we'll write a really big number
> > @@ -21506,9 +21493,11 @@ maybe_key_decl (tree ctx, tree decl)
> > && TREE_CODE (ctx) != CONCEPT_DECL)
> > return;
> > - /* For fields, key it to the containing type to handle deduplication
> > - correctly. */
> > - if (TREE_CODE (ctx) == FIELD_DECL)
> > + /* For members, key it to the containing type to handle deduplication
> > + correctly: otherwise we may run into issues when loading the lambda
> > + type before its keyed decl, if the member itself depends on the
> > + lambda type (PR c++/122310). */
> > + while (RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (ctx)))
> > ctx = TYPE_NAME (DECL_CONTEXT (ctx));
> > if (!keyed_table)
> > @@ -21523,6 +21512,30 @@ maybe_key_decl (tree ctx, tree decl)
> > vec.safe_push (decl);
> > }
> > +/* Find the scope that the lambda DECL is keyed to, if any. */
> > +
> > +static tree
> > +get_keyed_decl_scope (tree decl)
> > +{
> > + gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (decl)));
> > + tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl));
> > + if (!scope)
> > + return NULL_TREE;
> > +
> > + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL
> > + || TREE_CODE (scope) == FIELD_DECL
> > + || TREE_CODE (scope) == PARM_DECL
> > + || TREE_CODE (scope) == TYPE_DECL
> > + || TREE_CODE (scope) == CONCEPT_DECL);
> > +
> > + while (RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (scope)))
> > + scope = TYPE_NAME (DECL_CONTEXT (scope));
> > +
> > + gcc_checking_assert (DECL_LANG_SPECIFIC (scope)
> > + && DECL_MODULE_KEYED_DECLS_P (scope));
> > + return scope;
> > +}
> > +
> > /* DECL is an instantiated friend that should be attached to the same
> > module that ORIG is. */
> > diff --git a/gcc/testsuite/g++.dg/modules/lambda-12.h
> > b/gcc/testsuite/g++.dg/modules/lambda-12.h
> > new file mode 100644
> > index 00000000000..4dd329d4760
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lambda-12.h
> > @@ -0,0 +1,27 @@
> > +// PR c++/122310
> > +struct Foo {
> > + constexpr static inline auto do_nothing = [](auto && ...){};
> > + using TNothing = decltype(do_nothing);
> > +};
> > +
> > +template <typename T>
> > +struct X {
> > + struct Inner {
> > + union MoreInner {
> > + static constexpr auto x = []{};
> > +#if __cplusplus >= 202002L
> > + static decltype([]{}) y;
> > +#endif
> > + };
> > + };
> > +
> > + using A = decltype(Inner::MoreInner::x);
> > +#if __cplusplus >= 202002L
> > + using B = decltype(Inner::MoreInner::y);
> > +#endif
> > +};
> > +
> > +inline X<int>::A* a{};
> > +#if __cplusplus >= 202002L
> > +inline X<int>::B* b{};
> > +#endif
> > diff --git a/gcc/testsuite/g++.dg/modules/lambda-12_a.H
> > b/gcc/testsuite/g++.dg/modules/lambda-12_a.H
> > new file mode 100644
> > index 00000000000..83f5d150367
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lambda-12_a.H
> > @@ -0,0 +1,5 @@
> > +// PR c++/122310
> > +// { dg-additional-options "-fmodule-header" }
> > +// { dg-module-cmi {} }
> > +
> > +#include "lambda-12.h"
> > diff --git a/gcc/testsuite/g++.dg/modules/lambda-12_b.C
> > b/gcc/testsuite/g++.dg/modules/lambda-12_b.C
> > new file mode 100644
> > index 00000000000..c24d9398a45
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lambda-12_b.C
> > @@ -0,0 +1,5 @@
> > +// PR c++/122310
> > +// { dg-additional-options "-fmodules -fno-module-lazy" }
> > +
> > +#include "lambda-12.h"
> > +import "lambda-12_a.H";
>
