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*>;



Reply via email to