On 1/17/26 1:39 PM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

r14-9948-g716af95fd45487 added support for merging local types of
functions.  This however causes the linked PR to crash, because when
restreaming the local type from a partition for the primary module
interface's CMI, we no longer have the DECL_INITIAL for a function.

This patch fixes the issue by reusing the MK_keyed merge kind, as used
for lambda types.  This is required so that if a module partition
imports both the primary module interface and a different module
partition that both provide the same local type it can properly dedup
the declarations.

We only need to do this if !has_definition; in cases where a definition
is available we keep using the MK_local_type behaviour as that avoids
the need to maintain a separately allocated chain of keyed decls.

An additional change is to further the modifications made in r16-4671
and always attempt to key to the top-most decl, including going through
possibly many nested class and function definitions.  This avoids any
similar issues to that bug where we read a keyed decl before we see the
decl it's keyed to now that we support keying to functions as well.

        PR c++/123627

gcc/cp/ChangeLog:

        * class.cc (finish_struct): Maybe key function-local types.
        * module.cc (trees_out::get_merge_kind): Choose whether to use
        MK_local_type or MK_keyed for a local type based on if the
        immediate context's definition will be streamed.
        (trees_in::key_mergeable): Allow key decls on FUNCTION_DECL.
        (get_key_scope): New function.

I might call this adjust_key_scope.  OK either way.

        (maybe_key_decl): Handle key decls on FUNCTION_DECL; check that
        we only key a given decl to a context at most once.
        (get_keyed_decl_scope): Support non-lambda decls.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/block-decl-4_a.C: New test.
        * g++.dg/modules/block-decl-4_b.C: New test.
        * g++.dg/modules/block-decl-4_c.C: New test.

Signed-off-by: Nathaniel Shead <[email protected]>
---
  gcc/cp/class.cc                               |  5 +
  gcc/cp/module.cc                              | 96 ++++++++++++++-----
  gcc/testsuite/g++.dg/modules/block-decl-4_a.C | 70 ++++++++++++++
  gcc/testsuite/g++.dg/modules/block-decl-4_b.C |  6 ++
  gcc/testsuite/g++.dg/modules/block-decl-4_c.C |  8 ++
  5 files changed, 160 insertions(+), 25 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-4_a.C
  create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-4_b.C
  create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-4_c.C

diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 2a65ffb1c00..faf42c7979d 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -8348,6 +8348,11 @@ finish_struct (tree t, tree attributes)
        && !LAMBDA_TYPE_P (t))
      add_stmt (build_min (TAG_DEFN, t));
+ /* Lambdas will be keyed by their LAMBDA_TYPE_EXTRA_SCOPE when that
+     gets set; other local types might need keying anyway though.  */
+  if (at_function_scope_p () && !LAMBDA_TYPE_P (t))
+    maybe_key_decl (current_scope (), TYPE_NAME (t));
+
    return t;
  }
  
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 37b288767ab..e40ffec4b75 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11603,7 +11603,12 @@ trees_out::get_merge_kind (tree decl, depset *dep)
            gcc_checking_assert
              (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl)));
- mk = MK_local_type;
+           if (has_definition (ctx))
+             mk = MK_local_type;
+           else
+             /* We're not providing a definition of the context to key
+                the local type into; use the keyed map instead.  */
+             mk = MK_keyed;
            break;
case RECORD_TYPE:
@@ -12243,7 +12248,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree 
decl, tree inner,
               && DECL_MODULE_KEYED_DECLS_P (name))
        {
          gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL
-                              || TREE_CODE (container) == TYPE_DECL);
+                              || TREE_CODE (container) == TYPE_DECL
+                              || TREE_CODE (container) == FUNCTION_DECL);
          if (auto *set = keyed_table->get (name))
            if (key.index < set->length ())
              {
@@ -22046,23 +22052,12 @@ check_module_decl_linkage (tree decl)
      }
  }
