diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -116,6 +116,8 @@ - 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 ------------------------------------------------- diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -238,8 +238,11 @@ // FIXME: No way to easily map from TemplateTypeParmTypes to // TemplateTypeParmDecls, so we have this horrible PointerUnion. -typedef std::pair, - 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. diff --git a/clang/include/clang/Sema/SemaInternal.h b/clang/include/clang/Sema/SemaInternal.h --- a/clang/include/clang/Sema/SemaInternal.h +++ b/clang/include/clang/Sema/SemaInternal.h @@ -62,7 +62,7 @@ } /// Retrieve the depth and index of a template parameter. -inline std::pair getDepthAndIndex(NamedDecl *ND) { +inline std::pair getDepthAndIndex(const NamedDecl *ND) { if (const auto *TTP = dyn_cast(ND)) return std::make_pair(TTP->getDepth(), TTP->getIndex()); @@ -79,7 +79,7 @@ if (const auto *TTP = UPP.first.dyn_cast()) return std::make_pair(TTP->getDepth(), TTP->getIndex()); - return getDepthAndIndex(UPP.first.get()); + return getDepthAndIndex(UPP.first.get()); } class TypoCorrectionConsumer : public VisibleDeclConsumer { diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -738,8 +738,11 @@ SmallVector 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() || + U.first.is()) + continue; + auto [Depth, Index] = getDepthAndIndex(U); if (Depth == Info.getDeducedDepth()) AddPack(Index); } diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp --- a/clang/lib/Sema/SemaTemplateVariadic.cpp +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp @@ -88,6 +88,23 @@ 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) { @@ -306,7 +323,8 @@ auto *TTPD = dyn_cast(LocalPack); return TTPD && TTPD->getTypeForDecl() == TTPT; } - return declaresSameEntity(Pack.first.get(), LocalPack); + return declaresSameEntity(Pack.first.get(), + LocalPack); }; if (llvm::any_of(LSI->LocalPacks, DeclaresThisPack)) LambdaParamPackReferences.push_back(Pack); @@ -358,7 +376,7 @@ = Unexpanded[I].first.dyn_cast()) Name = TTP->getIdentifier(); else - Name = Unexpanded[I].first.get()->getIdentifier(); + Name = Unexpanded[I].first.get()->getIdentifier(); if (Name && NamesKnown.insert(Name).second) Names.push_back(Name); @@ -421,7 +439,7 @@ llvm::SmallPtrSet ParmSet(Parms.begin(), Parms.end()); SmallVector UnexpandedParms; for (auto Parm : Unexpanded) - if (ParmSet.contains(Parm.first.dyn_cast())) + if (ParmSet.contains(Parm.first.dyn_cast())) UnexpandedParms.push_back(Parm); if (UnexpandedParms.empty()) return false; @@ -673,50 +691,54 @@ ShouldExpand = true; RetainExpansion = false; std::pair FirstPack; - bool HaveFirstPack = false; - Optional NumPartialExpansions; + Optional CurNumExpansions, NumPartialExpansions; SourceLocation PartiallySubstitutedPackLoc; for (UnexpandedParameterPack ParmPack : Unexpanded) { // Compute the depth and index for this parameter pack. unsigned Depth = 0, Index = 0; IdentifierInfo *Name; - bool IsVarDeclPack = false; + unsigned NewPackSize; + bool NotPack = true; - if (const TemplateTypeParmType *TTP = + if (const auto *TTP = ParmPack.first.dyn_cast()) { Depth = TTP->getDepth(); Index = TTP->getIndex(); Name = TTP->getIdentifier(); - } else { - NamedDecl *ND = ParmPack.first.get(); - if (isa(ND)) - IsVarDeclPack = true; - else - std::tie(Depth, Index) = getDepthAndIndex(ND); - - Name = ND->getIdentifier(); - } - - // Determine the size of this argument pack. - unsigned NewPackSize; - if (IsVarDeclPack) { + } else if (const auto *STP = + ParmPack.first + .dyn_cast()) { + NewPackSize = STP->getNumArgs(); + Name = STP->getIdentifier(); + NotPack = false; + } else if (const auto *SEP = + ParmPack.first + .dyn_cast()) { + NewPackSize = SEP->getArgumentPack().pack_size(); + Name = SEP->getParameterPack()->getIdentifier(); + NotPack = false; + } else if (const auto *ND = ParmPack.first.get(); + isa(ND)) { // Figure out whether we're instantiating to an argument pack or not. - typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack; - - llvm::PointerUnion *Instantiation = - CurrentInstantiationScope->findInstantiationOf( - ParmPack.first.get()); - if (Instantiation->is()) { - // We could expand this function parameter pack. - NewPackSize = Instantiation->get()->size(); - } else { + const auto *DAP = + CurrentInstantiationScope->findInstantiationOf(ND) + ->dyn_cast(); + if (!DAP) { // We can't expand this function parameter pack, so we can't expand // the pack expansion. ShouldExpand = false; continue; } + NewPackSize = DAP->size(); + Name = ND->getIdentifier(); + NotPack = false; } else { + Name = ND->getIdentifier(); + std::tie(Depth, Index) = getDepthAndIndex(ND); + } + + if (NotPack) { // 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. @@ -725,56 +747,55 @@ ShouldExpand = false; continue; } - // Determine the size of the argument pack. NewPackSize = TemplateArgs(Depth, Index).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) { - RetainExpansion = true; - // We don't actually know the new pack size yet. - NumPartialExpansions = NewPackSize; - PartiallySubstitutedPackLoc = ParmPack.second; - continue; + // 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 (CurrentInstantiationScope) + if (NamedDecl *PartialPack = + CurrentInstantiationScope->getPartiallySubstitutedPack()) { + unsigned PartialDepth, PartialIndex; + std::tie(PartialDepth, PartialIndex) = getDepthAndIndex(PartialPack); + if (PartialDepth == Depth && PartialIndex == Index) { + RetainExpansion = true; + // We don't actually know the new pack size yet. + NumPartialExpansions = NewPackSize; + PartiallySubstitutedPackLoc = ParmPack.second; + continue; + } } - } } - if (!NumExpansions) { + if (!CurNumExpansions) { // The is the first pack we've seen for which we have an argument. // Record it. - NumExpansions = NewPackSize; + CurNumExpansions = NewPackSize; FirstPack.first = Name; FirstPack.second = ParmPack.second; - HaveFirstPack = true; continue; } - if (NewPackSize != *NumExpansions) { + 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); + Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict) + << FirstPack.first << Name << *CurNumExpansions << NewPackSize + << SourceRange(FirstPack.second) << SourceRange(ParmPack.second); 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: // @@ -785,16 +806,18 @@ // ... a call to 'A().f' should expand the pack once and // retain an expansion. if (NumPartialExpansions) { - if (NumExpansions && *NumExpansions < *NumPartialExpansions) { + if (CurNumExpansions && *CurNumExpansions < *NumPartialExpansions) { NamedDecl *PartialPack = CurrentInstantiationScope->getPartiallySubstitutedPack(); Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_partial) - << PartialPack << *NumPartialExpansions << *NumExpansions - << SourceRange(PartiallySubstitutedPackLoc); + << PartialPack << *NumPartialExpansions << *CurNumExpansions + << SourceRange(PartiallySubstitutedPackLoc); return true; } NumExpansions = NumPartialExpansions; + } else { + NumExpansions = CurNumExpansions; } return false; @@ -807,35 +830,45 @@ CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern); Optional Result; + auto setResult = [&Result](unsigned Size) { + assert((!Result || *Result == Size) && "inconsistent pack sizes"); + Result = Size; + }; + for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) { // Compute the depth and index for this parameter pack. unsigned Depth; unsigned Index; - if (const TemplateTypeParmType *TTP - = Unexpanded[I].first.dyn_cast()) { + if (const auto *TTP = + Unexpanded[I].first.dyn_cast()) { Depth = TTP->getDepth(); Index = TTP->getIndex(); + } else if (const auto *STP = + Unexpanded[I] + .first + .dyn_cast()) { + setResult(STP->getNumArgs()); + continue; + } else if (const auto *SEP = + Unexpanded[I] + .first + .dyn_cast()) { + setResult(SEP->getArgumentPack().pack_size()); + continue; + } else if (const auto *ND = Unexpanded[I].first.get(); + isa(ND)) { + // Function parameter pack or init-capture pack. + const auto *DAP = + CurrentInstantiationScope->findInstantiationOf(ND) + ->dyn_cast(); + if (!DAP) + // The pattern refers to an unexpanded pack. We're not ready to expand + // this pack yet. + return None; + setResult(DAP->size()); + continue; } else { - NamedDecl *ND = Unexpanded[I].first.get(); - if (isa(ND)) { - // Function parameter pack or init-capture pack. - typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack; - - llvm::PointerUnion *Instantiation - = CurrentInstantiationScope->findInstantiationOf( - Unexpanded[I].first.get()); - if (Instantiation->is()) - // The pattern refers to an unexpanded pack. We're not ready to expand - // this pack yet. - return None; - - unsigned Size = Instantiation->get()->size(); - assert((!Result || *Result == Size) && "inconsistent pack sizes"); - Result = Size; - continue; - } - std::tie(Depth, Index) = getDepthAndIndex(ND); } if (Depth >= TemplateArgs.getNumLevels() || @@ -845,9 +878,7 @@ 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; + setResult(TemplateArgs(Depth, Index).pack_size()); } return Result; diff --git a/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp b/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp --- a/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp +++ b/clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp @@ -473,7 +473,7 @@ namespace pr56094 { template struct D { template 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 D(B *); // expected-note@-1 {{in instantiation of template type alias 'B' requested here}} }; @@ -484,7 +484,7 @@ template struct G {}; template struct E { template using B = G...>; - // 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 E(B *); // expected-note@-1 {{in instantiation of template type alias 'B' requested here}} }; diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp --- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp +++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp @@ -97,7 +97,7 @@ template struct Constant {}; template struct Sum { - template 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 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}} diff --git a/clang/test/SemaTemplate/pack-deduction.cpp b/clang/test/SemaTemplate/pack-deduction.cpp --- a/clang/test/SemaTemplate/pack-deduction.cpp +++ b/clang/test/SemaTemplate/pack-deduction.cpp @@ -134,14 +134,14 @@ template struct tuple {}; template struct A { template static pair, tuple> f(pair ...p); - // expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}} + // expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}} // expected-note@-2 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}} template static pair, tuple> g(pair ...p, ...); - // expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}} + // expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}} template static tuple h(tuple..., pair>); - // expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}} + // expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}} }; pair, tuple> k1 = A().f(pair(), pair());