[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