Re: [PATCH] c++: don't substitute TEMPLATE_PARM_CONSTRAINT [PR100374]

[Jason Merrill <jason@redhat.com>]

On 6/2/22 11:40, Patrick Palka wrote:
> On Tue, 31 May 2022, Jason Merrill wrote:
> 
>> On 5/31/22 08:56, Patrick Palka wrote:
>>> On Sun, 29 May 2022, Jason Merrill wrote:
>>>
>>>> On 5/29/22 22:10, Jason Merrill wrote:
>>>>> On 5/27/22 14:05, Patrick Palka wrote:
>>>>>> This makes us avoid substituting into the TEMPLATE_PARM_CONSTRAINT of
>>>>>> each template parameter except as necessary for (friend) declaration
>>>>>> matching, like we already do for the overall
>>>>>> TEMPLATE_PARMS_CONSTRAINTS
>>>>>> of a template parameter list.
>>>>>>
>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
>>>>>> for
>>>>>> trunk and perhaps 12.2?  Also tested on range-v3 and cmcstl2.
>>>>>
>>>>> Are there already tests that cover the friend cases?
>>>
>>> Yes, by cpp2a/concepts-friend{2,3,7}.C I think.
>>>
>>>>
>>>> Also, don't you also need to handle specialization of partial
>>>> instantiations?
>>>
>>> Hmm, do you have an example?  IIUC we call tsubst_friend_function and
>>> tsubst_friend_class only from instantiate_class_template_1, which always
>>> uses the most general template and full template argument set to
>>> instantiate any friend declarations.  So friend declarations are never
>>> partially instantiated I think.  (And IIUC non-friends are irrelevant
>>> here since we don't ever want to substitute their constraints outside of
>>> satisfaction.)
>>
>>  From C++20 CA104:
>>
>>    template <class T> struct A {
>>      template <class U> U f(U) requires C<typename T::type>;
>>      template <class U> U f(U) requires C<T>;
>>    };
>>
>>    // Substitute int for T in above requirements to find match.
>>    template <> template <class U> U A<int>::f(U) requires C<int>  { }
> 
> Aha, thanks.  In this case of declaration matching, it looks like
> determine_specialization ignores all but the trailing requirement
> clause.  I think it's doable if a little messy to precisely handle
> this case but in the meantime it seems we could get 90% of the way there
> by considering the overall constraints instead of just the trailing
> constraints?  Something like the following.
> 
> (Either way, IIUC the tsubst_template_parm change shouldn't affect this
> case at all since determine_specialization uses comp_template_parms
> instead of template_heads_equivalent_p and so it ignores
> TEMPLATE_PARM_CONSTRAINT.)

OK.

