[Clang] Don't assume unexpanded PackExpansions' size when expanding packs

clang/docs/ReleaseNotes.rst

@@ -825,6 +825,7 @@ Bug Fixes to C++ Support
 - Clang no longer rejects deleting a pointer of incomplete enumeration type. (#GH99278)
 - Fixed recognition of ``std::initializer_list`` when it's surrounded with ``extern "C++"`` and exported
   out of a module (which is the case e.g. in MSVC's implementation of ``std`` module). (#GH118218)
+- Fixed a pack expansion issue in checking unexpanded parameter sizes. (#GH17042)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5863,10 +5863,10 @@ def err_pack_expansion_without_parameter_packs : Error<
   "pack expansion does not contain any unexpanded parameter packs">;
 def err_pack_expansion_length_conflict : Error<
   "pack expansion contains parameter packs %0 and %1 that have different "
-  "lengths (%2 vs. %3)">;
+  "lengths (%2 vs. %select{|at least }3%4))">;
 def err_pack_expansion_length_conflict_multilevel : Error<
   "pack expansion contains parameter pack %0 that has a different "
-  "length (%1 vs. %2) from outer parameter packs">;
+  "length (%1 vs. %select{|at least }2%3) from outer parameter packs">;
 def err_pack_expansion_length_conflict_partial : Error<
   "pack expansion contains parameter pack %0 that has a different "
   "length (at least %1 vs. %2) from outer parameter packs">;

clang/lib/Sema/SemaTemplateVariadic.cpp

@@ -780,7 +780,7 @@ bool Sema::CheckParameterPacksForExpansion(
     }
 
     // Determine the size of this argument pack.
-    unsigned NewPackSize;
+    unsigned NewPackSize, PendingPackExpansionSize = 0;
     if (IsVarDeclPack) {
       // Figure out whether we're instantiating to an argument pack or not.
       typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
@@ -808,7 +808,25 @@ bool Sema::CheckParameterPacksForExpansion(
       }
 
       // Determine the size of the argument pack.
-      NewPackSize = TemplateArgs(Depth, Index).pack_size();
+      ArrayRef<TemplateArgument> Pack =
+          TemplateArgs(Depth, Index).getPackAsArray();
+      NewPackSize = Pack.size();
+      PendingPackExpansionSize =
+          llvm::count_if(Pack, [](const TemplateArgument &TA) {
+            if (!TA.isPackExpansion())
+              return false;
+
+            if (TA.getKind() == TemplateArgument::Type)
+              return !TA.getAsType()
+                          ->getAs<PackExpansionType>()
+                          ->getNumExpansions();
+
+            if (TA.getKind() == TemplateArgument::Expression)
+              return !cast<PackExpansionExpr>(TA.getAsExpr())
+                          ->getNumExpansions();
+
+            return !TA.getNumTemplateExpansions();
+          });
     }
 
     // C++0x [temp.arg.explicit]p9:
@@ -831,7 +849,7 @@ bool Sema::CheckParameterPacksForExpansion(
     }
 
     if (!NumExpansions) {
-      // The is the first pack we've seen for which we have an argument.
+      // This is the first pack we've seen for which we have an argument.
       // Record it.
       NumExpansions = NewPackSize;
       FirstPack.first = Name;
@@ -841,17 +859,44 @@ bool Sema::CheckParameterPacksForExpansion(
     }
 
     if (NewPackSize != *NumExpansions) {
+      // In some cases, we might be handling packs with unexpanded template
+      // arguments. For example, this can occur when substituting into a type
+      // alias declaration that uses its injected template parameters as
+      // arguments:
+      //
+      //   template <class... Outer> struct S {
+      //     template <class... Inner> using Alias = S<void(Outer, Inner)...>;
+      //   };
+      //
+      // Consider an instantiation attempt like 'S<int>::Alias<Pack...>', where
+      // Pack comes from another template parameter. 'S<int>' is first
+      // instantiated, expanding the outer pack 'Outer' to <int>. The alias
+      // declaration is accordingly substituted, leaving the template arguments
+      // as unexpanded
+      // '<Pack...>'.
+      //
+      // Since we have no idea of the size of '<Pack...>' until its expansion,
+      // we shouldn't assume its pack size for validation. However if we are
+      // certain that there are extra arguments beyond unexpanded packs, in
+      // which case the pack size is already larger than the previous expansion,
+      // we can complain that before instantiation.
+      unsigned LeastNewPackSize = NewPackSize - PendingPackExpansionSize;
+      if (PendingPackExpansionSize && LeastNewPackSize <= *NumExpansions) {
+        ShouldExpand = false;
+        continue;
+      }
       // C++0x [temp.variadic]p5:
       //   All of the parameter packs expanded by a pack expansion shall have
       //   the same number of arguments specified.
       if (HaveFirstPack)
         Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
-            << FirstPack.first << Name << *NumExpansions << NewPackSize
+            << FirstPack.first << Name << *NumExpansions
+            << (LeastNewPackSize != NewPackSize) << LeastNewPackSize
             << SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
       else
         Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
-            << Name << *NumExpansions << NewPackSize
-            << SourceRange(ParmPack.second);
+            << Name << *NumExpansions << (LeastNewPackSize != NewPackSize)
+            << LeastNewPackSize << SourceRange(ParmPack.second);
       return true;
     }
   }

clang/test/SemaTemplate/pack-deduction.cpp

@@ -199,3 +199,62 @@ constexpr auto baz(Int<foo<T>(T())>... x) -> int { return 1; }
 
 static_assert(baz<Int<1>, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) == 1, "");
 }
+
+namespace GH17042 {
+
+template <class... Ts> struct X {
+  template <class... Us> using Y = X<void(Ts, Us)...>; // #GH17042_Y
+};
+
+template <class... T>
+using any_pairs_list = X<int, int>::Y<T...>; // #any_pairs_list
+
+template <class... T>
+using any_pairs_list_2 = X<int, int>::Y<>;
+// expected-error@#GH17042_Y {{different length (2 vs. 0)}} \
+// expected-note@-1 {{requested here}}
+
+template <class A, class B, class... P>
+using any_pairs_list_3 = X<int, int>::Y<A, B, P...>; // #any_pairs_list_3
+
+template <class A, class B, class C, class... P>
+using any_pairs_list_4 = X<int, int>::Y<A, B, C, P...>;
+// expected-error@#GH17042_Y {{different length (2 vs. at least 3)}} \
+// expected-note@-1 {{requested here}}
+
+static_assert(__is_same(any_pairs_list<char, char>, X<void(int, char), void(int, char)>), "");
+
+static_assert(!__is_same(any_pairs_list<char, char, char>, X<void(int, char), void(int, char)>), "");
+// expected-error@#GH17042_Y {{different length (2 vs. 3)}} \
+// expected-note@#any_pairs_list {{requested here}} \
+// expected-note@-1 {{requested here}}
+
+static_assert(__is_same(any_pairs_list_3<char, char>, X<void(int, char), void(int, char)>), "");
+
+static_assert(!__is_same(any_pairs_list_3<char, char, float>, X<void(int, char), void(int, char)>), "");
+// expected-error@#GH17042_Y {{different length (2 vs. 3)}} \
+// expected-note@#any_pairs_list_3 {{requested here}} \
+// expected-note@-1 {{requested here}}
+
+namespace TemplateTemplateParameters {
+template <class... T> struct C {};
+
+template <class T, template <class> class... Args1> struct Ttp {
+  template <template <class> class... Args2>
+  using B = C<void(Args1<T>, Args2<T>)...>;
+};
+template <class> struct D {};
+
+template <template <class> class... Args>
+using Alias = Ttp<int, D, D>::B<Args...>;
+}
+
+namespace NTTP {
+template <int... Args1> struct Nttp {
+  template <int... Args2> using B = Nttp<(Args1 + Args2)...>;
+};
+
+template <int... Args> using Alias = Nttp<1, 2, 3>::B<Args...>;
+}
+
+}