[PATCH] c++: template friend with variadic constraints [PR108066]

[PATCH] c++: template friend with variadic constraints [PR108066]

[Patrick Palka]

When instantiating a constrained hidden template friend, we need to
substitute into its constraints for sake of declaration matching.  For
this substitution we use a full argument vector whose outer levels
correspond to the class's arguments and innermost level corresponds to
the template's level-lowered generic arguments.

But for A<int>::f here, for which the relevant argument vector is
{{int}, {Us...}}, this substitution triggers the assert in
use_pack_expansion_extra_args_p since one argument is a pack expansion
and the other isn't.

And for A<int, int>::f, for which the relevant argument vector is
{{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
also trigger but we first get a bogus "mismatched argument pack lengths"
error from tsubst_pack_expansion.

These might ultimately be bugs in tsubst_pack_expansion, but it seems
we can work around them by tweaking the constraint substitution in
maybe_substitute_reqs_for to only use the friend's outer arguments in
the case of a template friend.  This should be otherwise equivalent to
substituting using the full arguments, since the template's innermost
arguments are just its generic arguments with level=1.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/12?

	PR c++/108066
	PR c++/108067

gcc/cp/ChangeLog:

	* constraint.cc (maybe_substitute_reqs_for): For a template
	friend, substitute using only its outer arguments.  Remove
	dead uses_template_parms test.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-friend12.C: New test.
---
 gcc/cp/constraint.cc                          |  8 +++++++
 .../g++.dg/cpp2a/concepts-friend12.C          | 22 +++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index d4cd92ec3b4..f9d1009c9fe 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl)
       tree tmpl = DECL_TI_TEMPLATE (decl);
       tree gargs = generic_targs_for (tmpl);
       processing_template_decl_sentinel s;
+      if (PRIMARY_TEMPLATE_P (tmpl))
+	{
+	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
+	    return reqs;
+	  ++processing_template_decl;
+	  gargs = copy_node (gargs);
+	  --TREE_VEC_LENGTH (gargs);
+	}
       if (uses_template_parms (gargs))
 	++processing_template_decl;
       reqs = tsubst_constraint (reqs, gargs,
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
new file mode 100644
index 00000000000..95973842afb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
@@ -0,0 +1,22 @@
+// PR c++/108066
+// PR c++/108067
+// { dg-do compile { target c++20 } }
+
+template<class T, class U>
+concept C = __is_same(T, U);
+
+template<class... Ts>
+struct A {
+  template<class... Us>
+    requires (... && C<Ts, Us>)
+  friend void f(A, A<Us...>) { }
+};
+
+int main() {
+  A<int> x;
+  f(x, x);
+  A<int, int> y;
+  f(y, y);
+  A<char> z;
+  f(x, z); // { dg-error "no match" }
+}
-- 
2.39.0.rc2.23.g694cb1b2ab

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Jason Merrill]

On 12/12/22 12:20, Patrick Palka wrote:
> When instantiating a constrained hidden template friend, we need to
> substitute into its constraints for sake of declaration matching.

Hmm, we shouldn't need to do declaration matching when there's only a 
single declaration.

> For
> this substitution we use a full argument vector whose outer levels
> correspond to the class's arguments and innermost level corresponds to
> the template's level-lowered generic arguments.
> 
> But for A<int>::f here, for which the relevant argument vector is
> {{int}, {Us...}}, this substitution triggers the assert in
> use_pack_expansion_extra_args_p since one argument is a pack expansion
> and the other isn't.
> 
> And for A<int, int>::f, for which the relevant argument vector is
> {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
> also trigger but we first get a bogus "mismatched argument pack lengths"
> error from tsubst_pack_expansion.
> 
> These might ultimately be bugs in tsubst_pack_expansion, but it seems
> we can work around them by tweaking the constraint substitution in
> maybe_substitute_reqs_for to only use the friend's outer arguments in
> the case of a template friend.

Yes, this is how we normally substitute a member template during class 
instantiation: with the class' template args, not its own.  The assert 
seems to be enforcing that.

> This should be otherwise equivalent to
> substituting using the full arguments, since the template's innermost
> arguments are just its generic arguments with level=1.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?
> 
> 	PR c++/108066
> 	PR c++/108067
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (maybe_substitute_reqs_for): For a template
> 	friend, substitute using only its outer arguments.  Remove
> 	dead uses_template_parms test.

I don't see any removal?

> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-friend12.C: New test.
> ---
>   gcc/cp/constraint.cc                          |  8 +++++++
>   .../g++.dg/cpp2a/concepts-friend12.C          | 22 +++++++++++++++++++
>   2 files changed, 30 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index d4cd92ec3b4..f9d1009c9fe 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl)
>         tree tmpl = DECL_TI_TEMPLATE (decl);
>         tree gargs = generic_targs_for (tmpl);
>         processing_template_decl_sentinel s;
> +      if (PRIMARY_TEMPLATE_P (tmpl))
> +	{
> +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> +	    return reqs;
> +	  ++processing_template_decl;
> +	  gargs = copy_node (gargs);
> +	  --TREE_VEC_LENGTH (gargs);

Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the 
targs for DECL_FRIEND_CONTEXT instead of decl itself?

> +	}
>         if (uses_template_parms (gargs))
>   	++processing_template_decl;
>         reqs = tsubst_constraint (reqs, gargs,
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> new file mode 100644
> index 00000000000..95973842afb
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> @@ -0,0 +1,22 @@
> +// PR c++/108066
> +// PR c++/108067
> +// { dg-do compile { target c++20 } }
> +
> +template<class T, class U>
> +concept C = __is_same(T, U);
> +
> +template<class... Ts>
> +struct A {
> +  template<class... Us>
> +    requires (... && C<Ts, Us>)
> +  friend void f(A, A<Us...>) { }
> +};
> +
> +int main() {
> +  A<int> x;
> +  f(x, x);
> +  A<int, int> y;
> +  f(y, y);
> +  A<char> z;
> +  f(x, z); // { dg-error "no match" }
> +}

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Patrick Palka]

On Thu, 15 Dec 2022, Jason Merrill wrote:

> On 12/12/22 12:20, Patrick Palka wrote:
> > When instantiating a constrained hidden template friend, we need to
> > substitute into its constraints for sake of declaration matching.
> 
> Hmm, we shouldn't need to do declaration matching when there's only a single
> declaration.

Oops, indeed.  It looks like in this case we're not calling
maybe_substitute_reqs_for during declaration matching, but rather from
tsubst_friend_function and specifically for the non-trailing requirements:

  if (TREE_CODE (new_friend) == TEMPLATE_DECL)
    {
      DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
      DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
      DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
        = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));

      /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
         match in decls_match.  */
      tree parms = DECL_TEMPLATE_PARMS (new_friend);
      tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
      treqs = maybe_substitute_reqs_for (treqs, new_friend);
      if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
        {
          TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
          /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
          tsubst_each_template_parm_constraints (parms, args,
                                                 tf_warning_or_error);
        }
    }

