c++: template friend with variadic constraints [PR108066]
Checks
Commit Message
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
Comments
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" }
> +}
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" }
> > +}
>
>
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" }
> > > +}
> >
> >
>
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*>;
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*>;
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*>;
@@ -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,
new file mode 100644
@@ -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" }
+}