[clang] Implement CWG2398 provisional TTP matching to class templates

[mizvekov]

This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling -frelaxed-template-template-args by default.

When performing template argument deduction, we extend the provisional wording introduced in #89807 so it also covers deduction of class templates.

Given the following example:

template <class T1, class T2 = float> struct A;
template <class T3> struct B;

template <template <class T4> class TT1, class T5> struct B<TT1<T5>>;   // #1
template <class T6, class T7>                      struct B<A<T6, T7>>; // #2

template struct B<A<int>>;

Prior to P0522, #2 was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, #2 is picked again.

This has the beneficial side effect of making the following code valid:

template<class T, class U> struct A {};
A<int, float> v;
template<template<class> class TT> void f(TT<int>);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }

---

Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling -frelaxed-template-template-args by default.

When performing template argument deduction, we extend the provisional wording introduced in #89807 so it also covers deduction of class templates.

Given the following example:

template <class T1, class T2 = float> struct A;
template <class T3> struct B;

template &lt;template &lt;class T4&gt; class TT1, class T5&gt; struct B&lt;TT1&lt;T5&gt;&gt;;   // #<!-- -->1
template &lt;class T6, class T7&gt;                      struct B&lt;A&lt;T6, T7&gt;&gt;; // #<!-- -->2

template struct B&lt;A&lt;int&gt;&gt;;

Prior to P0522, #<!-- -->2 was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, #<!-- -->2 is picked again.

This has the beneficial side effect of making the following code valid:

template<class T, class U> struct A {};
A<int, float> v;
template<template<class> class TT> void f(TT<int>);

// OK: TT picks 'float' as the default argument for the second parameter.
void g() { f(v); }

---

Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here.

---

Full diff: https://github.com/llvm/llvm-project/pull/92855.diff

4 Files Affected:

