https://gcc.gnu.org/g:e9e3383f86425f10d3a9bd4daac6e62e8f8176f0
commit r16-6853-ge9e3383f86425f10d3a9bd4daac6e62e8f8176f0 Author: Nathaniel Shead <[email protected]> Date: Sat Jan 17 10:41:58 2026 +1100 c++/modules: Fix local type handling when not streaming function definitions [PR123627] 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. (adjust_key_scope): New function. (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]> Reviewed-by: Jason Merrill <[email protected]> Diff: --- gcc/cp/class.cc | 5 ++ gcc/cp/module.cc | 94 ++++++++++++++++++++------- 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, 158 insertions(+), 25 deletions(-) diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index 2a65ffb1c009..faf42c7979d4 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 37b288767ab2..c786519c1c0a 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 +adjust_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,48 @@ 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 = adjust_key_scope (ctx); if (!keyed_table) keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); @@ -22089,16 +22124,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 +22148,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 = adjust_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 000000000000..931facc1d70e --- /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 000000000000..d9889270a7fe --- /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 000000000000..19eacae37f67 --- /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 } }
