[Clang] Don't assume unexpanded PackExpansions' size when expanding packs (#120380)

CheckParameterPacksForExpansion() previously assumed that template
arguments don't include PackExpansion types when attempting another pack
expansion (i.e. when NumExpansions is present). However, this assumption
doesn't hold for type aliases, whose substitution might involve
unexpanded packs. This can lead to incorrect diagnostics during
substitution because the pack size is not yet determined.

To address this, this patch calculates the minimum pack size (ignoring
unexpanded PackExpansionTypes) and compares it to the previously
expanded size. If the minimum pack size is smaller, then there's still a
chance for future substitution to expand it to a correct size, so we
don't diagnose it too eagerly.

Fixes #61415
Fixes #32252
Fixes #17042

Author: zyn0217

clang/docs/ReleaseNotes.rst

@@ -850,6 +850,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...>;
+}
+
+}