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" } > > +} > >