[clang] Improve diagnostics for expansion length mismatch

[mizvekov]

Summary

When checking parameter packs for expansion, instead of basing the diagnostic for
length mismatch for outer parameters only on the known number of expansions,
we should also analyze SubstTemplateTypeParmPackType and SubstNonTypeTemplateParmPackExpr
for unexpanded packs, so we can emit a diagnostic pointing to a concrete
outer parameter.

Signed-off-by: Matheus Izvekov <[e-mail]>

[mizvekov]

.

[mizvekov]

.

[mizvekov]

rebase

[erichkeane]

Can you make a better commit message? I don't have a good idea of the intent/description of what you're trying to do here.

[mizvekov]

In D128095#3712343, @erichkeane wrote:

Can you make a better commit message? I don't have a good idea of the intent/description of what you're trying to do here.

Done.

I split this up into two patches:

• D131802: Introduces the test and a simple fix.
• This one which improves the analyzer to give better diagnostics.

[shafik]

clang/lib/Sema/SemaTemplateDeduction.cpp

741–745

Structured bindings maybe?

[mizvekov]

clang/lib/Sema/SemaTemplateDeduction.cpp

741–745

Sure, they weren't an option back when this patch was first submitted :)

[mizvekov]

ping

[aaron.ballman]

clang/lib/Sema/SemaTemplateVariadic.cpp

103–106

Do we need to handle FunctionParmPackExpr as well?

[mizvekov]

clang/lib/Sema/SemaTemplateVariadic.cpp

103–106

Right, that and SubstTemplateTemplateParmPack are the two missing cases we could handle here, and perhaps that would allow us to get rid of that 'from outer parameter packs' diagnostics completely.

Would you prefer to handle everything in one patch?

[erichkeane]

just a pair of minor changes I'd like to see, otherwise this LGTM.

clang/lib/Sema/SemaTemplateVariadic.cpp

103–106

I think I'm alright doing those in a followup patch.

699

Can you please make this an else if at the level higher? The 'else' after this ends up being so small that it gets lost trying to figure out what is going on here.

So:

} else if (const auto *VD = dyn_cast<VarDecl>(ParmPack.first.get<const NamedDecl *>()))

844

same comment here, see if the 'vardecl' logic can be moved up a level in the 'if' tree.

[aaron.ballman]

clang/lib/Sema/SemaTemplateVariadic.cpp

103–106

I'm also fine handling it in a follow-up if the changes are involved.

[erichkeane]

clang/lib/Sema/SemaTemplateVariadic.cpp

845

This pattern with the init + condition is a little awkward here... any reason to not just use the cast around the 'ND" and just use the VD in the use below? or is there a good reason to split it up like this?

Same with the above case.

[mizvekov]

clang/lib/Sema/SemaTemplateVariadic.cpp

845

No very strong reason than that just from my different perception this looked fine :)

Minor advantage that we don't make the variable a VarDecl pointer if we don't need to access it as such.

But no strong preference here, I can have another look tomorrow.

[mizvekov]

clang/lib/Sema/SemaTemplateVariadic.cpp

845

I played a little bit with this change.

I think one thing that makes that pattern of adding separate ND and VD a bit confusing is that at a glance it almost looks like these are different cases in the PointerUnion variant. You have to do a double take to see that, since the nested cast is a bit subtle. This can become even subtler as we add more cases in the next patch.

Or we could stop using PointerUnion on the next patch, since it's so unergonomic with so many variants.

Anyway, I did some other refactorings and I think in general this will be much clearer to read on my next push.

With this refactoring, on this part here that problem goes away since we make this section produce a NamedDecl.

On the second part, below, I tidied up the code so much that I think that nested else isn't almost invisible anymore, since the other blocks become about the same size.

Otherwise, let me know what you think.

I also added to this first part here a FIXME comment describing a pre-existing problem where if we get a canonical TTP, the diagnostics fail to point to the relevant parameter since we won't have it's identifier.

