r225490 - PR22117: Fix a case where we would get confused about which function parameter

Richard Smith richard-llvm at metafoo.co.uk
Thu Jan 8 17:19:56 PST 2015


Author: rsmith
Date: Thu Jan  8 19:19:56 2015
New Revision: 225490

URL: http://llvm.org/viewvc/llvm-project?rev=225490&view=rev
Log:
PR22117: Fix a case where we would get confused about which function parameter
we're instantiating, if there's a ParmVarDecl within a FunctionDecl context
that is not a parameter of that function. Add some asserts to catch this kind
of issue more generally, and fix another bug exposed by those asserts where we
were missing a local instantiation scope around substitution of
explicitly-specified template arguments.

Modified:
    cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
    cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
    cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=225490&r1=225489&r2=225490&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Jan  8 19:19:56 2015
@@ -2588,6 +2588,9 @@ Sema::SubstituteExplicitTemplateArgument
     = Function->getType()->getAs<FunctionProtoType>();
   assert(Proto && "Function template does not have a prototype?");
 
+  // Isolate our substituted parameters from our caller.
+  LocalInstantiationScope InstScope(*this, /*MergeWithOuterScope*/true);
+
   // Instantiate the types of each of the function parameters given the
   // explicitly-specified template arguments. If the function has a trailing
   // return type, substitute it after the arguments to ensure we substitute

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=225490&r1=225489&r2=225490&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Thu Jan  8 19:19:56 2015
@@ -2762,8 +2762,7 @@ bool Sema::Subst(const TemplateArgumentL
   return Instantiator.TransformTemplateArguments(Args, NumArgs, Result);
 }
 
-
-static const Decl* getCanonicalParmVarDecl(const Decl *D) {
+static const Decl *getCanonicalParmVarDecl(const Decl *D) {
   // When storing ParmVarDecls in the local instantiation scope, we always
   // want to use the ParmVarDecl from the canonical function declaration,
   // since the map is then valid for any redeclaration or definition of that
@@ -2771,7 +2770,10 @@ static const Decl* getCanonicalParmVarDe
   if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(D)) {
     if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
       unsigned i = PV->getFunctionScopeIndex();
-      return FD->getCanonicalDecl()->getParamDecl(i);
+      // This parameter might be from a freestanding function type within the
+      // function and isn't necessarily referring to one of FD's parameters.
+      if (FD->getParamDecl(i) == PV)
+        return FD->getCanonicalDecl()->getParamDecl(i);
     }
   }
   return D;
@@ -2820,12 +2822,22 @@ LocalInstantiationScope::findInstantiati
 void LocalInstantiationScope::InstantiatedLocal(const Decl *D, Decl *Inst) {
   D = getCanonicalParmVarDecl(D);
   llvm::PointerUnion<Decl *, DeclArgumentPack *> &Stored = LocalDecls[D];
-  if (Stored.isNull())
+  if (Stored.isNull()) {
+#ifndef NDEBUG
+    // It should not be present in any surrounding scope either.
+    LocalInstantiationScope *Current = this;
+    while (Current->CombineWithOuterScope && Current->Outer) {
+      Current = Current->Outer;
+      assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+             "Instantiated local in inner and outer scopes");
+    }
+#endif
     Stored = Inst;
-  else if (DeclArgumentPack *Pack = Stored.dyn_cast<DeclArgumentPack *>())
+  } else if (DeclArgumentPack *Pack = Stored.dyn_cast<DeclArgumentPack *>()) {
     Pack->push_back(Inst);
-  else
+  } else {
     assert(Stored.get<Decl *>() == Inst && "Already instantiated this local");
+  }
 }
 
 void LocalInstantiationScope::InstantiatedLocalPackArg(const Decl *D, 
@@ -2836,9 +2848,16 @@ void LocalInstantiationScope::Instantiat
 }
 
 void LocalInstantiationScope::MakeInstantiatedLocalArgPack(const Decl *D) {
+#ifndef NDEBUG
+  // This should be the first time we've been told about this decl.
+  for (LocalInstantiationScope *Current = this;
+       Current && Current->CombineWithOuterScope; Current = Current->Outer)
+    assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+           "Creating local pack after instantiation of local");
+#endif
+
   D = getCanonicalParmVarDecl(D);
   llvm::PointerUnion<Decl *, DeclArgumentPack *> &Stored = LocalDecls[D];
-  assert(Stored.isNull() && "Already instantiated this local");
   DeclArgumentPack *Pack = new DeclArgumentPack;
   Stored = Pack;
   ArgumentPacks.push_back(Pack);

Modified: cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp?rev=225490&r1=225489&r2=225490&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp Thu Jan  8 19:19:56 2015
@@ -921,4 +921,13 @@ int run2 = x2.fooG3();
 
 namespace pr21684_disambiguate_auto_followed_by_ellipsis_no_id {
 int a = [](auto ...) { return 0; }();
-}
\ No newline at end of file
+}
+
+namespace PR22117 {
+  int x = [](auto) {
+    return [](auto... run_args) {
+      using T = int(decltype(run_args)...);
+      return 0;
+    };
+  }(0)(0);
+}





More information about the cfe-commits mailing list