On 10/19/25 2:14 AM, Nathaniel Shead wrote:
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.
Ah, indeed.
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).
I would imagine that in general, streaming members in declaration order
would improve how often a reference points to something already loaded.
Is there any rationale for reverse order?
In the meantime, this patch is OK with the tweaks below.
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.
@@ -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)))
DECL_CLASS_SCOPE_P
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)))
Likewise.
Jason