Will try to add a repro and fix for that on the next patch.

[erichkeane]

Happy enough, LGTM.

clang/lib/Sema/SemaTemplateVariadic.cpp

845

Thanks for spending time on this... the nested conditionals on the var-decl was hiding 90%+ of what was going on in the branch, and at least this top one is BETTER enough than before. I too hate the pointer union (note BTW, that _4_ is the maximum number of elements thanks to 32 bit platforms, that'll burn you :)).

[mizvekov]

clang/lib/Sema/SemaTemplateVariadic.cpp

845

Thanks, true about the maximum size :)

Some of the cases could be unified as we have two types and two expressions.

clang/docs/ReleaseNotes.rst

 - Clang will now print more information about failed static assertions. In
   particular, simple static assertion expressions are evaluated to their
   compile-time value and printed out if the assertion fails.
 - Diagnostics about uninitialized ``constexpr`` varaibles have been improved
   to mention the missing constant initializer.
 - Correctly diagnose a future keyword if it exist as a keyword in the higher
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
+- When diagnosing multi-level pack expansions of mismatched lengths, Clang will
+  now, in most cases, be able to point to the relevant outer parameter.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 

clang/include/clang/Sema/Sema.h

 
 namespace threadSafety {
   class BeforeSet;
   void threadSafetyCleanup(BeforeSet* Cache);
 }
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType*, NamedDecl*>,
-                  SourceLocation> UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+    llvm::PointerUnion<
+        const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+        const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+    SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
 struct FileNullability {
   /// The first pointer declarator (of any pointer kind) in the file that does
   /// not have a corresponding nullability annotation.
   SourceLocation PointerLoc;
 

clang/include/clang/Sema/SemaInternal.h

inline InheritableAttr *getDLLAttr(Decl *D) {
   if (auto *Import = D->getAttr<DLLImportAttr>())
     return Import;
   if (auto *Export = D->getAttr<DLLExportAttr>())
     return Export;
   return nullptr;
 }
 
 /// Retrieve the depth and index of a template parameter.
-inline std::pair<unsigned, unsigned> getDepthAndIndex(NamedDecl *ND) {
+inline std::pair<unsigned, unsigned> getDepthAndIndex(const NamedDecl *ND) {
   if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(ND))
     return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
   if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(ND))
     return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
 
   const auto *TTP = cast<TemplateTemplateParmDecl>(ND);
   return std::make_pair(TTP->getDepth(), TTP->getIndex());
 }
 
 /// Retrieve the depth and index of an unexpanded parameter pack.
 inline std::pair<unsigned, unsigned>
 getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = UPP.first.dyn_cast<const TemplateTypeParmType *>())
     return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(UPP.first.get<NamedDecl *>());
+  return getDepthAndIndex(UPP.first.get<const NamedDecl *>());
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
   typedef SmallVector<TypoCorrection, 1> TypoResultList;
   typedef llvm::StringMap<TypoResultList> TypoResultsMap;
   typedef std::map<unsigned, TypoResultsMap> TypoEditDistanceMap;
 
 public:

clang/lib/Sema/SemaTemplateDeduction.cpp

auto AddPack = [&](unsigned Index) {
       // FIXME: Also collect the unexpanded packs in any type and template
       // parameter packs that are pack expansions.
     };
 
     auto Collect = [&](TemplateArgument Pattern) {
       SmallVector<UnexpandedParameterPack, 2> Unexpanded;
       S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
       for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-        unsigned Depth, Index;
-        std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+        UnexpandedParameterPack U = Unexpanded[I];
+        if (U.first.is<const SubstTemplateTypeParmPackType *>() ||
+            U.first.is<const SubstNonTypeTemplateParmPackExpr *>())
+          continue;
+        auto [Depth, Index] = getDepthAndIndex(U);
         if (Depth == Info.getDeducedDepth())
           AddPack(Index);
       }
     };
 
     // Look for unexpanded packs in the pattern.
     Collect(Pattern);
     assert(!Packs.empty() && "Pack expansion without unexpanded packs?");