I'll adjust the commit message appropriately.

> 
> > For
> > this substitution we use a full argument vector whose outer levels
> > correspond to the class's arguments and innermost level corresponds to
> > the template's level-lowered generic arguments.
> > 
> > But for A<int>::f here, for which the relevant argument vector is
> > {{int}, {Us...}}, this substitution triggers the assert in
> > use_pack_expansion_extra_args_p since one argument is a pack expansion
> > and the other isn't.
> > 
> > And for A<int, int>::f, for which the relevant argument vector is
> > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
> > also trigger but we first get a bogus "mismatched argument pack lengths"
> > error from tsubst_pack_expansion.
> > 
> > These might ultimately be bugs in tsubst_pack_expansion, but it seems
> > we can work around them by tweaking the constraint substitution in
> > maybe_substitute_reqs_for to only use the friend's outer arguments in
> > the case of a template friend.
> 
> Yes, this is how we normally substitute a member template during class
> instantiation: with the class' template args, not its own.  The assert seems
> to be enforcing that.

Ah, makes snese.

> 
> > This should be otherwise equivalent to
> > substituting using the full arguments, since the template's innermost
> > arguments are just its generic arguments with level=1.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/12?
> > 
> > 	PR c++/108066
> > 	PR c++/108067
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* constraint.cc (maybe_substitute_reqs_for): For a template
> > 	friend, substitute using only its outer arguments.  Remove
> > 	dead uses_template_parms test.
> 
> I don't see any removal?

Oops, I reverted that change before posting the patch but forgot to adjust the
ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
it's not necessary to fix the bug.

> 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-friend12.C: New test.
> > ---
> >   gcc/cp/constraint.cc                          |  8 +++++++
> >   .../g++.dg/cpp2a/concepts-friend12.C          | 22 +++++++++++++++++++
> >   2 files changed, 30 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index d4cd92ec3b4..f9d1009c9fe 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
> > decl)
> >         tree tmpl = DECL_TI_TEMPLATE (decl);
> >         tree gargs = generic_targs_for (tmpl);
> >         processing_template_decl_sentinel s;
> > +      if (PRIMARY_TEMPLATE_P (tmpl))
> > +	{
> > +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> > +	    return reqs;
> > +	  ++processing_template_decl;
> > +	  gargs = copy_node (gargs);
> > +	  --TREE_VEC_LENGTH (gargs);
> 
> Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
> for DECL_FRIEND_CONTEXT instead of decl itself?

IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template friend
is declared inside a partial specialization:

  template<class T, class U>
  concept C = __is_same(T, U);

  template<class T>
  struct A;

  template<class T>
  struct A<T*> {
    template<class U>
      requires (__is_same(T, U))
    friend void f() { };
  };

  template struct A<int*>;

The DECL_FRIEND_CONTEXT of f is A<int*>, whose TYPE_TI_ARGS are {int*},
relative to the primary template instead of the partial specialization.  So
we'd wrongly end up substituting T=int* instead of T=int into f's
TEMPLATE_PARMS_CONSTRAINTS.

r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into
class-scope deduction guides declared inside a partial specialization.

> 
> > +	}
> >         if (uses_template_parms (gargs))
> >   	++processing_template_decl;
> >         reqs = tsubst_constraint (reqs, gargs,
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > new file mode 100644
> > index 00000000000..95973842afb
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > @@ -0,0 +1,22 @@
> > +// PR c++/108066
> > +// PR c++/108067
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T, class U>
> > +concept C = __is_same(T, U);
> > +
> > +template<class... Ts>
> > +struct A {
> > +  template<class... Us>
> > +    requires (... && C<Ts, Us>)
> > +  friend void f(A, A<Us...>) { }
> > +};
> > +
> > +int main() {
> > +  A<int> x;
> > +  f(x, x);
> > +  A<int, int> y;
> > +  f(y, y);
> > +  A<char> z;
> > +  f(x, z); // { dg-error "no match" }
> > +}
> 
> 

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Patrick Palka]

On Thu, 15 Dec 2022, Patrick Palka wrote:

