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.
(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 } }
--
2.51.0