-/* DECL is keyed to CTX for odr purposes. */
+/* Given a scope CTX, find the scope we want to attach the key to,
+   or NULL if no key scope is required.  */
-void
-maybe_key_decl (tree ctx, tree decl)
+static tree
+get_key_scope (tree ctx)
  {
-  if (!modules_p ())
-    return;
-
-  /* We only need to deal with lambdas attached to var, field,
-     parm, type, or concept decls.  */
-  if (TREE_CODE (ctx) != VAR_DECL
-      && TREE_CODE (ctx) != FIELD_DECL
-      && TREE_CODE (ctx) != PARM_DECL
-      && TREE_CODE (ctx) != TYPE_DECL
-      && TREE_CODE (ctx) != CONCEPT_DECL)
-    return;
-
    /* For members, key it to the containing type to handle deduplication
       correctly.  For fields, this is necessary as FIELD_DECLs have no
       dep and so would only be streamed after the lambda type, defeating
@@ -22077,8 +22072,50 @@ maybe_key_decl (tree ctx, tree decl)
       Perhaps sort_cluster can be adjusted to handle this better, but
       this is a simple workaround (and might down on the number of
       entries in keyed_table as a bonus).  */
-  while (DECL_CLASS_SCOPE_P (ctx))
-    ctx = TYPE_NAME (DECL_CONTEXT (ctx));
+  while (!DECL_NAMESPACE_SCOPE_P (ctx))
+    if (DECL_CLASS_SCOPE_P (ctx))
+      ctx = TYPE_NAME (DECL_CONTEXT (ctx));
+    else
+      ctx = DECL_CONTEXT (ctx);
+
+  return ctx;
+}
+
+/* DECL is keyed to CTX for odr purposes.  */
+
+void
+maybe_key_decl (tree ctx, tree decl)
+{
+  if (!modules_p ())
+    return;
+
+  /* We only need to deal here with decls attached to var, field,
+     parm, type, function, or concept decls.  */
+  if (TREE_CODE (ctx) != VAR_DECL
+      && TREE_CODE (ctx) != FIELD_DECL
+      && TREE_CODE (ctx) != PARM_DECL
+      && TREE_CODE (ctx) != TYPE_DECL
+      && TREE_CODE (ctx) != FUNCTION_DECL
+      && TREE_CODE (ctx) != CONCEPT_DECL)
+    return;
+
+  gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (decl))
+                      || TREE_CODE (ctx) == FUNCTION_DECL);
+
+  /* We don't need to use the keyed map for functions with definitions,
+     as we can instead use the MK_local_type handling for streaming.  */
+  if (TREE_CODE (ctx) == FUNCTION_DECL
+      && (has_definition (ctx)
+         /* If we won't be streaming this definition there's also no
+            need to record the key, as it will not be useful for merging
+            (this function is non-inline and so a matching declaration
+            will always be an ODR violation anyway).  */
+         || !module_maybe_has_cmi_p ()))
+    return;
+
+  ctx = get_key_scope (ctx);
+  if (!ctx)
+    return;
if (!keyed_table)
      keyed_table = new keyed_map_t (EXPERIMENT (1, 400));
@@ -22089,16 +22126,23 @@ maybe_key_decl (tree ctx, tree decl)
        retrofit_lang_decl (ctx);
        DECL_MODULE_KEYED_DECLS_P (ctx) = true;
      }
+  if (CHECKING_P)
+    for (tree t : vec)
+      gcc_checking_assert (t != decl);
+
    vec.safe_push (decl);
  }