> On Thu, 15 Dec 2022, Jason Merrill wrote:
> 
> > On 12/12/22 12:20, Patrick Palka wrote:
> > > When instantiating a constrained hidden template friend, we need to
> > > substitute into its constraints for sake of declaration matching.
> > 
> > Hmm, we shouldn't need to do declaration matching when there's only a single
> > declaration.
> 
> Oops, indeed.  It looks like in this case we're not calling
> maybe_substitute_reqs_for during declaration matching, but rather from
> tsubst_friend_function and specifically for the non-trailing requirements:
> 
>   if (TREE_CODE (new_friend) == TEMPLATE_DECL)
>     {
>       DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
>       DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
>       DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
>         = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));
> 
>       /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
>          match in decls_match.  */
>       tree parms = DECL_TEMPLATE_PARMS (new_friend);
>       tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
>       treqs = maybe_substitute_reqs_for (treqs, new_friend);
>       if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
>         {
>           TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
>           /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
>           tsubst_each_template_parm_constraints (parms, args,
>                                                  tf_warning_or_error);
>         }
>     }
> 
> I'll adjust the commit message appropriately.
> 
> > 
> > > For
> > > this substitution we use a full argument vector whose outer levels
> > > correspond to the class's arguments and innermost level corresponds to
> > > the template's level-lowered generic arguments.
> > > 
> > > But for A<int>::f here, for which the relevant argument vector is
> > > {{int}, {Us...}}, this substitution triggers the assert in
> > > use_pack_expansion_extra_args_p since one argument is a pack expansion
> > > and the other isn't.
> > > 
> > > And for A<int, int>::f, for which the relevant argument vector is
> > > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
> > > also trigger but we first get a bogus "mismatched argument pack lengths"
> > > error from tsubst_pack_expansion.
> > > 
> > > These might ultimately be bugs in tsubst_pack_expansion, but it seems
> > > we can work around them by tweaking the constraint substitution in
> > > maybe_substitute_reqs_for to only use the friend's outer arguments in
> > > the case of a template friend.
> > 
> > Yes, this is how we normally substitute a member template during class
> > instantiation: with the class' template args, not its own.  The assert seems
> > to be enforcing that.
> 
> Ah, makes snese.
> 
> > 
> > > This should be otherwise equivalent to
> > > substituting using the full arguments, since the template's innermost
> > > arguments are just its generic arguments with level=1.
> > > 
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > trunk/12?
> > > 
> > > 	PR c++/108066
> > > 	PR c++/108067
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* constraint.cc (maybe_substitute_reqs_for): For a template
> > > 	friend, substitute using only its outer arguments.  Remove
> > > 	dead uses_template_parms test.
> > 
> > I don't see any removal?
> 
> Oops, I reverted that change before posting the patch but forgot to adjust the
> ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
> it's not necessary to fix the bug.
> 
> > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/cpp2a/concepts-friend12.C: New test.
> > > ---
> > >   gcc/cp/constraint.cc                          |  8 +++++++
> > >   .../g++.dg/cpp2a/concepts-friend12.C          | 22 +++++++++++++++++++
> > >   2 files changed, 30 insertions(+)
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > 
> > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > index d4cd92ec3b4..f9d1009c9fe 100644
> > > --- a/gcc/cp/constraint.cc
> > > +++ b/gcc/cp/constraint.cc
> > > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
> > > decl)
> > >         tree tmpl = DECL_TI_TEMPLATE (decl);
> > >         tree gargs = generic_targs_for (tmpl);
> > >         processing_template_decl_sentinel s;
> > > +      if (PRIMARY_TEMPLATE_P (tmpl))
> > > +	{
> > > +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> > > +	    return reqs;
> > > +	  ++processing_template_decl;
> > > +	  gargs = copy_node (gargs);
> > > +	  --TREE_VEC_LENGTH (gargs);
> > 
> > Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
> > for DECL_FRIEND_CONTEXT instead of decl itself?
> 
> IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template friend
> is declared inside a partial specialization:
> 
>   template<class T, class U>
>   concept C = __is_same(T, U);
> 
>   template<class T>
>   struct A;
> 
>   template<class T>
>   struct A<T*> {
>     template<class U>
>       requires (__is_same(T, U))
>     friend void f() { };
>   };
> 
>   template struct A<int*>;
> 
> The DECL_FRIEND_CONTEXT of f is A<int*>, whose TYPE_TI_ARGS are {int*},
> relative to the primary template instead of the partial specialization.  So
> we'd wrongly end up substituting T=int* instead of T=int into f's
> TEMPLATE_PARMS_CONSTRAINTS.
> 
> r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into
> class-scope deduction guides declared inside a partial specialization.

Here's a concrete testcase that we'd begin to reject if we used
the args from DECL_FRIEND_CONTEXT for substitution:

  // Verify we substitute the correct outer template arguments
  // when instantiating a constrained template friend.
  // { dg-do compile { target c++20 } }
  
  template<class U>
    requires __is_same(int*, U)
  void f() { };
  
  template<class T>
  struct A;
  
  template<class T>
  struct A<T*> {
    template<class U>
      requires __is_same(T, U)
    friend void f() { } // { dg-bogus "redefinition" }
  };
  
  template struct A<int*>;

> 
> > 
> > > +	}
> > >         if (uses_template_parms (gargs))
> > >   	++processing_template_decl;
> > >         reqs = tsubst_constraint (reqs, gargs,
> > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > new file mode 100644
> > > index 00000000000..95973842afb
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > @@ -0,0 +1,22 @@
> > > +// PR c++/108066
> > > +// PR c++/108067
> > > +// { dg-do compile { target c++20 } }
> > > +
> > > +template<class T, class U>
> > > +concept C = __is_same(T, U);
> > > +
> > > +template<class... Ts>
> > > +struct A {
> > > +  template<class... Us>
> > > +    requires (... && C<Ts, Us>)
> > > +  friend void f(A, A<Us...>) { }
> > > +};
> > > +
> > > +int main() {
> > > +  A<int> x;
> > > +  f(x, x);
> > > +  A<int, int> y;
> > > +  f(y, y);
> > > +  A<char> z;
> > > +  f(x, z); // { dg-error "no match" }
> > > +}
> > 
> > 
> 

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Jason Merrill]