• (modified) clang/lib/Sema/SemaTemplate.cpp (+4-1)
• (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+44-28)
• (modified) clang/test/CXX/temp/temp.decls/temp.alias/p2.cpp (+3-2)
• (modified) clang/test/SemaTemplate/cwg2398.cpp (-3)

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 6af35ac8911bb..b7479cbcdba23 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1807,6 +1807,8 @@ static void SetNestedNameSpecifier(Sema &S, TagDecl *T,
 // Returns the template parameter list with all default template argument
 // information.
 static TemplateParameterList *GetTemplateParameterList(TemplateDecl *TD) {
+  if (TD->isImplicit())
+    return TD->getTemplateParameters();
   // Make sure we get the template parameter list from the most
   // recent declaration, since that is the only one that is guaranteed to
   // have all the default template argument information.
@@ -1827,7 +1829,8 @@ static TemplateParameterList *GetTemplateParameterList(TemplateDecl *TD) {
   //    template <class = void> friend struct C;
   //  };
   //  template struct S<int>;
-  while (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None &&
+  while ((D->isImplicit() ||
+          D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) &&
          D->getPreviousDecl())
     D = D->getPreviousDecl();
   return cast<TemplateDecl>(D)->getTemplateParameters();
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index f16a07e1a1b34..791e44658bd96 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -583,37 +583,53 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
       return TemplateDeductionResult::Success;
 
     auto NewDeduced = DeducedTemplateArgument(Arg);
-    // Provisional resolution for CWG2398: If Arg is also a template template
-    // param, and it names a template specialization, then we deduce a
-    // synthesized template template parameter based on A, but using the TS's
-    // arguments as defaults.
-    if (auto *TempArg = dyn_cast_or_null<TemplateTemplateParmDecl>(
-            Arg.getAsTemplateDecl())) {
+    // Provisional resolution for CWG2398: If Arg names a template
+    // specialization, then we deduce a synthesized template template parameter
+    // based on A, but using the TS's arguments as defaults.
+    if (DefaultArguments.size() != 0) {
       assert(Arg.getKind() == TemplateName::Template);
-      assert(!TempArg->isExpandedParameterPack());
-
+      TemplateDecl *TempArg = Arg.getAsTemplateDecl();
       TemplateParameterList *As = TempArg->getTemplateParameters();
-      if (DefaultArguments.size() != 0) {
-        assert(DefaultArguments.size() <= As->size());
-        SmallVector<NamedDecl *, 4> Params(As->size());
-        for (unsigned I = 0; I < DefaultArguments.size(); ++I)
-          Params[I] = getTemplateParameterWithDefault(S, As->getParam(I),
-                                                      DefaultArguments[I]);
-        for (unsigned I = DefaultArguments.size(); I < As->size(); ++I)
-          Params[I] = As->getParam(I);
-        // FIXME: We could unique these, and also the parameters, but we don't
-        // expect programs to contain a large enough amount of these deductions
-        // for that to be worthwhile.
-        auto *TPL = TemplateParameterList::Create(
-            S.Context, SourceLocation(), SourceLocation(), Params,
-            SourceLocation(), As->getRequiresClause());
-        NewDeduced = DeducedTemplateArgument(
-            TemplateName(TemplateTemplateParmDecl::Create(
-                S.Context, TempArg->getDeclContext(), SourceLocation(),
-                TempArg->getDepth(), TempArg->getPosition(),
-                TempArg->isParameterPack(), TempArg->getIdentifier(),
-                TempArg->wasDeclaredWithTypename(), TPL)));
+      assert(DefaultArguments.size() <= As->size());
+
+      SmallVector<NamedDecl *, 4> Params(As->size());
+      for (unsigned I = 0; I < DefaultArguments.size(); ++I)
+        Params[I] = getTemplateParameterWithDefault(S, As->getParam(I),
+                                                    DefaultArguments[I]);
+      for (unsigned I = DefaultArguments.size(); I < As->size(); ++I)
+        Params[I] = As->getParam(I);
+      // FIXME: We could unique these, and also the parameters, but we don't
+      // expect programs to contain a large enough amount of these deductions
+      // for that to be worthwhile.
+      auto *TPL = TemplateParameterList::Create(
+          S.Context, SourceLocation(), SourceLocation(), Params,
+          SourceLocation(), As->getRequiresClause());
+
+      TemplateDecl *TD;
+      switch (TempArg->getKind()) {
+      case Decl::TemplateTemplateParm: {
+        auto *A = cast<TemplateTemplateParmDecl>(TempArg);
+        assert(!A->isExpandedParameterPack());
+        TD = TemplateTemplateParmDecl::Create(
+            S.Context, A->getDeclContext(), SourceLocation(), A->getDepth(),
+            A->getPosition(), A->isParameterPack(), A->getIdentifier(),
+            A->wasDeclaredWithTypename(), TPL);
+        break;
+      }
+      case Decl::ClassTemplate: {
+        auto *A = cast<ClassTemplateDecl>(TempArg);
+        auto *CT = ClassTemplateDecl::Create(S.Context, A->getDeclContext(),
+                                             SourceLocation(), A->getDeclName(),
+                                             TPL, A->getTemplatedDecl());
+        CT->setPreviousDecl(A);
+        TD = CT;
+        break;
+      }
+      default:
+        llvm_unreachable("Unexpected Template Kind");
       }
+      TD->setImplicit(true);
+      NewDeduced = DeducedTemplateArgument(TemplateName(TD));
     }
 
     DeducedTemplateArgument Result = checkDeducedTemplateArguments(S.Context,
diff --git a/clang/test/CXX/temp/temp.decls/temp.alias/p2.cpp b/clang/test/CXX/temp/temp.decls/temp.alias/p2.cpp
index a5b39fe5c51f7..bc39431253880 100644
--- a/clang/test/CXX/temp/temp.decls/temp.alias/p2.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.alias/p2.cpp
@@ -28,13 +28,14 @@ namespace StdExample {
     { /* ... */ }
 
   template<template<class> class TT>
-    void f(TT<int>); // expected-note {{candidate template ignored}}
+    void f(TT<int>);
 
   template<template<class,class> class TT>
     void g(TT<int, Alloc<int>>);
 
   int h() {
-    f(v); // expected-error {{no matching function for call to 'f'}}
+    f(v); // OK: TT = vector, Alloc<int> is used as the default argument for the
+          // second parameter.
     g(v); // OK: TT = vector
   }
 
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index e3b5e575374d3..4cc946735a1e2 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -65,13 +65,10 @@ namespace class_template {
   template <class T3> struct B;
 
   template <template <class T4> class TT1, class T5> struct B<TT1<T5>>;
-  // new-note@-1 {{partial specialization matches}}
 
   template <class T6, class T7> struct B<A<T6, T7>> {};
-  // new-note@-1 {{partial specialization matches}}
 
   template struct B<A<int>>;
-  // new-error@-1 {{ambiguous partial specialization}}
 } // namespace class_template
 
 namespace type_pack1 {

[erichkeane]

Seems reasonable, lgtm.

[jyknight]

Heads up: this commit has triggered some weird errors for a compile, but only when clang header modules are enabled. A std::vector, that's built through a large amount of template gunk that definitely involves template template parameter matching, ends up failing vector's "Allocator::value_type must be same type as value_type" static assertion -- the vector's allocator's value_type seems to be some unrelated type. But, only when modules are enabled.

That seems to probably indicate a bug in this commit (presumably related to AST serialization or deserialization?), but I don't have a test-case, and, that's harder to get for issues that only arise with modules enabled...

If this very vague and unhelpful description makes you think "oh, I see where the problem likely is!", then great! But, otherwise, I'll see if I can create a test case...

[dcci]

I am seeing something similar in vector (without modules). I'm reducing a test case now.

[dcci]

Example error while I reduce:

fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:404:21: error: static assertion failed due to requirement 'is_same<std::basic_string<char, std::char_traits<char>, std::allocator<char>>, quic::Interval<unsigned long, 1>>::value': std::vector must have the same value_type as its allocator
  404 |       static_assert(is_same<typename _Alloc::value_type, _Tp>::value,
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buck-out/v2/gen/fbcode/21eca57c08ba71f9/quic/common/__interval_set__/buck-headers/quic/common/IntervalSet.h:61:29: note: in instantiation of template class 'std::vector<quic::Interval<unsigned long>>' requested here
   61 | class IntervalSet : private Container<Interval<T, Unit>> {
      |                             ^
buck-out/v2/gen/fbcode/21eca57c08ba71f9/quic/state/__quic_state_machine__/buck-headers/quic/state/QuicStreamManager.h:79:41: note: in instantiation of template class 'quic::IntervalSet<unsigned long, 1, std::vector>' requested here
   79 |   IntervalSet<StreamId, 1, std::vector> streams_;
      |                                         ^

[mizvekov]

Thanks for the heads up, this does look like a problem. I am reverting it for now.

[mizvekov]

Heads up: this commit has triggered some weird errors for a compile, but only when clang header modules are enabled.
That seems to probably indicate a bug in this commit (presumably related to AST serialization or deserialization?), but I don't have a test-case, and, that's harder to get for issues that only arise with modules enabled...

Thanks for the heads-up, James.

Indeed, and it's not only modules, our tooling for reducing test cases is getting less and less effective.

[mizvekov]

I figured a reproducer based on your hints:

template<class T, class U> struct A {};

template<template<class> class TT> auto f(TT<int> a) { return a; }

A<int, float> v1;
A<int, double> v2;

using X = decltype(f(v1));
using Y = decltype(f(v2));

Fails with:

t.cc:9:20: error: no matching function for call to 'f'
    9 | using Y = decltype(f(v2));
      |                    ^
t.cc:3:41: note: candidate template ignored: deduced type 'A<[...], (default) float>' of 1st parameter does not match adjusted type 'A<[...], double>' of argument [with TT = A]
    3 | template<template<class> class TT> auto f(TT<int> a) { return a; }
      |                                         ^

This shows we are not properly canonicalizing the synthesized template declaration.

The problem does not show up with class template partial specializations due to a pre-existing issue, but otherwise this was an oversight on my part.

No need to keep reducing on your end, I already have enough to go with.