-/* Find the scope that the lambda DECL is keyed to, if any. */
+/* Find the scope that the local type or 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));
+  gcc_checking_assert (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl)));
+
+  tree scope = (LAMBDA_TYPE_P (TREE_TYPE (decl))
+               ? LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl))
+               : CP_DECL_CONTEXT (decl));
    if (!scope)
      return NULL_TREE;
@@ -22106,12 +22150,14 @@ get_keyed_decl_scope (tree decl)
                       || TREE_CODE (scope) == FIELD_DECL
                       || TREE_CODE (scope) == PARM_DECL
                       || TREE_CODE (scope) == TYPE_DECL
+                      || (TREE_CODE (scope) == FUNCTION_DECL
+                          && !has_definition (scope))
                       || TREE_CODE (scope) == CONCEPT_DECL);
- while (DECL_CLASS_SCOPE_P (scope))
-    scope = TYPE_NAME (DECL_CONTEXT (scope));
+  scope = get_key_scope (scope);
- gcc_checking_assert (DECL_LANG_SPECIFIC (scope)
+  gcc_checking_assert (scope
+                      && DECL_LANG_SPECIFIC (scope)
                       && DECL_MODULE_KEYED_DECLS_P (scope));
    return scope;
  }
diff --git a/gcc/testsuite/g++.dg/modules/block-decl-4_a.C 
b/gcc/testsuite/g++.dg/modules/block-decl-4_a.C
new file mode 100644
index 00000000000..931facc1d70
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/block-decl-4_a.C
@@ -0,0 +1,70 @@
+// PR c++/123627
+// { dg-additional-options "-fmodules -fdump-lang-module-alias" }
+// { dg-module-cmi m:part }
+
+export module m:part;
+
+auto foo() {
+  struct S {} s;
+  return s;
+}
+
+inline auto inline_foo() {
+  struct S {} s;
+  return s;
+}
+
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s attached merge key 
type_decl:'::foo::S'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s local type merge key 
type_decl:'::inline_foo::S'} module } }
+
+auto bar() {
+  auto lambda = []{};
+  return lambda;
+}
+
+inline auto inline_bar() {
+  auto lambda = []{};
+  return lambda;
+}
+
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s attached merge key 
type_decl:'::bar::._anon_[0-9]*'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s local type merge key 
type_decl:'::inline_bar::._anon_[0-9]*'} module } }
+
+auto qux() {
+  struct XN {
+    auto inner() {
+      struct YNN {} y;
+      return y;
+    }
+    inline auto inline_inner() {
+      struct YNI {} y;
+      return y;
+    }
+  } x;
+  return x;
+}
+
+inline auto inline_qux() {
+  struct XI {
+    auto inner() {
+      struct YIN {} y;
+      return y;
+    }
+    inline auto inline_inner() {
+      struct YII {} y;
+      return y;
+    }
+  } x;
+  return x;
+}
+
+// If the innermost function has no definition ('N'), merge via atached key 
decls;
+// even if an outer function does have an inline body, we can't see this 
definition.
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s attached merge key 
type_decl:'::qux::XN'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s attached merge key 
type_decl:'::qux::XN::inner::YNN'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s attached merge key 
type_decl:'::inline_qux::XI::inner::YIN'} module } }
+
+// But if the innermost function has an emitted definition then we can use it.
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s local type merge key 
type_decl:'::qux::XN::inline_inner::YNI'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s local type merge key 
type_decl:'::inline_qux::XI'} module } }
+// { dg-final { scan-lang-dump {Wrote:-[0-9]*'s local type merge key 
type_decl:'::inline_qux::XI::inline_inner::YII'} module } }
diff --git a/gcc/testsuite/g++.dg/modules/block-decl-4_b.C 
b/gcc/testsuite/g++.dg/modules/block-decl-4_b.C
new file mode 100644
index 00000000000..d9889270a7f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/block-decl-4_b.C
@@ -0,0 +1,6 @@
+// PR c++/123627
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi m }
+
+export module m;
+export import :part;
diff --git a/gcc/testsuite/g++.dg/modules/block-decl-4_c.C 
b/gcc/testsuite/g++.dg/modules/block-decl-4_c.C
new file mode 100644
index 00000000000..19eacae37f6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/block-decl-4_c.C
@@ -0,0 +1,8 @@
+// PR c++/123627
+// { dg-additional-options "-fmodules -fno-module-lazy 
-fdump-lang-module-alias" }
+
+module m;
+import :part;
+
+// { dg-final { scan-lang-dump-times {Read:-[0-9]*'s attached merge key 
\(matched\) type_decl} 5 module } }
+// { dg-final { scan-lang-dump-times {Read:-[0-9]*'s local type merge key 
\(matched\) type_decl} 5 module } }

Reply via email to