[clang] Handle template argument conversions for non-pack param to pack argument

[mizvekov]

This fixes a regression introduced in #96023, reported in #110231 (comment)

[llvmbot]

@llvm/pr-subscribers-libcxx

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes a regression introduced in #96023, reported in #110231 (comment)

---

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

6 Files Affected:

• (modified) clang/lib/Sema/SemaTemplate.cpp (+12-12)
• (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+32-27)
• (modified) clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp (+2-2)
• (modified) clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp (+4)
• (modified) clang/test/SemaTemplate/cwg2398.cpp (+6)
• (modified) clang/test/SemaTemplate/temp_arg_template_p0522.cpp (+3-3)

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 226c1172a059d4..bd275a4c4f1e64 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5579,9 +5579,6 @@ bool Sema::CheckTemplateArgumentList(
         return true;
       }
 
-      // We're now done with this argument.
-      ++ArgIdx;
-
       if ((*Param)->isTemplateParameterPack()) {
         // The template parameter was a template parameter pack, so take the
         // deduced argument and place it on the argument pack. Note that we
@@ -5592,8 +5589,16 @@ bool Sema::CheckTemplateArgumentList(
       } else {
         // Move to the next template parameter.
         ++Param;
+        if (PartialOrderingTTP && PackExpansionIntoNonPack) {
+          SugaredConverted.pop_back();
+          CanonicalConverted.pop_back();
+          continue;
+        }
       }
 
+      // We're now done with this argument.
+      ++ArgIdx;
+
       // If we just saw a pack expansion into a non-pack, then directly convert
       // the remaining arguments, because we don't know what parameters they'll
       // match up with.
@@ -5727,15 +5732,10 @@ bool Sema::CheckTemplateArgumentList(
   // pack expansions; they might be empty. This can happen even if
   // PartialTemplateArgs is false (the list of arguments is complete but
   // still dependent).
-  if (PartialOrderingTTP ||
-      (CurrentInstantiationScope &&
-       CurrentInstantiationScope->getPartiallySubstitutedPack())) {
-    while (ArgIdx < NumArgs &&
-           NewArgs[ArgIdx].getArgument().isPackExpansion()) {
-      const TemplateArgument &Arg = NewArgs[ArgIdx++].getArgument();
-      SugaredConverted.push_back(Arg);
-      CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg));
-    }
+  while (ArgIdx < NumArgs && NewArgs[ArgIdx].getArgument().isPackExpansion()) {
+    const TemplateArgument &Arg = NewArgs[ArgIdx++].getArgument();
+    SugaredConverted.push_back(Arg);
+    CanonicalConverted.push_back(Context.getCanonicalTemplateArgument(Arg));
   }
 
   // If we have any leftover arguments, then there were too many arguments.
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 03ff8145e3b4ac..ce3317db5a8202 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3377,35 +3377,40 @@ static TemplateDeductionResult FinishTemplateArgumentDeduction(
     return Result;
 
   // Check that we produced the correct argument list.
-  TemplateParameterList *TemplateParams = Template->getTemplateParameters();
-  auto isSame = [&](unsigned I, const TemplateArgument &P,
-                    const TemplateArgument &A) {
-    if (isSameTemplateArg(S.Context, P, A, PartialOrdering,
-                          /*PackExpansionMatchesPack=*/true))
-      return true;
-    Info.Param = makeTemplateParameter(TemplateParams->getParam(I));
+  for (ArrayRef<TemplateArgument> Ps = TemplateArgs, As = CanonicalBuilder;
+       !Ps.empty() && !As.empty();
+       /**/) {
+    TemplateArgument P = Ps.front(), A = As.front();
+    if (P.getKind() == TemplateArgument::Pack) {
+      assert(Ps.size() == 1 && "Pack not last element?");
+      Ps = P.getPackAsArray();
+      continue;
+    }
+    if (A.getKind() == TemplateArgument::Pack) {
+      assert(As.size() == 1 && "Pack not last element?");
+      As = A.getPackAsArray();
+      continue;
+    }
+
+    if (P.isPackExpansion())
+      P = P.getPackExpansionPattern();
+    else
+      Ps = Ps.drop_front();
+    if (A.isPackExpansion())
+      A = A.getPackExpansionPattern();
+    else
+      As = As.drop_front();
+
+    if (isSameTemplateArg(S.Context, P, A, PartialOrdering))
+      continue;
+    unsigned I = As.end() == CanonicalBuilder.end()
+                     ? As.begin() - CanonicalBuilder.begin()
+                     : CanonicalBuilder.size() - 1;
+    Info.Param =
+        makeTemplateParameter(Template->getTemplateParameters()->getParam(I));
     Info.FirstArg = P;
     Info.SecondArg = A;
-    return false;
-  };
-  for (unsigned I = 0, E = TemplateParams->size(); I != E; ++I) {
-    const TemplateArgument &P = TemplateArgs[I];
-    if (P.isPackExpansion()) {
-      assert(I == TemplateArgs.size() - 1);
-      for (/**/; I != E; ++I) {
-        const TemplateArgument &A = CanonicalBuilder[I];
-        if (A.getKind() == TemplateArgument::Pack) {
-          for (const TemplateArgument &Ai : A.getPackAsArray())
-            if (!isSame(I, P, Ai))
-              return TemplateDeductionResult::NonDeducedMismatch;
-        } else if (!isSame(I, P, A)) {
-          return TemplateDeductionResult::NonDeducedMismatch;
-        }
-      }
-      break;
-    }
-    if (!isSame(I, P, CanonicalBuilder[I]))
-      return TemplateDeductionResult::NonDeducedMismatch;
+    return TemplateDeductionResult::NonDeducedMismatch;
   }
 
   if (!PartialOrdering) {
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp
index 1bbbd1d3429ddd..ce27e6aa83c3b9 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp
@@ -18,7 +18,7 @@ eval<D<int, 17>> eD; // expected-error{{implicit instantiation of undefined temp
 eval<E<int, float>> eE; // expected-error{{implicit instantiation of undefined template 'eval<E<int, float>>}}
 
 template<
-  template <int ...N> // expected-error{{deduced non-type template argument does not have the same type as the corresponding template parameter ('int' vs 'long')}}
+  template <int ...N> // expected-error{{deduced non-type template argument does not have the same type as the corresponding template parameter ('long' vs 'int')}}
   class TT // expected-note {{previous template template parameter is here}}
 > struct X0 { };
 
@@ -31,7 +31,7 @@ X0<X0b> inst_x0b;
 X0<X0c> inst_x0c; // expected-note{{template template argument has different template parameters than its corresponding template template parameter}}
 
 template<typename T,
-         template <T ...N> // expected-error{{deduced non-type template argument does not have the same type as the corresponding template parameter ('short' vs 'long')}}
+         template <T ...N> // expected-error{{deduced non-type template argument does not have the same type as the corresponding template parameter ('long' vs 'short')}}
          class TT // expected-note {{previous template template parameter is here}}
 > struct X1 { };
 template<int I, int J, int ...Rest> struct X1a;
diff --git a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp
index 51df1e0b14541b..fbda7cbe696bb0 100644
--- a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp
+++ b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp
@@ -29,10 +29,14 @@ namespace PackExpansionNotAtEnd {
                                                 >::value? 1 : -1];
 
   template<typename ... Types> struct UselessPartialSpec;
+  // expected-note@-1 {{template is declared here}}
 
+  // FIXME: We should simply disallow a pack expansion which is not at
+  // the end of the partial spec template argument list.
   template<typename ... Types, // expected-note{{non-deducible template parameter 'Types'}}
            typename Tail> // expected-note{{non-deducible template parameter 'Tail'}}
   struct UselessPartialSpec<Types..., Tail>; // expected-error{{class template partial specialization contains template parameters that cannot be deduced; this partial specialization will never be used}}
+  // expected-error@-1 {{is not more specialized than the primary template}}
 }
 
 // When a pack expansion occurs within a template argument list, the entire
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index b9e9e9f0c97f26..1b55af782fcb7c 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -541,3 +541,9 @@ namespace regression2 {
   template <typename, int> struct Matrix;
   template struct D<Matrix<double, 3>>;
 } // namespace regression2
+
+namespace regression3 {
+  template <template <auto...> class TT> struct A {};
+  template <auto, int> struct B;
+  template struct A<B>;
+} // namespace regression3
diff --git a/clang/test/SemaTemplate/temp_arg_template_p0522.cpp b/clang/test/SemaTemplate/temp_arg_template_p0522.cpp
index d40577d5270468..441b9e5af9d3d7 100644
--- a/clang/test/SemaTemplate/temp_arg_template_p0522.cpp
+++ b/clang/test/SemaTemplate/temp_arg_template_p0522.cpp
@@ -21,7 +21,7 @@ template<int, int> struct ii;
 template<int...> struct Pi;
 template<int, int, int...> struct iiPi;
 
-template<int, typename = int> struct iDt; // #iDt
+template<int, typename = int> struct iDt;
 template<int, typename> struct it; // #it
 
 template<typename T, T v> struct t0;
@@ -50,9 +50,9 @@ namespace IntPackParam {
   using ok_compat = Pt<TPi<i>, TPi<iDi>, TPi<ii>, TPi<iiPi>>;
   using err1 = TPi<t0>; // expected-error@#TPi {{template argument for template type parameter must be a type}}
                         // expected-note@-1 {{different template parameters}}
-  using err2 = TPi<iDt>; // expected-error@#iDt {{could not match 'type-parameter-0-1' against}}
+  using err2 = TPi<iDt>; // expected-error@#TPi {{template argument for template type parameter must be a type}}
                          // expected-note@-1 {{different template parameters}}
-  using err3 = TPi<it>; // expected-error@#it {{could not match 'type-parameter-0-1' against}}
+  using err3 = TPi<it>; // expected-error@#TPi {{template argument for template type parameter must be a type}}
                         // expected-note@-1 {{different template parameters}}
 }
 

[erichkeane]

I admittedly don't have a great idea of what is going on here, but I don't see anything glaringly out of place, so +1.