diff mbox series

c++/modules: Ensure all partial specialisations are tracked [PR114947]

Message ID 6640c449.170a0220.9790b.fc84@mx.google.com
State New
Headers show
Series c++/modules: Ensure all partial specialisations are tracked [PR114947] | expand

Commit Message

Nathaniel Shead May 12, 2024, 1:29 p.m. UTC 
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 <nathanieloshead@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

Comments

Nathaniel Shead May 23, 2024, 1:33 p.m. UTC | #1 
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 <nathanieloshead@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
>
diff mbox series

Patch

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 ()
+      && 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;