From: Richard Smith Date: Wed, 18 Jul 2012 01:29:05 +0000 (+0000) Subject: PR13386: When matching up parameters between a function template declaration X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=500d729e85028944355a119f9823ac99fa5ddcab;p=clang PR13386: When matching up parameters between a function template declaration and a function template instantiation, if there's a parameter pack in the declaration and one at the same place in the instantiation, don't assume that the pack wasn't expanded -- it may have expanded to nothing. Instead, go ahead and check whether the parameter pack was expandable. We can do this as a side-effect of the work we'd need to do anyway, to find how many parameters were produced. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160416 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 519e023b99..c0dae22e0d 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -37,6 +37,7 @@ #include "clang/Basic/TypeTraits.h" #include "clang/Basic/ExpressionTraits.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/OwningPtr.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" @@ -5217,10 +5218,11 @@ public: /// \brief Determine the number of arguments in the given pack expansion /// type. /// - /// This routine already assumes that the pack expansion type can be - /// expanded and that the number of arguments in the expansion is + /// This routine assumes that the number of arguments in the expansion is /// consistent across all of the unexpanded parameter packs in its pattern. - unsigned getNumArgumentsInExpansion(QualType T, + /// + /// Returns an empty Optional if the type can't be expanded. + llvm::Optional getNumArgumentsInExpansion(QualType T, const MultiLevelTemplateArgumentList &TemplateArgs); /// \brief Determine whether the given declarator contains any unexpanded diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index a2efa597c2..5a10dfbccd 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2176,35 +2176,31 @@ TemplateDeclInstantiator::SubstFunctionType(FunctionDecl *D, TypeLoc NewTL = NewTInfo->getTypeLoc().IgnoreParens(); FunctionProtoTypeLoc *NewProtoLoc = cast(&NewTL); assert(NewProtoLoc && "Missing prototype?"); - unsigned NewIdx = 0, NumNewParams = NewProtoLoc->getNumArgs(); + unsigned NewIdx = 0; for (unsigned OldIdx = 0, NumOldParams = OldProtoLoc->getNumArgs(); OldIdx != NumOldParams; ++OldIdx) { ParmVarDecl *OldParam = OldProtoLoc->getArg(OldIdx); - if (!OldParam->isParameterPack() || - // FIXME: Is this right? OldParam could expand to an empty parameter - // pack and the next parameter could be an unexpanded parameter pack - (NewIdx < NumNewParams && - NewProtoLoc->getArg(NewIdx)->isParameterPack())) { + LocalInstantiationScope *Scope = SemaRef.CurrentInstantiationScope; + + llvm::Optional NumArgumentsInExpansion; + if (OldParam->isParameterPack()) + NumArgumentsInExpansion = + SemaRef.getNumArgumentsInExpansion(OldParam->getType(), + TemplateArgs); + if (!NumArgumentsInExpansion) { // Simple case: normal parameter, or a parameter pack that's // instantiated to a (still-dependent) parameter pack. ParmVarDecl *NewParam = NewProtoLoc->getArg(NewIdx++); Params.push_back(NewParam); - SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldParam, - NewParam); - continue; - } - - // Parameter pack: make the instantiation an argument pack. - SemaRef.CurrentInstantiationScope->MakeInstantiatedLocalArgPack( - OldParam); - unsigned NumArgumentsInExpansion - = SemaRef.getNumArgumentsInExpansion(OldParam->getType(), - TemplateArgs); - while (NumArgumentsInExpansion--) { - ParmVarDecl *NewParam = NewProtoLoc->getArg(NewIdx++); - Params.push_back(NewParam); - SemaRef.CurrentInstantiationScope->InstantiatedLocalPackArg(OldParam, - NewParam); + Scope->InstantiatedLocal(OldParam, NewParam); + } else { + // Parameter pack expansion: make the instantiation an argument pack. + Scope->MakeInstantiatedLocalArgPack(OldParam); + for (unsigned I = 0; I != *NumArgumentsInExpansion; ++I) { + ParmVarDecl *NewParam = NewProtoLoc->getArg(NewIdx++); + Params.push_back(NewParam); + Scope->InstantiatedLocalPackArg(OldParam, NewParam); + } } } } @@ -2248,9 +2244,11 @@ static void addInstantiatedParametersToScope(Sema &S, FunctionDecl *Function, // Expand the parameter pack. Scope.MakeInstantiatedLocalArgPack(PatternParam); - unsigned NumArgumentsInExpansion + llvm::Optional NumArgumentsInExpansion = S.getNumArgumentsInExpansion(PatternParam->getType(), TemplateArgs); - for (unsigned Arg = 0; Arg < NumArgumentsInExpansion; ++Arg) { + assert(NumArgumentsInExpansion && + "should only be called when all template arguments are known"); + for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) { ParmVarDecl *FunctionParam = Function->getParamDecl(FParamIdx); FunctionParam->setDeclName(PatternParam->getDeclName()); Scope.InstantiatedLocalPackArg(PatternParam, FunctionParam); diff --git a/lib/Sema/SemaTemplateVariadic.cpp b/lib/Sema/SemaTemplateVariadic.cpp index a40100c7a7..0d0f992bed 100644 --- a/lib/Sema/SemaTemplateVariadic.cpp +++ b/lib/Sema/SemaTemplateVariadic.cpp @@ -597,12 +597,13 @@ bool Sema::CheckParameterPacksForExpansion(SourceLocation EllipsisLoc, return false; } -unsigned Sema::getNumArgumentsInExpansion(QualType T, +llvm::Optional Sema::getNumArgumentsInExpansion(QualType T, const MultiLevelTemplateArgumentList &TemplateArgs) { QualType Pattern = cast(T)->getPattern(); SmallVector Unexpanded; CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern); + llvm::Optional Result; for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) { // Compute the depth and index for this parameter pack. unsigned Depth; @@ -621,9 +622,14 @@ unsigned Sema::getNumArgumentsInExpansion(QualType T, llvm::PointerUnion *Instantiation = CurrentInstantiationScope->findInstantiationOf( Unexpanded[I].first.get()); - if (Instantiation->is()) - return Instantiation->get()->size(); - + if (Instantiation->is()) + // The pattern refers to an unexpanded pack. We're not ready to expand + // this pack yet. + return llvm::Optional(); + + unsigned Size = Instantiation->get()->size(); + assert((!Result || *Result == Size) && "inconsistent pack sizes"); + Result = Size; continue; } @@ -631,13 +637,17 @@ unsigned Sema::getNumArgumentsInExpansion(QualType T, } if (Depth >= TemplateArgs.getNumLevels() || !TemplateArgs.hasTemplateArgument(Depth, Index)) - continue; + // The pattern refers to an unknown template argument. We're not ready to + // expand this pack yet. + return llvm::Optional(); // Determine the size of the argument pack. - return TemplateArgs(Depth, Index).pack_size(); + unsigned Size = TemplateArgs(Depth, Index).pack_size(); + assert((!Result || *Result == Size) && "inconsistent pack sizes"); + Result = Size; } - llvm_unreachable("No unexpanded parameter packs in type expansion."); + return Result; } bool Sema::containsUnexpandedParameterPacks(Declarator &D) { diff --git a/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp b/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp index 21aa24fb52..485068e648 100644 --- a/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp +++ b/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp @@ -249,3 +249,46 @@ namespace PR10230 { int (&ir3)[3] = s().f(); } } + +namespace PR13386 { + template struct tuple {}; + template + struct S { + template + void f(T &&...t, U &&...u) {} // expected-note {{candidate}} + template + void g(U &&...u, T &&...t) {} // expected-note {{candidate}} + template + void h(tuple &&...) {} // expected-note 2{{candidate}} + + template + struct X { + template + void x(tuple &&...); // expected-error {{different lengths}} + }; + }; + + void test() { + S<>().f(); + S<>().f(0); + S().f(0); + S().f(0, 1); + S().f(0); // expected-error {{no matching member function for call}} + + S<>().g(); + S<>().g(0); + S().g(0); + S().g(0, 1); // expected-error {{no matching member function for call}} + S().g(0, 1); + S().g(0, 1); + + S<>().h(); + S<>().h(0); // expected-error {{no matching member function for call}} + S().h({}); // expected-error {{no matching member function for call}} + S().h({}); + S().h(tuple{}); + S().h(tuple{}, tuple{}); + + S::X(); // expected-note {{here}} + } +}