On 12/15/22 14:31, Patrick Palka wrote:
> On Thu, 15 Dec 2022, Patrick Palka wrote:
> 
>> On Thu, 15 Dec 2022, Jason Merrill wrote:
>>
>>> On 12/12/22 12:20, Patrick Palka wrote:
>>>> When instantiating a constrained hidden template friend, we need to
>>>> substitute into its constraints for sake of declaration matching.
>>>
>>> Hmm, we shouldn't need to do declaration matching when there's only a single
>>> declaration.
>>
>> Oops, indeed.  It looks like in this case we're not calling
>> maybe_substitute_reqs_for during declaration matching, but rather from
>> tsubst_friend_function and specifically for the non-trailing requirements:
>>
>>    if (TREE_CODE (new_friend) == TEMPLATE_DECL)
>>      {
>>        DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
>>        DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
>>        DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
>>          = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));
>>
>>        /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
>>           match in decls_match.  */
>>        tree parms = DECL_TEMPLATE_PARMS (new_friend);
>>        tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
>>        treqs = maybe_substitute_reqs_for (treqs, new_friend);
>>        if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
>>          {
>>            TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
>>            /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
>>            tsubst_each_template_parm_constraints (parms, args,
>>                                                   tf_warning_or_error);
>>          }
>>      }
>>
>> I'll adjust the commit message appropriately.
>>
>>>
>>>> For
>>>> this substitution we use a full argument vector whose outer levels
>>>> correspond to the class's arguments and innermost level corresponds to
>>>> the template's level-lowered generic arguments.
>>>>
>>>> But for A<int>::f here, for which the relevant argument vector is
>>>> {{int}, {Us...}}, this substitution triggers the assert in
>>>> use_pack_expansion_extra_args_p since one argument is a pack expansion
>>>> and the other isn't.
>>>>
>>>> And for A<int, int>::f, for which the relevant argument vector is
>>>> {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
>>>> also trigger but we first get a bogus "mismatched argument pack lengths"
>>>> error from tsubst_pack_expansion.
>>>>
>>>> These might ultimately be bugs in tsubst_pack_expansion, but it seems
>>>> we can work around them by tweaking the constraint substitution in
>>>> maybe_substitute_reqs_for to only use the friend's outer arguments in
>>>> the case of a template friend.
>>>
>>> Yes, this is how we normally substitute a member template during class
>>> instantiation: with the class' template args, not its own.  The assert seems
>>> to be enforcing that.
>>
>> Ah, makes snese.
>>
>>>
>>>> This should be otherwise equivalent to
>>>> substituting using the full arguments, since the template's innermost
>>>> arguments are just its generic arguments with level=1.
>>>>
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>>> trunk/12?
>>>>
>>>> 	PR c++/108066
>>>> 	PR c++/108067
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> 	* constraint.cc (maybe_substitute_reqs_for): For a template
>>>> 	friend, substitute using only its outer arguments.  Remove
>>>> 	dead uses_template_parms test.
>>>
>>> I don't see any removal?
>>
>> Oops, I reverted that change before posting the patch but forgot to adjust the
>> ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
>> it's not necessary to fix the bug.
>>
>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	* g++.dg/cpp2a/concepts-friend12.C: New test.
>>>> ---
>>>>    gcc/cp/constraint.cc                          |  8 +++++++
>>>>    .../g++.dg/cpp2a/concepts-friend12.C          | 22 +++++++++++++++++++
>>>>    2 files changed, 30 insertions(+)
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
>>>>
>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>>> index d4cd92ec3b4..f9d1009c9fe 100644
>>>> --- a/gcc/cp/constraint.cc
>>>> +++ b/gcc/cp/constraint.cc
>>>> @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
>>>> decl)
>>>>          tree tmpl = DECL_TI_TEMPLATE (decl);
>>>>          tree gargs = generic_targs_for (tmpl);
>>>>          processing_template_decl_sentinel s;
>>>> +      if (PRIMARY_TEMPLATE_P (tmpl))
>>>> +	{
>>>> +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
>>>> +	    return reqs;
>>>> +	  ++processing_template_decl;
>>>> +	  gargs = copy_node (gargs);
>>>> +	  --TREE_VEC_LENGTH (gargs);
>>>
>>> Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
>>> for DECL_FRIEND_CONTEXT instead of decl itself?
>>
>> IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template friend
>> is declared inside a partial specialization:
>>
>>    template<class T, class U>
>>    concept C = __is_same(T, U);
>>
>>    template<class T>
>>    struct A;
>>
>>    template<class T>
>>    struct A<T*> {
>>      template<class U>
>>        requires (__is_same(T, U))
>>      friend void f() { };
>>    };
>>
>>    template struct A<int*>;
>>
>> The DECL_FRIEND_CONTEXT of f is A<int*>, whose TYPE_TI_ARGS are {int*},
>> relative to the primary template instead of the partial specialization.  So
>> we'd wrongly end up substituting T=int* instead of T=int into f's
>> TEMPLATE_PARMS_CONSTRAINTS.
>>
>> r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into
>> class-scope deduction guides declared inside a partial specialization.

Ah, so this is a workaround for not being able to get at the template 
args used in substituting A<int*> from A<int*> itself.

Maybe factor this out from here and ctor_deduction_guides_for into an 
outer_template_args function with a comment to that effect?

Maybe it would make sense to update CLASSTYPE_TEMPLATE_INFO for A<int*> 
after we choose a partial specialization in instantiate_class_template, 
but I'm sure a bunch of places would need to be adjusted to handle that.

> Here's a concrete testcase that we'd begin to reject if we used
> the args from DECL_FRIEND_CONTEXT for substitution:
> 
>    // Verify we substitute the correct outer template arguments
>    // when instantiating a constrained template friend.
>    // { dg-do compile { target c++20 } }
>    
>    template<class U>
>      requires __is_same(int*, U)
>    void f() { };
>    
>    template<class T>
>    struct A;
>    
>    template<class T>
>    struct A<T*> {
>      template<class U>
>        requires __is_same(T, U)
>      friend void f() { } // { dg-bogus "redefinition" }
>    };
>    
>    template struct A<int*>;

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Patrick Palka]

On Thu, 15 Dec 2022, Jason Merrill wrote:

