[PATCH] Implement transformations of non-capturing nested lambdas.

Richard Smith richard at metafoo.co.uk
Mon Oct 7 12:31:49 PDT 2013



================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:950-953
@@ +949,6 @@
+                                                      OldCallOperatorTemplate);
+        // Set this as a specialization so we don't go digging into the 
+        // OldCallOperatorTemplate when retrieving the 
+        // 'FunctionDecl::getTemplateInstantiationPattern()' 
+        NewCallOperatorTemplate->setMemberSpecialization();
+      }
----------------
It's a bit nasty to (essentially) set this bit incorrectly. Can you put the check into getTemplateInstantiationPattern instead?

================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:960-961
@@ +959,4 @@
+      TemplateParameterList *NewTPL = 0;
+      if (OrigTPL) {
+        if (!OrigTPL->size()) return OrigTPL; // size 0, do nothing
+         
----------------
Fold these together into

    if (!OrigTPL || !OrigTPL->size()) return OrigTPL;

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4192-4193
@@ +4191,4 @@
+  // fooT('a'); 
+  // ... without this check below, findInstantiationOf fails with
+  // an assertion violation.
+  if (isa<ParmVarDecl>(D) && !ParentDC->isDependentContext() &&
----------------
Drop this line. It's enough for the check to be correct; you don't need to explain what would happen if we didn't have it.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4195
@@ +4194,3 @@
+  if (isa<ParmVarDecl>(D) && !ParentDC->isDependentContext() &&
+      !cast<ParmVarDecl>(D)->getType()->isInstantiationDependentType())
+    return D;
----------------
An instantiation dependent type should not be possible here if the parent is not a dependent context.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4198
@@ -4174,2 +4197,3 @@
+
   if (isa<ParmVarDecl>(D) || isa<NonTypeTemplateParmDecl>(D) ||
       isa<TemplateTypeParmDecl>(D) || isa<TemplateTemplateParmDecl>(D) ||
----------------
I think a simpler way of making the above change is to just remove the ParmVarDecl case here. Dependent parameters are caught by the

    (ParentDC->isFunctionOrMethod() && ParentDC->isDependentContext())

check here, and non-dependent parameters will fall through to the 'return D;' case below.

================
Comment at: lib/Sema/TreeTransform.h:4582-4594
@@ -4575,2 +4581,15 @@
   const DecltypeType *T = TL.getTypePtr();
+  // Don't transform a decltype construct that has already been transformed 
+  // into a non-dependent type.
+  // Allows the following to compile:
+  // auto L = [](auto a) {
+  //   return [](auto b) ->decltype(a) {
+  //     return b;
+  //  };
+  //};  
+  if (!T->isInstantiationDependentType()) {
+    DecltypeTypeLoc NewTL = TLB.push<DecltypeTypeLoc>(TL.getType());
+    NewTL.setNameLoc(TL.getNameLoc());
+    return NewTL.getType();
+  }
 
----------------
Is this change necessary, given your fix to `FindInstantiatedDecl`? There doesn't seem to be anything specific to `decltype` in this example. If this doesn't work, I'd expect

  auto L = [](auto a) {
    return [](auto b) -> int (&)[sizeof(a)] { ... }
  }

to fail in the same way.

================
Comment at: lib/Sema/TreeTransform.h:4590
@@ +4589,3 @@
+  //};  
+  if (!T->isInstantiationDependentType()) {
+    DecltypeTypeLoc NewTL = TLB.push<DecltypeTypeLoc>(TL.getType());
----------------
It's not OK for TreeTransform to look at the dependence of things. It's not just used for instantiation.

================
Comment at: lib/Sema/TreeTransform.h:8328
@@ +8327,3 @@
+  
+  // Use the Old Call Operator's TypeSourceInfo if it is already transformed.
+  if (CallOpWasAlreadyTransformed)  
----------------
Strange capitalization here?

================
Comment at: lib/Sema/TreeTransform.h:8322
@@ +8321,3 @@
+  FunctionProtoTypeLoc OldCallOpFPTL = 
+      OldCallOpTSI->getTypeLoc().getAs<FunctionProtoTypeLoc>();
+  TypeSourceInfo *NewCallOpTSI = 0;
----------------
castAs.

================
Comment at: lib/Sema/TreeTransform.h:8329-8343
@@ -8293,1 +8328,17 @@
+  // Use the Old Call Operator's TypeSourceInfo if it is already transformed.
+  if (CallOpWasAlreadyTransformed)  
+    NewCallOpTSI = OldCallOpTSI;  
+  else {
+    // Transform the TypeSourceInfo of the Original Lambda's Call Operator.
+    // The transformation MUST be done in the CurrentInstantiationScope since
+    // it introduces a mapping of the original to the newly created 
+    // transformed parameters.
+
+    TypeLocBuilder NewCallOpTLBuilder;
+    QualType NewCallOpType = TransformFunctionProtoType(NewCallOpTLBuilder, 
+                                                        OldCallOpFPTL, 
+                                                        0, 0);
+    NewCallOpTSI = NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context,
+                                                        NewCallOpType);
   }
+  // Extract the ParmVarDecls from the NewCallOpTSI and add them to
----------------
This looks equivalent to just:

  NewCallOpTSI = getDerived().TransformType(OldCallOpTSI);

================
Comment at: lib/Sema/TreeTransform.h:8348-8362
@@ +8347,17 @@
+  // lambda call operator to the CurrentInstantiationScope.
+  SmallVector<ParmVarDecl *, 4> Params;  
+  {
+    FunctionProtoTypeLoc NewCallOpFPTL = 
+        NewCallOpTSI->getTypeLoc().getAs<FunctionProtoTypeLoc>();
+    ParmVarDecl **NewParamDeclArray = NewCallOpFPTL.getParmArray();
+    const unsigned NewNumArgs = NewCallOpFPTL.getNumArgs();
+
+    for (unsigned I = 0; I < NewNumArgs; ++I) {
+      if (CallOpWasAlreadyTransformed)
+        SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+                                                         NewParamDeclArray[I],
+                                                         NewParamDeclArray[I]);
+      Params.push_back(NewParamDeclArray[I]);
+    }
+  }
+
----------------
... and with this piece, this looks like

  NewCallOpTSI = SubstFunctionType(E->getCallOperator(), Params);

(`SubstFunctionType` is on `TemplateDeclInstantiator`.) You're also missing the pack instantiation case here. For instance, given:

  template<int...N> void f() {
    [](auto ...a[N]) {};
  }

we need to instantiate `a` into a pack here.

================
Comment at: lib/Sema/TreeTransform.h:8357
@@ +8356,3 @@
+      if (CallOpWasAlreadyTransformed)
+        SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+                                                         NewParamDeclArray[I],
----------------
This is very instantiation-specific. It's not really OK to do this in TreeTransform. You should instead move this out into a hook that we can handle in a template-instantiation-specific way.


http://llvm-reviews.chandlerc.com/D1784



More information about the cfe-commits mailing list