diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -118,6 +118,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; @@ -672,109 +690,95 @@ bool &RetainExpansion, Optional &NumExpansions) { ShouldExpand = true; RetainExpansion = false; - std::pair FirstPack; - bool HaveFirstPack = false; - Optional NumPartialExpansions; - SourceLocation PartiallySubstitutedPackLoc; + std::pair FirstPack; + Optional> PartialExpansion; + Optional 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()) { - 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. + Optional> Pos; unsigned NewPackSize; - if (IsVarDeclPack) { - // 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 *ND = P.dyn_cast(); + if (ND && isa(ND)) { + 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(); + } else if (ND) { + Pos = getDepthAndIndex(ND); + } else if (const auto *TTP = P.dyn_cast()) { + 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()) { + NewPackSize = STP->getNumArgs(); + ND = STP->getReplacedParameter()->getDecl(); } else { + const auto *SEP = P.get(); + 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(); - } - - // 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) { + 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 (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); + Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict) + << 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: // @@ -784,17 +788,18 @@ // // ... a call to 'A().f' 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; @@ -807,47 +812,48 @@ CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern); Optional 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 &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()) { - Depth = TTP->getDepth(); - Index = TTP->getIndex(); + for (auto [I, _] : Unexpanded) { + if (const auto *TTP = I.dyn_cast()) { + if (setResultPos({TTP->getDepth(), TTP->getIndex()})) + return None; + } else if (const auto *STP = + I.dyn_cast()) { + setResultSz(STP->getNumArgs()); + } else if (const auto *SEP = + I.dyn_cast()) { + setResultSz(SEP->getArgumentPack().pack_size()); } else { - NamedDecl *ND = Unexpanded[I].first.get(); + const auto *ND = I.get(); + // Function parameter pack or init-capture pack. 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()) + 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; - - unsigned Size = Instantiation->get()->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; 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());