> On 12/15/22 14:31, Patrick Palka wrote:
> > On Thu, 15 Dec 2022, Patrick Palka wrote:
> > 
> > > On Thu, 15 Dec 2022, Jason Merrill wrote:
> > > 
> > > > On 12/12/22 12:20, Patrick Palka wrote:
> > > > > When instantiating a constrained hidden template friend, we need to
> > > > > substitute into its constraints for sake of declaration matching.
> > > > 
> > > > Hmm, we shouldn't need to do declaration matching when there's only a
> > > > single
> > > > declaration.
> > > 
> > > Oops, indeed.  It looks like in this case we're not calling
> > > maybe_substitute_reqs_for during declaration matching, but rather from
> > > tsubst_friend_function and specifically for the non-trailing requirements:
> > > 
> > >    if (TREE_CODE (new_friend) == TEMPLATE_DECL)
> > >      {
> > >        DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
> > >        DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
> > >        DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
> > >          = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));
> > > 
> > >        /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels
> > > will
> > >           match in decls_match.  */
> > >        tree parms = DECL_TEMPLATE_PARMS (new_friend);
> > >        tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
> > >        treqs = maybe_substitute_reqs_for (treqs, new_friend);
> > >        if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
> > >          {
> > >            TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
> > >            /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
> > >            tsubst_each_template_parm_constraints (parms, args,
> > >                                                   tf_warning_or_error);
> > >          }
> > >      }
> > > 
> > > I'll adjust the commit message appropriately.
> > > 
> > > > 
> > > > > For
> > > > > this substitution we use a full argument vector whose outer levels
> > > > > correspond to the class's arguments and innermost level corresponds to
> > > > > the template's level-lowered generic arguments.
> > > > > 
> > > > > But for A<int>::f here, for which the relevant argument vector is
> > > > > {{int}, {Us...}}, this substitution triggers the assert in
> > > > > use_pack_expansion_extra_args_p since one argument is a pack expansion
> > > > > and the other isn't.
> > > > > 
> > > > > And for A<int, int>::f, for which the relevant argument vector is
> > > > > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert
> > > > > would
> > > > > also trigger but we first get a bogus "mismatched argument pack
> > > > > lengths"
> > > > > error from tsubst_pack_expansion.
> > > > > 
> > > > > These might ultimately be bugs in tsubst_pack_expansion, but it seems
> > > > > we can work around them by tweaking the constraint substitution in
> > > > > maybe_substitute_reqs_for to only use the friend's outer arguments in
> > > > > the case of a template friend.
> > > > 
> > > > Yes, this is how we normally substitute a member template during class
> > > > instantiation: with the class' template args, not its own.  The assert
> > > > seems
> > > > to be enforcing that.
> > > 
> > > Ah, makes snese.
> > > 
> > > > 
> > > > > This should be otherwise equivalent to
> > > > > substituting using the full arguments, since the template's innermost
> > > > > arguments are just its generic arguments with level=1.
> > > > > 
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > for
> > > > > trunk/12?
> > > > > 
> > > > > 	PR c++/108066
> > > > > 	PR c++/108067
> > > > > 
> > > > > gcc/cp/ChangeLog:
> > > > > 
> > > > > 	* constraint.cc (maybe_substitute_reqs_for): For a template
> > > > > 	friend, substitute using only its outer arguments.  Remove
> > > > > 	dead uses_template_parms test.
> > > > 
> > > > I don't see any removal?
> > > 
> > > Oops, I reverted that change before posting the patch but forgot to adjust
> > > the
> > > ChangeLog as well.  Removing the uses_template_parms test seems to be safe
> > > but
> > > it's not necessary to fix the bug.
> > > 
> > > > 
> > > > > gcc/testsuite/ChangeLog:
> > > > > 
> > > > > 	* g++.dg/cpp2a/concepts-friend12.C: New test.
> > > > > ---
> > > > >    gcc/cp/constraint.cc                          |  8 +++++++
> > > > >    .../g++.dg/cpp2a/concepts-friend12.C          | 22
> > > > > +++++++++++++++++++
> > > > >    2 files changed, 30 insertions(+)
> > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > > > 
> > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > > > index d4cd92ec3b4..f9d1009c9fe 100644
> > > > > --- a/gcc/cp/constraint.cc
> > > > > +++ b/gcc/cp/constraint.cc
> > > > > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs,
> > > > > const_tree
> > > > > decl)
> > > > >          tree tmpl = DECL_TI_TEMPLATE (decl);
> > > > >          tree gargs = generic_targs_for (tmpl);
> > > > >          processing_template_decl_sentinel s;
> > > > > +      if (PRIMARY_TEMPLATE_P (tmpl))
> > > > > +	{
> > > > > +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> > > > > +	    return reqs;
> > > > > +	  ++processing_template_decl;
> > > > > +	  gargs = copy_node (gargs);
> > > > > +	  --TREE_VEC_LENGTH (gargs);
> > > > 
> > > > Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the
> > > > targs
> > > > for DECL_FRIEND_CONTEXT instead of decl itself?
> > > 
> > > IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template
> > > friend
> > > is declared inside a partial specialization:
> > > 
> > >    template<class T, class U>
> > >    concept C = __is_same(T, U);
> > > 
> > >    template<class T>
> > >    struct A;
> > > 
> > >    template<class T>
> > >    struct A<T*> {
> > >      template<class U>
> > >        requires (__is_same(T, U))
> > >      friend void f() { };
> > >    };
> > > 
> > >    template struct A<int*>;
> > > 
> > > The DECL_FRIEND_CONTEXT of f is A<int*>, whose TYPE_TI_ARGS are {int*},
> > > relative to the primary template instead of the partial specialization.
> > > So
> > > we'd wrongly end up substituting T=int* instead of T=int into f's
> > > TEMPLATE_PARMS_CONSTRAINTS.
> > > 
> > > r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into
> > > class-scope deduction guides declared inside a partial specialization.
> 
> Ah, so this is a workaround for not being able to get at the template args
> used in substituting A<int*> from A<int*> itself.
> 
> Maybe factor this out from here and ctor_deduction_guides_for into an
> outer_template_args function with a comment to that effect?

Like so?  Bootstrapped and regtested on x86_64-pc-linux-gnu.

> 
> Maybe it would make sense to update CLASSTYPE_TEMPLATE_INFO for A<int*> after
> we choose a partial specialization in instantiate_class_template, but I'm sure
> a bunch of places would need to be adjusted to handle that.

*nod* Having that information readily available instead of having to
call most_specialized_partial_spec would be nice.

-- >8 --


Subject: [PATCH] c++: template friend with variadic constraints [PR107853]

	PR c++/107853

gcc/cp/ChangeLog:

	* constraint.cc (maybe_substitute_reqs_for): For a template
	friend, substitute using only its outer arguments via
	outer_template_args.
	* cp-tree.h (outer_template_args): Declare.
	* pt.cc (outer_template_args): Define, factored out and
	generalized from ...
	(ctor_deduction_guides_for): ... here.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-friend12.C: New test.
	* g++.dg/cpp2a/concepts-friend13.C: New test.
---
 gcc/cp/constraint.cc                          |  7 ++--
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/pt.cc                                  | 35 +++++++++++++------
 .../g++.dg/cpp2a/concepts-friend12.C          | 21 +++++++++++
 .../g++.dg/cpp2a/concepts-friend13.C          | 20 +++++++++++
 5 files changed, 71 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 37eae03afdb..247a8278d40 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1350,11 +1350,12 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl)
   if (DECL_UNIQUE_FRIEND_P (decl) && DECL_TEMPLATE_INFO (decl))
     {
       tree tmpl = DECL_TI_TEMPLATE (decl);
-      tree gargs = generic_targs_for (tmpl);
+      tree outer_args = outer_template_args (tmpl);
       processing_template_decl_sentinel s;
-      if (uses_template_parms (gargs))
+      if (PRIMARY_TEMPLATE_P (tmpl)
+	  || uses_template_parms (outer_args))
 	++processing_template_decl;
-      reqs = tsubst_constraint (reqs, gargs,
+      reqs = tsubst_constraint (reqs, outer_args,
 				tf_warning_or_error, NULL_TREE);
     }
   return reqs;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 541d760492d..1f4967c2ba0 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7056,6 +7056,7 @@ extern tree maybe_set_retval_sentinel		(void);
 extern tree template_parms_to_args		(tree);
 extern tree template_parms_level_to_args	(tree);
 extern tree generic_targs_for			(tree);
+extern tree outer_template_args			(tree);
 
 /* in expr.cc */
 extern tree cplus_expand_constant		(tree);
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index e68c74913f5..c0593e9cac6 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -4958,6 +4958,29 @@ generic_targs_for (tree tmpl)
   return template_parms_to_args (DECL_TEMPLATE_PARMS (tmpl));
 }
 
