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

[Patrick Palka <ppalka@redhat.com>]

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.)

-- >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>;
+}
-- 
2.36.1.210.g2668e3608e