clang/lib/Sema/SemaTemplateVariadic.cpp

public:
     /// Ideally, this routine would never be used.
     bool VisitTemplateTypeParmType(TemplateTypeParmType *T) {
       if (T->isParameterPack())
         addUnexpanded(T);
 
       return true;
     }
 
+    bool
+    VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
+      Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+      return true;
+    }
+
+    bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+      Unexpanded.push_back({T, SourceLocation()});
+      return true;
+    }
+
+    bool
+    VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+      Unexpanded.push_back({E, E->getParameterPackLocation()});
+      return true;
+    }
+
     /// Record occurrences of function and non-type template
     /// parameter packs in an expression.
     bool VisitDeclRefExpr(DeclRefExpr *E) {
       if (E->getDecl()->isParameterPack())
         addUnexpanded(E->getDecl(), E->getLocation());
 
       return true;
     }
Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
   SmallVector<UnexpandedParameterPack, 4> LambdaParamPackReferences;
   if (auto *LSI = getEnclosingLambda()) {
     for (auto &Pack : Unexpanded) {
       auto DeclaresThisPack = [&](NamedDecl *LocalPack) {
         if (auto *TTPT = Pack.first.dyn_cast<const TemplateTypeParmType *>()) {
           auto *TTPD = dyn_cast<TemplateTypeParmDecl>(LocalPack);
           return TTPD && TTPD->getTypeForDecl() == TTPT;
         }
-        return declaresSameEntity(Pack.first.get<NamedDecl *>(), LocalPack);
+        return declaresSameEntity(Pack.first.get<const NamedDecl *>(),
+                                  LocalPack);
       };
       if (llvm::any_of(LSI->LocalPacks, DeclaresThisPack))
         LambdaParamPackReferences.push_back(Pack);
     }
 
     if (LambdaParamPackReferences.empty()) {
       // Construct in lambda only references packs declared outside the lambda.
       // That's OK for now, but the lambda itself is considered to contain an
Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
   llvm::SmallPtrSet<IdentifierInfo *, 4> NamesKnown;
 
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
     IdentifierInfo *Name = nullptr;
     if (const TemplateTypeParmType *TTP
           = Unexpanded[I].first.dyn_cast<const TemplateTypeParmType *>())
       Name = TTP->getIdentifier();
     else
-      Name = Unexpanded[I].first.get<NamedDecl *>()->getIdentifier();
+      Name = Unexpanded[I].first.get<const NamedDecl *>()->getIdentifier();
 
     if (Name && NamesKnown.insert(Name).second)
       Names.push_back(Name);
 
     if (Unexpanded[I].second.isValid())
       Locations.push_back(Unexpanded[I].second);
   }
 
bool Sema::DiagnoseUnexpandedParameterPackInRequiresExpr(RequiresExpr *RE) {
   assert(!Unexpanded.empty() && "Unable to find unexpanded parameter packs");
 
   // We only care about unexpanded references to the RequiresExpr's own
   // parameter packs.
   auto Parms = RE->getLocalParameters();
   llvm::SmallPtrSet<NamedDecl*, 8> ParmSet(Parms.begin(), Parms.end());
   SmallVector<UnexpandedParameterPack, 2> UnexpandedParms;
   for (auto Parm : Unexpanded)
-    if (ParmSet.contains(Parm.first.dyn_cast<NamedDecl*>()))
+    if (ParmSet.contains(Parm.first.dyn_cast<const NamedDecl *>()))
       UnexpandedParms.push_back(Parm);
   if (UnexpandedParms.empty())
     return false;
 
   return DiagnoseUnexpandedParameterPacks(RE->getBeginLoc(), UPPC_Requirement,
                                           UnexpandedParms);
 }
 

 
 bool Sema::CheckParameterPacksForExpansion(
     SourceLocation EllipsisLoc, SourceRange PatternRange,
     ArrayRef<UnexpandedParameterPack> Unexpanded,
     const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
     bool &RetainExpansion, Optional<unsigned> &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair<IdentifierInfo *, SourceLocation> FirstPack;
-  bool HaveFirstPack = false;
-  Optional<unsigned> NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
+  std::pair<const IdentifierInfo *, SourceLocation> FirstPack;
+  Optional<std::pair<unsigned, SourceLocation>> PartialExpansion;
+  Optional<unsigned> CurNumExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
     // Compute the depth and index for this parameter pack.
-    unsigned Depth = 0, Index = 0;
-    IdentifierInfo *Name;
-    bool IsVarDeclPack = false;
-
-    if (const TemplateTypeParmType *TTP =
-            ParmPack.first.dyn_cast<const TemplateTypeParmType *>()) {
-      Depth = TTP->getDepth();
-      Index = TTP->getIndex();
-      Name = TTP->getIdentifier();
-    } else {
-      NamedDecl *ND = ParmPack.first.get<NamedDecl *>();
-      if (isa<VarDecl>(ND))
-        IsVarDeclPack = true;
-      else
-        std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-      Name = ND->getIdentifier();
-    }
-
-    // Determine the size of this argument pack.
+    Optional<std::pair<unsigned, unsigned>> Pos;
     unsigned NewPackSize;
-    if (IsVarDeclPack) {
-      // Figure out whether we're instantiating to an argument pack or not.
-      typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
-
-      llvm::PointerUnion<Decl *, DeclArgumentPack *> *Instantiation =
-          CurrentInstantiationScope->findInstantiationOf(
-              ParmPack.first.get<NamedDecl *>());
-      if (Instantiation->is<DeclArgumentPack *>()) {
-        // We could expand this function parameter pack.
-        NewPackSize = Instantiation->get<DeclArgumentPack *>()->size();
-      } else {
+    const auto *ND = P.dyn_cast<const NamedDecl *>();
+    if (ND && isa<VarDecl>(ND)) {
+      const auto *DAP =
+          CurrentInstantiationScope->findInstantiationOf(ND)
+              ->dyn_cast<LocalInstantiationScope::DeclArgumentPack *>();
+      if (!DAP) {
         // We can't expand this function parameter pack, so we can't expand
         // the pack expansion.
         ShouldExpand = false;
         continue;
       }
+      NewPackSize = DAP->size();
+    } else if (ND) {
+      Pos = getDepthAndIndex(ND);
+    } else if (const auto *TTP = P.dyn_cast<const TemplateTypeParmType *>()) {
+      Pos = {TTP->getDepth(), TTP->getIndex()};
+      ND = TTP->getDecl();
+      // FIXME: We either should have some fallback for canonical TTP, or
+      //        never have canonical TTP here.
+    } else if (const auto *STP =
+                   P.dyn_cast<const SubstTemplateTypeParmPackType *>()) {
+      NewPackSize = STP->getNumArgs();
+      ND = STP->getReplacedParameter()->getDecl();
     } else {
+      const auto *SEP = P.get<const SubstNonTypeTemplateParmPackExpr *>();
+      NewPackSize = SEP->getArgumentPack().pack_size();
+      ND = SEP->getParameterPack();
+    }
+
+    if (Pos) {
       // If we don't have a template argument at this depth/index, then we
       // cannot expand the pack expansion. Make a note of this, but we still
       // want to check any parameter packs we *do* have arguments for.
-      if (Depth >= TemplateArgs.getNumLevels() ||
-          !TemplateArgs.hasTemplateArgument(Depth, Index)) {
+      if (Pos->first >= TemplateArgs.getNumLevels() ||
+          !TemplateArgs.hasTemplateArgument(Pos->first, Pos->second)) {
         ShouldExpand = false;
         continue;
       }
-
       // Determine the size of the argument pack.
-      NewPackSize = TemplateArgs(Depth, Index).pack_size();
-    }
-
+      NewPackSize = TemplateArgs(Pos->first, Pos->second).pack_size();
       // C++0x [temp.arg.explicit]p9:
       //   Template argument deduction can extend the sequence of template
       //   arguments corresponding to a template parameter pack, even when the
       //   sequence contains explicitly specified template arguments.
-    if (!IsVarDeclPack && CurrentInstantiationScope) {
-      if (NamedDecl *PartialPack
-                    = CurrentInstantiationScope->getPartiallySubstitutedPack()){
-        unsigned PartialDepth, PartialIndex;
-        std::tie(PartialDepth, PartialIndex) = getDepthAndIndex(PartialPack);
-        if (PartialDepth == Depth && PartialIndex == Index) {
+      if (CurrentInstantiationScope)
+        if (const NamedDecl *PartialPack =
+                CurrentInstantiationScope->getPartiallySubstitutedPack();
+            PartialPack && getDepthAndIndex(PartialPack) == *Pos) {
           RetainExpansion = true;
           // We don't actually know the new pack size yet.
-          NumPartialExpansions = NewPackSize;
-          PartiallySubstitutedPackLoc = ParmPack.second;
+          PartialExpansion = {NewPackSize, Loc};
           continue;
         }
-      }
-    }
 
-    if (!NumExpansions) {
+    // FIXME: Workaround for Canonical TTP.
+    const IdentifierInfo *Name = ND ? ND->getIdentifier() : nullptr;
+    if (!CurNumExpansions) {
       // The is the first pack we've seen for which we have an argument.
       // Record it.
-      NumExpansions = NewPackSize;
-      FirstPack.first = Name;
-      FirstPack.second = ParmPack.second;
-      HaveFirstPack = true;
-      continue;
-    }
-
-    if (NewPackSize != *NumExpansions) {
+      CurNumExpansions = NewPackSize;
+      FirstPack = {Name, Loc};
+    } else if (NewPackSize != *CurNumExpansions) {
       // 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
-            << SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
-      else
-        Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
-            << Name << *NumExpansions << NewPackSize
-            << SourceRange(ParmPack.second);
+          << FirstPack.first << Name << *CurNumExpansions << NewPackSize
+          << SourceRange(FirstPack.second) << SourceRange(Loc);
       return true;
     }
   }
 
+  if (NumExpansions && CurNumExpansions &&
+      *NumExpansions != *CurNumExpansions) {
+    Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
+        << FirstPack.first << *CurNumExpansions << *NumExpansions
+        << SourceRange(FirstPack.second);
+    return true;
+  }
+
   // If we're performing a partial expansion but we also have a full expansion,
   // expand to the number of common arguments. For example, given:
   //
   //   template<typename ...T> struct A {
   //     template<typename ...U> void f(pair<T, U>...);
   //   };
   //
   // ... a call to 'A<int, int>().f<int>' should expand the pack once and
   // retain an expansion.
-  if (NumPartialExpansions) {
-    if (NumExpansions && *NumExpansions < *NumPartialExpansions) {
+  if (PartialExpansion) {
+    if (CurNumExpansions && *CurNumExpansions < PartialExpansion->first) {
       NamedDecl *PartialPack =
           CurrentInstantiationScope->getPartiallySubstitutedPack();
       Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_partial)
-        << PartialPack << *NumPartialExpansions << *NumExpansions
-        << SourceRange(PartiallySubstitutedPackLoc);
+          << PartialPack << PartialExpansion->first << *CurNumExpansions
+          << SourceRange(PartialExpansion->second);
       return true;
     }
-
-    NumExpansions = NumPartialExpansions;
+    NumExpansions = PartialExpansion->first;
+  } else {
+    NumExpansions = CurNumExpansions;
   }
 
   return false;
 }
 
 Optional<unsigned> Sema::getNumArgumentsInExpansion(QualType T,
                           const MultiLevelTemplateArgumentList &TemplateArgs) {
   QualType Pattern = cast<PackExpansionType>(T)->getPattern();
   SmallVector<UnexpandedParameterPack, 2> Unexpanded;
   CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern);
 
   Optional<unsigned> Result;
-  for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-    // Compute the depth and index for this parameter pack.
-    unsigned Depth;
-    unsigned Index;
+  auto setResultSz = [&Result](unsigned Size) {
+    assert((!Result || *Result == Size) && "inconsistent pack sizes");
+    Result = Size;
+  };
+  auto setResultPos = [&](const std::pair<unsigned, unsigned> &Pos) -> bool {
+    unsigned Depth = Pos.first, Index = Pos.second;
+    if (Depth >= TemplateArgs.getNumLevels() ||
+        !TemplateArgs.hasTemplateArgument(Depth, Index))
+      // The pattern refers to an unknown template argument. We're not ready to
+      // expand this pack yet.
+      return true;
+    // Determine the size of the argument pack.
+    setResultSz(TemplateArgs(Depth, Index).pack_size());
+    return false;
+  };
 
-    if (const TemplateTypeParmType *TTP
-          = Unexpanded[I].first.dyn_cast<const TemplateTypeParmType *>()) {
-      Depth = TTP->getDepth();
-      Index = TTP->getIndex();
+  for (auto [I, _] : Unexpanded) {
+    if (const auto *TTP = I.dyn_cast<const TemplateTypeParmType *>()) {
+      if (setResultPos({TTP->getDepth(), TTP->getIndex()}))
+        return None;
+    } else if (const auto *STP =
+                   I.dyn_cast<const SubstTemplateTypeParmPackType *>()) {
+      setResultSz(STP->getNumArgs());
+    } else if (const auto *SEP =
+                   I.dyn_cast<const SubstNonTypeTemplateParmPackExpr *>()) {
+      setResultSz(SEP->getArgumentPack().pack_size());
     } else {
-      NamedDecl *ND = Unexpanded[I].first.get<NamedDecl *>();
-      if (isa<VarDecl>(ND)) {
-        // Function parameter pack or init-capture pack.
-        typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
-
-        llvm::PointerUnion<Decl *, DeclArgumentPack *> *Instantiation
-          = CurrentInstantiationScope->findInstantiationOf(
-                                        Unexpanded[I].first.get<NamedDecl *>());
-        if (Instantiation->is<Decl*>())
+      const auto *ND = I.get<const NamedDecl *>();
+      if (isa<VarDecl>(ND)) {
+        const auto *DAP =
+            CurrentInstantiationScope->findInstantiationOf(ND)
+                ->dyn_cast<LocalInstantiationScope::DeclArgumentPack *>();
+        if (!DAP)
           // The pattern refers to an unexpanded pack. We're not ready to expand
           // this pack yet.
           return None;
-
-        unsigned Size = Instantiation->get<DeclArgumentPack *>()->size();
-        assert((!Result || *Result == Size) && "inconsistent pack sizes");
-        Result = Size;
-        continue;
+        setResultSz(DAP->size());
+      } else if (setResultPos(getDepthAndIndex(ND))) {
+        return None;
       }
-
-      std::tie(Depth, Index) = getDepthAndIndex(ND);
     }
-    if (Depth >= TemplateArgs.getNumLevels() ||
-        !TemplateArgs.hasTemplateArgument(Depth, Index))
-      // The pattern refers to an unknown template argument. We're not ready to
-      // expand this pack yet.
-      return None;
-
-    // Determine the size of the argument pack.
-    unsigned Size = TemplateArgs(Depth, Index).pack_size();
-    assert((!Result || *Result == Size) && "inconsistent pack sizes");
-    Result = Size;
   }
 
   return Result;
 }
 
 bool Sema::containsUnexpandedParameterPacks(Declarator &D) {
   const DeclSpec &DS = D.getDeclSpec();
   switch (DS.getTypeSpecType()) {

clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp

int fn() {
   FooAlias<int> b;
   bar(b);
 }
 }
 
 namespace pr56094 {
 template <typename... T> struct D {
   template <typename... U> using B = int(int (*...p)(T, U));
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
   template <typename U1, typename U2> D(B<U1, U2> *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
 using t1 = D<float>::B<int>;
 // expected-note@-1 {{in instantiation of template class 'pr56094::D<float>' requested here}}
 
 template <bool...> struct F {};
 template <class...> struct G {};
 template <bool... I> struct E {
   template <bool... U> using B = G<F<I, U>...>;
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'I' and 'U' that have different lengths (1 vs. 2)}}
   template <bool U1, bool U2> E(B<U1, U2> *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
 using t2 = E<true>::B<false>;
 // expected-note@-1 {{in instantiation of template class 'pr56094::E<true>' requested here}}
 } // namespace pr56094

clang/test/SemaTemplate/cxx1z-fold-expressions.cpp

 template<typename ...T> constexpr auto binary_conditional1(T ...t) { return (t ?: ...); } // expected-error {{expected expression}}
 template<typename ...T> constexpr auto binary_conditional2(T ...t) { return (... ?: t); } // expected-error {{expected expression}}
 template<typename ...T> constexpr auto binary_conditional3(T ...t) { return (t ?: ... ?: 0); } // expected-error {{expected expression}}
 
 namespace PR41845 {
   template <int I> struct Constant {};
 
   template <int... Is> struct Sum {
-    template <int... Js> using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter pack 'Js' that has a different length (1 vs. 2) from outer parameter packs}}
+    template <int... Js> using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter packs 'Is' and 'Js' that have different lengths (1 vs. 2)}}
   };
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
 }
 
 namespace PR30738 {
   namespace N {
     struct S {};

clang/test/SemaTemplate/pack-deduction.cpp

namespace fully_expanded_packs {
   static_assert(A<int>::j<1, 2, 3, 40>() == 43002001, "");
 }
 
 namespace partial_full_mix {
   template<typename T, typename U> struct pair {};
   template<typename ...T> struct tuple {};
   template<typename ...T> struct A {
     template<typename ...U> static pair<tuple<T...>, tuple<U...>> f(pair<T, U> ...p);
-    // expected-note@-1 {{[with U = <char, double, long>]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+    // expected-note@-1 {{[with U = <char, double, long>]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
     // expected-note@-2 {{[with U = <char, double, void>]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}}
 
     template<typename ...U> static pair<tuple<T...>, tuple<U...>> g(pair<T, U> ...p, ...);
-    // expected-note@-1 {{[with U = <char, double, long>]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+    // expected-note@-1 {{[with U = <char, double, long>]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 
     template<typename ...U> static tuple<U...> h(tuple<pair<T, U>..., pair<int, int>>);
-    // expected-note@-1 {{[with U = <int[2]>]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}}
+    // expected-note@-1 {{[with U = <int[2]>]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}}
   };
 
   pair<tuple<int, float>, tuple<char, double>> k1 = A<int, float>().f<char>(pair<int, char>(), pair<float, double>());
   pair<tuple<int, float>, tuple<char, double>> k2 = A<int, float>().f<char>(pair<int, char>(), pair<float, double>(), pair<void, long>()); // expected-error {{no match}}
   pair<tuple<int, float>, tuple<char, double>> k3 = A<int, float>().f<char, double, void>(pair<int, char>(), pair<float, double>()); // expected-error {{no match}}
 
   // FIXME: We should accept this by treating the pack 'p' as having a fixed length of 2 here.
   pair<tuple<int, float>, tuple<char, double>> k4 = A<int, float>().g<char>(pair<int, char>(), pair<float, double>(), pair<void, long>()); // expected-error {{no match}}