On Sun, May 12, 2024 at 11:29:39PM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>
> -- >8 --
>
> Constrained partial specialisations aren't all necessarily tracked on
> the instantiation table. The modules code uses a separate
> 'partial_specializations' table to track them instead to ensure that
> they get walked and emitted when emitting a module, but currently this
> does not always happen.
>
> The attached testcase fails in two ways. First, because the partial
> specialisation is just a declaration (and not a definition),
> 'set_defining_module' never ends up getting called on it and so it never
> gets added to the partial specialisation table. We fix this by ensuring
> that when partial specializations are created they always get added, and
> so we never miss one. To prevent adding partial specialisations multiple
> times we split this out as a new function.
>
> The second way it fails is that when exporting the primary interface for
> a module with partitions, we also re-walk the specializations of all
> imported partitions to merge them into a single BMI. So this patch
> ensures that after calling 'match_mergeable_specialization' we also
> ensure that if the name came from a partition it gets added to the
> specialization table so that a dependency is correctly created for it.
>
> PR c++/114947
>
> gcc/cp/ChangeLog:
>
> * cp-tree.h (set_defining_module_for_partial_spec): Declare.
> * module.cc (trees_in::decl_value): Track partial specs coming
> from partitions.
> (set_defining_module): Don't track partial specialisations here
> anymore.
> (set_defining_module_for_partial_spec): New function.
> * pt.cc (process_partial_specialization): Call it.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/partial-4_a.C: New test.
> * g++.dg/modules/partial-4_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> ---
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/module.cc | 22 ++++++++++++++++++----
> gcc/cp/pt.cc | 2 ++
> gcc/testsuite/g++.dg/modules/partial-4_a.C | 8 ++++++++
> gcc/testsuite/g++.dg/modules/partial-4_b.C | 5 +++++
> 5 files changed, 34 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-4_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-4_b.C
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index db098c32f2d..2580bf05fb2 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7418,6 +7418,7 @@ extern unsigned get_importing_module (tree, bool =
> false) ATTRIBUTE_PURE;
> /* Where current instance of the decl got declared/defined/instantiated. */
> extern void set_instantiating_module (tree);
> extern void set_defining_module (tree);
> +extern void set_defining_module_for_partial_spec (tree);
> extern void maybe_key_decl (tree ctx, tree decl);
> extern void propagate_defining_module (tree decl, tree orig);
> extern void remove_defining_module (tree decl);
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 520dd710549..3ca963cb3e9 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8416,6 +8416,11 @@ trees_in::decl_value ()
> add_mergeable_specialization (!is_type, &spec, decl, spec_flags);
> }
>
> + /* When making a CMI from a partition we're going to need to walk
> partial
> + specializations again, so make sure they're tracked. */
> + if (state->is_partition () && (spec_flags & 2))
> + set_defining_module_for_partial_spec (inner);
> +
> if (NAMESPACE_SCOPE_P (decl)
> && (mk == MK_named || mk == MK_unique
> || mk == MK_enum || mk == MK_friend_spec)
> @@ -19246,13 +19251,22 @@ set_defining_module (tree decl)
> vec_safe_push (class_members, decl);
> }
> }
> - else if (DECL_IMPLICIT_TYPEDEF_P (decl)
> - && CLASSTYPE_TEMPLATE_SPECIALIZATION (TREE_TYPE (decl)))
> - /* This is a partial or explicit specialization. */
> - vec_safe_push (partial_specializations, decl);
> }
> }
>
> +/* Also remember DECL if it's a newly declared class template partial
> + specialization, because these are not necessarily added to the
> + instantiation tables. */
> +
> +void
> +set_defining_module_for_partial_spec (tree decl)
> +{
> + if (module_p ()
...but this can be 'module_maybe_has_cmi_p' if my change to
'module_has_cmi_p' to include header units is merged first.
> + && DECL_IMPLICIT_TYPEDEF_P (decl)
> + && CLASSTYPE_TEMPLATE_SPECIALIZATION (TREE_TYPE (decl)))
> + vec_safe_push (partial_specializations, decl);
> +}
> +
> void
> set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
> {
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 1816bfd1f40..6d33bac90b0 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -5456,6 +5456,8 @@ process_partial_specialization (tree decl)
> gcc_checking_assert (!TI_PARTIAL_INFO (tinfo));
> TI_PARTIAL_INFO (tinfo) = build_template_info (tmpl, NULL_TREE);
>
> + set_defining_module_for_partial_spec (decl);
> +
> for (inst = DECL_TEMPLATE_INSTANTIATIONS (maintmpl); inst;
> inst = TREE_CHAIN (inst))
> {
> diff --git a/gcc/testsuite/g++.dg/modules/partial-4_a.C
> b/gcc/testsuite/g++.dg/modules/partial-4_a.C
> new file mode 100644
> index 00000000000..e74bb0ad866
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-4_a.C
> @@ -0,0 +1,8 @@
> +// PR c++/114947
> +// { dg-additional-options "-fmodules-ts -std=c++20" }
> +// { dg-module-cmi M:part }
> +module M:part;
> +
> +template <typename> struct R {};
> +template <typename T> requires false struct R<T> {};
> +template <typename T> requires true struct R<T>;
> diff --git a/gcc/testsuite/g++.dg/modules/partial-4_b.C
> b/gcc/testsuite/g++.dg/modules/partial-4_b.C
> new file mode 100644
> index 00000000000..1c645671a55
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-4_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/114947
> +// { dg-additional-options "-fmodules-ts -std=c++20" }
> +// { dg-module-cmi M }
> +export module M;
> +import :part;
> --
> 2.43.2
>
