[cfe-commits] r160416 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Sema/SemaTemplateVariadic.cpp test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp

Richard Smith richard-llvm at metafoo.co.uk
Tue Jul 17 18:29:05 PDT 2012


Author: rsmith
Date: Tue Jul 17 20:29:05 2012
New Revision: 160416

URL: http://llvm.org/viewvc/llvm-project?rev=160416&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
    cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
    cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=160416&r1=160415&r2=160416&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Jul 17 20:29:05 2012
@@ -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 @@
   /// \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<unsigned> getNumArgumentsInExpansion(QualType T,
                             const MultiLevelTemplateArgumentList &TemplateArgs);
 
   /// \brief Determine whether the given declarator contains any unexpanded

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=160416&r1=160415&r2=160416&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Jul 17 20:29:05 2012
@@ -2176,35 +2176,31 @@
       TypeLoc NewTL = NewTInfo->getTypeLoc().IgnoreParens();
       FunctionProtoTypeLoc *NewProtoLoc = cast<FunctionProtoTypeLoc>(&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<unsigned> 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 @@
 
     // Expand the parameter pack.
     Scope.MakeInstantiatedLocalArgPack(PatternParam);
-    unsigned NumArgumentsInExpansion
+    llvm::Optional<unsigned> 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);

Modified: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp?rev=160416&r1=160415&r2=160416&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp Tue Jul 17 20:29:05 2012
@@ -597,12 +597,13 @@
   return false;
 }
 
-unsigned Sema::getNumArgumentsInExpansion(QualType T, 
+llvm::Optional<unsigned> Sema::getNumArgumentsInExpansion(QualType T,
                           const MultiLevelTemplateArgumentList &TemplateArgs) {
   QualType Pattern = cast<PackExpansionType>(T)->getPattern();
   SmallVector<UnexpandedParameterPack, 2> Unexpanded;
   CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern);
 
+  llvm::Optional<unsigned> 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 @@
         llvm::PointerUnion<Decl *, DeclArgumentPack *> *Instantiation
           = CurrentInstantiationScope->findInstantiationOf(
                                         Unexpanded[I].first.get<NamedDecl *>());
-        if (Instantiation->is<DeclArgumentPack *>())
-          return Instantiation->get<DeclArgumentPack *>()->size();
-        
+        if (Instantiation->is<Decl*>())
+          // The pattern refers to an unexpanded pack. We're not ready to expand
+          // this pack yet.
+          return llvm::Optional<unsigned>();
+
+        unsigned Size = Instantiation->get<DeclArgumentPack *>()->size();
+        assert((!Result || *Result == Size) && "inconsistent pack sizes");
+        Result = Size;
         continue;
       }
       
@@ -631,13 +637,17 @@
     }
     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<unsigned>();
     
     // 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) {

Modified: cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp?rev=160416&r1=160415&r2=160416&view=diff
==============================================================================
--- cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.decls/temp.variadic/multi-level-substitution.cpp Tue Jul 17 20:29:05 2012
@@ -249,3 +249,46 @@
     int (&ir3)[3] = s<int>().f<int, float, double>();
   }
 }
+
+namespace PR13386 {
+  template<typename...> struct tuple {};
+  template<typename...T>
+  struct S {
+    template<typename...U>
+    void f(T &&...t, U &&...u) {} // expected-note {{candidate}}
+    template<typename...U>
+    void g(U &&...u, T &&...t) {} // expected-note {{candidate}}
+    template<typename...U>
+    void h(tuple<T, U> &&...) {} // expected-note 2{{candidate}}
+
+    template<typename...U>
+    struct X {
+      template<typename...V>
+      void x(tuple<T, U, V> &&...); // expected-error {{different lengths}}
+    };
+  };
+
+  void test() {
+    S<>().f();
+    S<>().f(0);
+    S<int>().f(0);
+    S<int>().f(0, 1);
+    S<int, int>().f(0); // expected-error {{no matching member function for call}}
+
+    S<>().g();
+    S<>().g(0);
+    S<int>().g(0);
+    S<int>().g(0, 1); // expected-error {{no matching member function for call}}
+    S<int>().g<int>(0, 1);
+    S<int, int>().g(0, 1);
+
+    S<>().h();
+    S<>().h(0); // expected-error {{no matching member function for call}}
+    S<int>().h({}); // expected-error {{no matching member function for call}}
+    S<int>().h<int>({});
+    S<int>().h(tuple<int,int>{});
+    S<int, int>().h(tuple<int,int>{}, tuple<int,int>{});
+
+    S<int, int>::X<char>(); // expected-note {{here}}
+  }
+}





More information about the cfe-commits mailing list