> -- >8 --
> 
> Subject: [PATCH] c++: don't substitute TEMPLATE_PARM_CONSTRAINT [PR100374]
> 
> This makes us avoid substituting into the TEMPLATE_PARM_CONSTRAINT of
> each template parameter except as necessary for friend declaration
> matching, like we already do for the overall associated constraints.
> 
> In passing this improves upon the CA104 implementation of explicit
> specialization mathing of a constrained function template inside
> a class template, by considering the overall constraints instead of
> just the trailing constraints.  This allows us to correctly handle the
> first three explicit specializations in concepts-spec2.C below, but
> because we compare the constraints as a whole, it means we incorrectly
> accept the fourth explicit specialization.  For complete correctness,
> we should be using tsubst_each_template_parm_constraint and
> template_parameter_heads_equivalent_p in determine_specialization.
> 
> 	PR c++/100374
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (determine_specialization): Compare overall constraints,
> 	not just the trailing constraints, in the CA104 case.
> 	(tsubst_each_template_parm_constraint): Define.
> 	(tsubst_friend_function): Use it.
> 	(tsubst_friend_class): Use it.
> 	(tsubst_template_parm): Don't substitute TEMPLATE_PARM_CONSTRAINT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-template-parm11.C: New test.
> ---
>   gcc/cp/pt.cc                                  | 41 +++++++++++++++----
>   gcc/testsuite/g++.dg/cpp2a/concepts-spec2.C   | 15 +++++++
>   .../g++.dg/cpp2a/concepts-template-parm11.C   | 20 +++++++++
>   3 files changed, 69 insertions(+), 7 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-spec2.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-template-parm11.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 45dd036c2d5..d867ce8e141 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -184,6 +184,7 @@ static int unify_pack_expansion (tree, tree, tree,
>   				 tree, unification_kind_t, bool, bool);
>   static tree copy_template_args (tree);
>   static tree tsubst_template_parms (tree, tree, tsubst_flags_t);
> +static void tsubst_each_template_parm_constraint (tree, tree, tsubst_flags_t);
>   tree most_specialized_partial_spec (tree, tsubst_flags_t);
>   static tree tsubst_aggr_type (tree, tree, tsubst_flags_t, tree, int);
>   static tree tsubst_arg_types (tree, tree, tree, tsubst_flags_t, tree);
> @@ -2323,8 +2324,8 @@ determine_specialization (tree template_id,
>   	      if (!compparms (fn_arg_types, decl_arg_types))
>   		continue;
>   
> -	      tree freq = get_trailing_function_requirements (fn);
> -	      tree dreq = get_trailing_function_requirements (decl);
> +	      tree freq = get_constraints (fn);
> +	      tree dreq = get_constraints (decl);
>   	      if (!freq != !dreq)
>   		continue;
>   	      if (freq)
> @@ -2333,7 +2334,7 @@ determine_specialization (tree template_id,
>   		     constraint-expression.  */
>   		  tree fargs = DECL_TI_ARGS (fn);
>   		  tsubst_flags_t complain = tf_none;
> -		  freq = tsubst_constraint (freq, fargs, complain, fn);
> +		  freq = tsubst_constraint_info (freq, fargs, complain, fn);
>   		  if (!cp_tree_equal (freq, dreq))
>   		    continue;
>   		}
> @@ -11235,7 +11236,12 @@ tsubst_friend_function (tree decl, tree args)
>         tree parms = DECL_TEMPLATE_PARMS (new_friend);
>         tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
>         treqs = maybe_substitute_reqs_for (treqs, new_friend);
> -      TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
> +      if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
> +	{
> +	  TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
> +	  /* As well as each TEMPLATE_PARM_CONSTRAINT.  */
> +	  tsubst_each_template_parm_constraint (parms, args, tf_warning_or_error);
> +	}
>       }
>   
>     /* The mangled name for the NEW_FRIEND is incorrect.  The function
> @@ -11481,6 +11487,8 @@ tsubst_friend_class (tree friend_tmpl, tree args)
>   	{
>   	  tree parms = tsubst_template_parms (DECL_TEMPLATE_PARMS (friend_tmpl),
>   					      args, tf_warning_or_error);
> +	  tsubst_each_template_parm_constraint (parms, args,
> +						tf_warning_or_error);
>             location_t saved_input_location = input_location;
>             input_location = DECL_SOURCE_LOCATION (friend_tmpl);
>             tree cons = get_constraints (tmpl);
> @@ -11515,6 +11523,8 @@ tsubst_friend_class (tree friend_tmpl, tree args)
>   					   DECL_FRIEND_CONTEXT (friend_tmpl));
>   	      --processing_template_decl;
>   	      set_constraints (tmpl, ci);
> +	      tsubst_each_template_parm_constraint (DECL_TEMPLATE_PARMS (tmpl),
> +						    args, tf_warning_or_error);
>   	    }
>   
>   	  /* Inject this template into the enclosing namspace scope.  */
> @@ -13627,7 +13637,6 @@ tsubst_template_parm (tree t, tree args, tsubst_flags_t complain)
>   
>     default_value = TREE_PURPOSE (t);
>     parm_decl = TREE_VALUE (t);
> -  tree constraint = TEMPLATE_PARM_CONSTRAINTS (t);
>   
>     parm_decl = tsubst (parm_decl, args, complain, NULL_TREE);
>     if (TREE_CODE (parm_decl) == PARM_DECL
> @@ -13635,13 +13644,31 @@ tsubst_template_parm (tree t, tree args, tsubst_flags_t complain)
>       parm_decl = error_mark_node;
>     default_value = tsubst_template_arg (default_value, args,
>   				       complain, NULL_TREE);
> -  constraint = tsubst_constraint (constraint, args, complain, NULL_TREE);
>   
>     tree r = build_tree_list (default_value, parm_decl);
> -  TEMPLATE_PARM_CONSTRAINTS (r) = constraint;
> +  TEMPLATE_PARM_CONSTRAINTS (r) = TEMPLATE_PARM_CONSTRAINTS (t);
>     return r;
>   }
>   
> +/* Substitute in-place the TEMPLATE_PARM_CONSTRAINT of each template
> +   parameter in PARMS for sake of declaration matching.  */
> +
> +static void
> +tsubst_each_template_parm_constraint (tree parms, tree args,
> +				      tsubst_flags_t complain)
> +{
> +  ++processing_template_decl;
> +  for (; parms; parms = TREE_CHAIN (parms))
> +    {
> +      tree level = TREE_VALUE (parms);
> +      for (tree parm : tree_vec_range (level))
> +	TEMPLATE_PARM_CONSTRAINTS (parm)
> +	  = tsubst_constraint (TEMPLATE_PARM_CONSTRAINTS (parm), args,
> +			       complain, NULL_TREE);
> +    }
> +  --processing_template_decl;
> +}
> +
>   /* Substitute the ARGS into the indicated aggregate (or enumeration)
>      type T.  If T is not an aggregate or enumeration type, it is
>      handled as if by tsubst.  IN_DECL is as for tsubst.  If
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-spec2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-spec2.C
> new file mode 100644
> index 00000000000..e95a5c10c9c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-spec2.C
> @@ -0,0 +1,15 @@
> +// { dg-do compile { target c++20 } }
> +
> +template<class T, int> concept C = true;
> +
> +template<class T> struct A {
> +  template<C<sizeof(T)> U> void f();
> +  template<C<0> U>         void f();
> +  template<C<-1> U>        void f();
> +};
> +
> +constexpr int n = sizeof(int);
> +template<> template<C<n> U>  void A<int>::f() { }
> +template<> template<C<0> U>  void A<int>::f() { }
> +template<> template<C<-2> U> void A<int>::f() { } // { dg-error "match" }
> +template<> template<class U> void A<int>::f() requires C<U, -1> { } // { dg-error "match" "" { xfail *-*-* } }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-template-parm11.C b/gcc/testsuite/g++.dg/cpp2a/concepts-template-parm11.C
> new file mode 100644
> index 00000000000..498e3c175cf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-template-parm11.C
> @@ -0,0 +1,20 @@
> +// PR c++/100374
> +// { dg-do compile { target c++20 } }
> +
> +template<class T, class U>
> +concept C = requires { typename T; };
> +
> +template<class T>
> +struct A {
> +  template<C<typename T::value_type> U>
> +  void f();
> +
> +  template<C<typename T::value_type> U>
> +  struct B;
> +};
> +
> +int main() {
> +  A<int> a;
> +  a.f<void>();
> +  using type = A<int>::B<void>;
> +}