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

Reply via email to