+/* Return the template arguments corresponding to the template
+   parameters of TMPL's enclosing scope.  When TMPL is a member
+   template of a partial specialization, this returns the arguments
+   for the partial specialization as opposed to those for the primary
+   template, which is the main difference between this function and
+   simply inspecting e.g. the TYPE_TI_ARGS of TMPL's DECL_CONTEXT.  */
+
+tree
+outer_template_args (tree tmpl)
+{
+  tree ti = get_template_info (DECL_TEMPLATE_RESULT (tmpl));
+  if (!ti)
+    return NULL_TREE;
+  tree args = TI_ARGS (ti);
+  if (!PRIMARY_TEMPLATE_P (tmpl))
+    return args;
+  if (TMPL_ARGS_DEPTH (args) == 1)
+    return NULL_TREE;
+  args = copy_node (args);
+  --TREE_VEC_LENGTH (args);
+  return args;
+}
+
 /* Update the declared TYPE by doing any lookups which were thought to be
    dependent, but are not now that we know the SCOPE of the declarator.  */
 
@@ -30081,16 +30104,8 @@ alias_ctad_tweaks (tree tmpl, tree uguides)
 static tree
 ctor_deduction_guides_for (tree tmpl, tsubst_flags_t complain)
 {
-  tree type = TREE_TYPE (tmpl);
-  tree outer_args = NULL_TREE;
-  if (DECL_CLASS_SCOPE_P (tmpl)
-      && CLASSTYPE_TEMPLATE_INSTANTIATION (DECL_CONTEXT (tmpl)))
-    {
-      outer_args = copy_node (CLASSTYPE_TI_ARGS (type));
-      gcc_assert (TMPL_ARGS_DEPTH (outer_args) > 1);
-      --TREE_VEC_LENGTH (outer_args);
-      type = TREE_TYPE (most_general_template (tmpl));
-    }
+  tree outer_args = outer_template_args (tmpl);
+  tree type = TREE_TYPE (most_general_template (tmpl));
 
   tree cands = NULL_TREE;
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
new file mode 100644
index 00000000000..9687be5931a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
@@ -0,0 +1,21 @@
+// PR c++/107853
+// { dg-do compile { target c++20 } }
+
+template<class T, class U>
+concept C = __is_same(T, U);
+
+template<class... Ts>
+struct A {
+  template<class... Us>
+    requires (C<Ts, Us> && ...)
+  friend void f(A, A<Us...>) { }
+};
+
+int main() {
+  A<int> x;
+  f(x, x);
+  A<int, int> y;
+  f(y, y);
+  A<char> z;
+  f(x, z); // { dg-error "no match" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C
new file mode 100644
index 00000000000..3cc4505a7ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C
@@ -0,0 +1,20 @@
+// Verify we substitute the correct outer template arguments
+// when instantiating a constrained template friend declared
+// inside a partial specialization.
+// { dg-do compile { target c++20 } }
+
+template<class U>
+  requires __is_same(int*, U)
+void f() { };
+
+template<class T>
+struct A;
+
+template<class T>
+struct A<T*> {
+  template<class U>
+    requires __is_same(T, U)
+  friend void f() { } // { dg-bogus "redefinition" }
+};
+
+template struct A<int*>;
-- 
2.39.0.95.g7c2ef319c5

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Jason Merrill]

On 12/22/22 12:34, Patrick Palka wrote:
> On Thu, 15 Dec 2022, Jason Merrill wrote:
> 
>> On 12/15/22 14:31, Patrick Palka wrote:
>>> On Thu, 15 Dec 2022, Patrick Palka wrote:
>>>
>>>> On Thu, 15 Dec 2022, Jason Merrill wrote:
>>>>
>>>>> On 12/12/22 12:20, Patrick Palka wrote:
>>>>>> When instantiating a constrained hidden template friend, we need to
>>>>>> substitute into its constraints for sake of declaration matching.
>>>>>
>>>>> Hmm, we shouldn't need to do declaration matching when there's only a
>>>>> single
>>>>> declaration.
>>>>
>>>> Oops, indeed.  It looks like in this case we're not calling
>>>> maybe_substitute_reqs_for during declaration matching, but rather from
>>>> tsubst_friend_function and specifically for the non-trailing requirements:
>>>>
>>>>     if (TREE_CODE (new_friend) == TEMPLATE_DECL)
>>>>       {
>>>>         DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
>>>>         DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
>>>>         DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
>>>>           = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));
>>>>
>>>>         /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels
>>>> will
>>>>            match in decls_match.  */
>>>>         tree parms = DECL_TEMPLATE_PARMS (new_friend);
>>>>         tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
>>>>         treqs = maybe_substitute_reqs_for (treqs, new_friend);
>>>>         if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
>>>>           {
>>>>             TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
>>>>             /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
>>>>             tsubst_each_template_parm_constraints (parms, args,
>>>>                                                    tf_warning_or_error);
>>>>           }
>>>>       }
>>>>
>>>> I'll adjust the commit message appropriately.
>>>>
>>>>>
>>>>>> For
>>>>>> this substitution we use a full argument vector whose outer levels
>>>>>> correspond to the class's arguments and innermost level corresponds to
>>>>>> the template's level-lowered generic arguments.
>>>>>>
>>>>>> But for A<int>::f here, for which the relevant argument vector is
>>>>>> {{int}, {Us...}}, this substitution triggers the assert in
>>>>>> use_pack_expansion_extra_args_p since one argument is a pack expansion
>>>>>> and the other isn't.
>>>>>>
>>>>>> And for A<int, int>::f, for which the relevant argument vector is
>>>>>> {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert
>>>>>> would
>>>>>> also trigger but we first get a bogus "mismatched argument pack
>>>>>> lengths"
>>>>>> error from tsubst_pack_expansion.
>>>>>>
>>>>>> These might ultimately be bugs in tsubst_pack_expansion, but it seems
>>>>>> we can work around them by tweaking the constraint substitution in
>>>>>> maybe_substitute_reqs_for to only use the friend's outer arguments in
>>>>>> the case of a template friend.
>>>>>
>>>>> Yes, this is how we normally substitute a member template during class
>>>>> instantiation: with the class' template args, not its own.  The assert
>>>>> seems
>>>>> to be enforcing that.
>>>>
>>>> Ah, makes snese.
>>>>
>>>>>
>>>>>> This should be otherwise equivalent to
>>>>>> substituting using the full arguments, since the template's innermost
>>>>>> arguments are just its generic arguments with level=1.
>>>>>>
>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
>>>>>> for
>>>>>> trunk/12?
>>>>>>
>>>>>> 	PR c++/108066
>>>>>> 	PR c++/108067
>>>>>>
>>>>>> gcc/cp/ChangeLog:
>>>>>>
>>>>>> 	* constraint.cc (maybe_substitute_reqs_for): For a template
>>>>>> 	friend, substitute using only its outer arguments.  Remove
>>>>>> 	dead uses_template_parms test.
>>>>>
>>>>> I don't see any removal?
>>>>
>>>> Oops, I reverted that change before posting the patch but forgot to adjust
>>>> the
>>>> ChangeLog as well.  Removing the uses_template_parms test seems to be safe
>>>> but
>>>> it's not necessary to fix the bug.
>>>>
>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 	* g++.dg/cpp2a/concepts-friend12.C: New test.
>>>>>> ---
>>>>>>     gcc/cp/constraint.cc                          |  8 +++++++
>>>>>>     .../g++.dg/cpp2a/concepts-friend12.C          | 22
>>>>>> +++++++++++++++++++
>>>>>>     2 files changed, 30 insertions(+)
>>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
>>>>>>
>>>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
>>>>>> index d4cd92ec3b4..f9d1009c9fe 100644
>>>>>> --- a/gcc/cp/constraint.cc
>>>>>> +++ b/gcc/cp/constraint.cc
>>>>>> @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs,
>>>>>> const_tree
>>>>>> decl)
>>>>>>           tree tmpl = DECL_TI_TEMPLATE (decl);
>>>>>>           tree gargs = generic_targs_for (tmpl);
>>>>>>           processing_template_decl_sentinel s;
>>>>>> +      if (PRIMARY_TEMPLATE_P (tmpl))
>>>>>> +	{
>>>>>> +	  if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
>>>>>> +	    return reqs;
>>>>>> +	  ++processing_template_decl;
>>>>>> +	  gargs = copy_node (gargs);
>>>>>> +	  --TREE_VEC_LENGTH (gargs);
>>>>>
>>>>> Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the
>>>>> targs
>>>>> for DECL_FRIEND_CONTEXT instead of decl itself?
>>>>
>>>> IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template
>>>> friend
>>>> is declared inside a partial specialization:
>>>>
>>>>     template<class T, class U>
>>>>     concept C = __is_same(T, U);
>>>>
>>>>     template<class T>
>>>>     struct A;
>>>>
>>>>     template<class T>
>>>>     struct A<T*> {
>>>>       template<class U>
>>>>         requires (__is_same(T, U))
>>>>       friend void f() { };
>>>>     };
>>>>
>>>>     template struct A<int*>;
>>>>
>>>> The DECL_FRIEND_CONTEXT of f is A<int*>, whose TYPE_TI_ARGS are {int*},
>>>> relative to the primary template instead of the partial specialization.
>>>> So
>>>> we'd wrongly end up substituting T=int* instead of T=int into f's
>>>> TEMPLATE_PARMS_CONSTRAINTS.
>>>>
>>>> r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into
>>>> class-scope deduction guides declared inside a partial specialization.
>>
>> Ah, so this is a workaround for not being able to get at the template args
>> used in substituting A<int*> from A<int*> itself.
>>
>> Maybe factor this out from here and ctor_deduction_guides_for into an
>> outer_template_args function with a comment to that effect?
> 
> Like so?  Bootstrapped and regtested on x86_64-pc-linux-gnu.

OK.

>> Maybe it would make sense to update CLASSTYPE_TEMPLATE_INFO for A<int*> after
>> we choose a partial specialization in instantiate_class_template, but I'm sure
>> a bunch of places would need to be adjusted to handle that.
> 
> *nod* Having that information readily available instead of having to
> call most_specialized_partial_spec would be nice.
> 
> -- >8 --
> 
> 
> Subject: [PATCH] c++: template friend with variadic constraints [PR107853]
> 
> 	PR c++/107853
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (maybe_substitute_reqs_for): For a template
> 	friend, substitute using only its outer arguments via
> 	outer_template_args.
> 	* cp-tree.h (outer_template_args): Declare.
> 	* pt.cc (outer_template_args): Define, factored out and
> 	generalized from ...
> 	(ctor_deduction_guides_for): ... here.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-friend12.C: New test.
> 	* g++.dg/cpp2a/concepts-friend13.C: New test.
> ---
>   gcc/cp/constraint.cc                          |  7 ++--
>   gcc/cp/cp-tree.h                              |  1 +
>   gcc/cp/pt.cc                                  | 35 +++++++++++++------
>   .../g++.dg/cpp2a/concepts-friend12.C          | 21 +++++++++++
>   .../g++.dg/cpp2a/concepts-friend13.C          | 20 +++++++++++
>   5 files changed, 71 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 37eae03afdb..247a8278d40 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -1350,11 +1350,12 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl)
>     if (DECL_UNIQUE_FRIEND_P (decl) && DECL_TEMPLATE_INFO (decl))
>       {
>         tree tmpl = DECL_TI_TEMPLATE (decl);
> -      tree gargs = generic_targs_for (tmpl);
> +      tree outer_args = outer_template_args (tmpl);
>         processing_template_decl_sentinel s;
> -      if (uses_template_parms (gargs))
> +      if (PRIMARY_TEMPLATE_P (tmpl)
> +	  || uses_template_parms (outer_args))
>   	++processing_template_decl;
> -      reqs = tsubst_constraint (reqs, gargs,
> +      reqs = tsubst_constraint (reqs, outer_args,
>   				tf_warning_or_error, NULL_TREE);
>       }
>     return reqs;
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 541d760492d..1f4967c2ba0 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7056,6 +7056,7 @@ extern tree maybe_set_retval_sentinel		(void);
>   extern tree template_parms_to_args		(tree);
>   extern tree template_parms_level_to_args	(tree);
>   extern tree generic_targs_for			(tree);
> +extern tree outer_template_args			(tree);
>   
>   /* in expr.cc */
>   extern tree cplus_expand_constant		(tree);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index e68c74913f5..c0593e9cac6 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -4958,6 +4958,29 @@ generic_targs_for (tree tmpl)
>     return template_parms_to_args (DECL_TEMPLATE_PARMS (tmpl));
>   }
>   
> +/* Return the template arguments corresponding to the template
> +   parameters of TMPL's enclosing scope.  When TMPL is a member
> +   template of a partial specialization, this returns the arguments
> +   for the partial specialization as opposed to those for the primary
> +   template, which is the main difference between this function and
> +   simply inspecting e.g. the TYPE_TI_ARGS of TMPL's DECL_CONTEXT.  */
> +
> +tree
> +outer_template_args (tree tmpl)
> +{
> +  tree ti = get_template_info (DECL_TEMPLATE_RESULT (tmpl));
> +  if (!ti)
> +    return NULL_TREE;
> +  tree args = TI_ARGS (ti);
> +  if (!PRIMARY_TEMPLATE_P (tmpl))
> +    return args;
> +  if (TMPL_ARGS_DEPTH (args) == 1)
> +    return NULL_TREE;
> +  args = copy_node (args);
> +  --TREE_VEC_LENGTH (args);
> +  return args;
> +}
> +
>   /* Update the declared TYPE by doing any lookups which were thought to be
>      dependent, but are not now that we know the SCOPE of the declarator.  */
>   
> @@ -30081,16 +30104,8 @@ alias_ctad_tweaks (tree tmpl, tree uguides)
>   static tree
>   ctor_deduction_guides_for (tree tmpl, tsubst_flags_t complain)
>   {
> -  tree type = TREE_TYPE (tmpl);
> -  tree outer_args = NULL_TREE;
> -  if (DECL_CLASS_SCOPE_P (tmpl)
> -      && CLASSTYPE_TEMPLATE_INSTANTIATION (DECL_CONTEXT (tmpl)))
> -    {
> -      outer_args = copy_node (CLASSTYPE_TI_ARGS (type));
> -      gcc_assert (TMPL_ARGS_DEPTH (outer_args) > 1);
> -      --TREE_VEC_LENGTH (outer_args);
> -      type = TREE_TYPE (most_general_template (tmpl));
> -    }
> +  tree outer_args = outer_template_args (tmpl);
> +  tree type = TREE_TYPE (most_general_template (tmpl));
>   
>     tree cands = NULL_TREE;
>   
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> new file mode 100644
> index 00000000000..9687be5931a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> @@ -0,0 +1,21 @@
> +// PR c++/107853
> +// { dg-do compile { target c++20 } }
> +
> +template<class T, class U>
> +concept C = __is_same(T, U);
> +
> +template<class... Ts>
> +struct A {
> +  template<class... Us>
> +    requires (C<Ts, Us> && ...)
> +  friend void f(A, A<Us...>) { }
> +};
> +
> +int main() {
> +  A<int> x;
> +  f(x, x);
> +  A<int, int> y;
> +  f(y, y);
> +  A<char> z;
> +  f(x, z); // { dg-error "no match" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C
> new file mode 100644
> index 00000000000..3cc4505a7ef
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C
> @@ -0,0 +1,20 @@
> +// Verify we substitute the correct outer template arguments
> +// when instantiating a constrained template friend declared
> +// inside a partial specialization.
> +// { dg-do compile { target c++20 } }
> +
> +template<class U>
> +  requires __is_same(int*, U)
> +void f() { };
> +
> +template<class T>
> +struct A;
> +
> +template<class T>
> +struct A<T*> {
> +  template<class U>
> +    requires __is_same(T, U)
> +  friend void f() { } // { dg-bogus "redefinition" }
> +};
> +
> +template struct A<int*>;