c++/modules: Really always track partial specialisations [PR116496]

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

My last fix for this issue (PR c++/114947, r15-810) didn't go far
enough; I had assumed that the issue where we lost track of partial
specialisations we would need to walk again later was limited to
partitions (where we always re-walk all specialisations), but the linked
PR is the same cause but for header units, and it is possible to
construct test cases exposing the same bug just for normal modules.

As such this patch just unconditionally ensures that whenever we modify
DECL_TEMPLATE_SPECIALIZATIONS we also track any partial specialisations
that might have added.

Also clean up a couple of comments and assertions to make expected state
more obvious when processing these specs.

	PR c++/116496

gcc/cp/ChangeLog:

	* module.cc (trees_in::decl_value): Don't call
	set_defining_module_for_partial_spec here.
	(depset::hash::add_partial_entities): Clarity assertions.
	* pt.cc (add_mergeable_specialization): Always call
	set_defining_module_for_partial_spec when adding a partial spec.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/partial-5_a.C: New test.
	* g++.dg/modules/partial-5_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                           | 25 +++++++++++-----------
 gcc/cp/pt.cc                               |  1 +
 gcc/testsuite/g++.dg/modules/partial-5_a.C |  9 ++++++++
 gcc/testsuite/g++.dg/modules/partial-5_b.C |  9 ++++++++
 4 files changed, 31 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-5_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-5_b.C

On 9/11/24 8:51 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> My last fix for this issue (PR c++/114947, r15-810) didn't go far
> enough; I had assumed that the issue where we lost track of partial
> specialisations we would need to walk again later was limited to
> partitions (where we always re-walk all specialisations), but the linked
> PR is the same cause but for header units, and it is possible to
> construct test cases exposing the same bug just for normal modules.
> 
> As such this patch just unconditionally ensures that whenever we modify
> DECL_TEMPLATE_SPECIALIZATIONS we also track any partial specialisations
> that might have added.
> 
> Also clean up a couple of comments and assertions to make expected state
> more obvious when processing these specs.
> 
> 	PR c++/116496
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_in::decl_value): Don't call
> 	set_defining_module_for_partial_spec here.
> 	(depset::hash::add_partial_entities): Clarity assertions.
> 	* pt.cc (add_mergeable_specialization): Always call
> 	set_defining_module_for_partial_spec when adding a partial spec.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/partial-5_a.C: New test.
> 	* g++.dg/modules/partial-5_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                           | 25 +++++++++++-----------
>   gcc/cp/pt.cc                               |  1 +
>   gcc/testsuite/g++.dg/modules/partial-5_a.C |  9 ++++++++
>   gcc/testsuite/g++.dg/modules/partial-5_b.C |  9 ++++++++
>   4 files changed, 31 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-5_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-5_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 647208944da..eedcd0ec076 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8434,11 +8434,6 @@ 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)
> @@ -13356,16 +13351,20 @@ depset::hash::add_partial_entities (vec<tree, va_gc> *partial_classes)
>   	     specialization.  */
>   	  gcc_checking_assert (dep->get_entity_kind ()
>   			       == depset::EK_PARTIAL);
> +
> +	  /* Only emit GM entities if reached.  */
> +	  if (!DECL_LANG_SPECIFIC (inner)
> +	      || !DECL_MODULE_PURVIEW_P (inner))
> +	    dep->set_flag_bit<DB_UNREACHED_BIT> ();
>   	}
>         else
> -	/* It was an explicit specialization, not a partial one.  */
> -	gcc_checking_assert (dep->get_entity_kind ()
> -			     == depset::EK_SPECIALIZATION);
> -
> -      /* Only emit GM entities if reached.  */
> -      if (!DECL_LANG_SPECIFIC (inner)
> -	  || !DECL_MODULE_PURVIEW_P (inner))
> -	dep->set_flag_bit<DB_UNREACHED_BIT> ();
> +	{
> +	  /* It was an explicit specialization, not a partial one.
> +	     We should have already added this.  */
> +	  gcc_checking_assert (dep->get_entity_kind ()
> +			       == depset::EK_SPECIALIZATION);
> +	  gcc_checking_assert (dep->is_special ());
> +	}
>       }
>   }
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 9195a5274e1..b8dd7e3a0ee 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -31685,6 +31685,7 @@ add_mergeable_specialization (bool decl_p, spec_entry *elt, tree decl,
>   			     DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl));
>         TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec;
>         DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons;
> +      set_defining_module_for_partial_spec (STRIP_TEMPLATE (decl));
>       }
>   }
>   
> diff --git a/gcc/testsuite/g++.dg/modules/partial-5_a.C b/gcc/testsuite/g++.dg/modules/partial-5_a.C
> new file mode 100644
> index 00000000000..768e6995f0f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-5_a.C
> @@ -0,0 +1,9 @@
> +// PR c++/116496
> +// { dg-additional-options "-fmodules-ts -std=c++20 -Wno-global-module" }
> +// { dg-module-cmi A }
> +
> +module;
> +template <typename T> struct S {};
> +export module A;
> +template <typename T> struct S<T*> {};
> +template <typename T> requires false struct S<T*> {};
> diff --git a/gcc/testsuite/g++.dg/modules/partial-5_b.C b/gcc/testsuite/g++.dg/modules/partial-5_b.C
> new file mode 100644
> index 00000000000..95401fe8b56
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-5_b.C
> @@ -0,0 +1,9 @@
> +// PR c++/116496
> +// { dg-additional-options "-fmodules-ts -std=c++20 -Wno-global-module" }
> +// { dg-module-cmi B }
> +
> +module;
> +template <typename T> struct S {};
> +export module B;
> +import A;
> +template <typename T> requires true struct S<T*> {};