* [PATCH] c++: template friend with variadic constraints [PR108066] @ 2022-12-12 17:20 Patrick Palka 2022-12-15 16:22 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2022-12-12 17:20 UTC (permalink / raw) To: gcc-patches; +Cc: jason, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-12 17:20 [PATCH] c++: template friend with variadic constraints [PR108066] Patrick Palka @ 2022-12-15 16:22 ` Jason Merrill 2022-12-15 19:04 ` Patrick Palka 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2022-12-15 16:22 UTC (permalink / raw) To: Patrick Palka, gcc-patches 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" } > +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-15 16:22 ` Jason Merrill @ 2022-12-15 19:04 ` Patrick Palka 2022-12-15 19:31 ` Patrick Palka 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2022-12-15 19:04 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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" } > > +} > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-15 19:04 ` Patrick Palka @ 2022-12-15 19:31 ` Patrick Palka 2022-12-15 22:29 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2022-12-15 19:31 UTC (permalink / raw) To: Patrick Palka; +Cc: Jason Merrill, gcc-patches 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" } > > > +} > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-15 19:31 ` Patrick Palka @ 2022-12-15 22:29 ` Jason Merrill 2022-12-22 17:34 ` Patrick Palka 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2022-12-15 22:29 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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*>; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-15 22:29 ` Jason Merrill @ 2022-12-22 17:34 ` Patrick Palka 2022-12-22 21:00 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2022-12-22 17:34 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: template friend with variadic constraints [PR108066] 2022-12-22 17:34 ` Patrick Palka @ 2022-12-22 21:00 ` Jason Merrill 0 siblings, 0 replies; 7+ messages in thread From: Jason Merrill @ 2022-12-22 21:00 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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*>; ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-22 21:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-12 17:20 [PATCH] c++: template friend with variadic constraints [PR108066] Patrick Palka 2022-12-15 16:22 ` Jason Merrill 2022-12-15 19:04 ` Patrick Palka 2022-12-15 19:31 ` Patrick Palka 2022-12-15 22:29 ` Jason Merrill 2022-12-22 17:34 ` Patrick Palka 2022-12-22 21:00 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).