[cfe-commits] [PATCH] Sema and AST for C++0x Lambda Expressions
Douglas Gregor
dgregor at apple.com
Fri Sep 23 16:32:30 PDT 2011
On Sep 7, 2011, at 7:36 PM, John Freeman wrote:
> Attached is an updated patch addressing suggestions.
>
> I tried to start working on handling non-trivial lambda bodies, but ran into trouble with Scope/DeclContext nesting. How should I handle the semantic and lexical contexts for the lambda's closure type and its members (constructor, fields, function call operator)? When should DeclContext::addDecl be used, if at all?
>
> - John
> <lambda-sema.patch>
diff --git include/clang/AST/ExprCXX.h include/clang/AST/ExprCXX.h
index 1911704..821f312 100644
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -3123,7 +3123,113 @@ public:
// Iterators
child_range children() { return child_range(&Temporary, &Temporary + 1); }
};
-
+
+ /// Convenience function for variable captures.
+ VarDecl *getVar() const {
+ DeclRefExpr *VarRef = cast<DeclRefExpr>(getInit());
+ return cast<VarDecl>(VarRef->getDecl());
+ }
For sanity's sake, there should be an assert() here to check that this is actually a variable capture. A method isVariableCapture() would also be useful for such cases.
+ /// Array of explicit captures in source order.
+ Capture *Explicits;
+ unsigned NumExplicits;
I'd prefer that we allocate these at the end of the LambdaExpr object, to reduce the amount of storage it needs.
+ /// Declaration for the closure type.
+ CXXRecordDecl *ClosureType;
This is just getType()->getAsCXXRecordDecl(), no?
+ /// The temporary object resulting from evaluating the lambda expression,
+ /// represented by a call to the constructor of the closure type.
+ CXXConstructExpr *ClosureObject;
This makes me think that LambdaExpr should just be a subclass of CXXConstructExpr, because there's a lot of behavior we get "for free" for lambdas if it derives from CXXConstructExpr (such as proper handling of the temporary). It would require some twisting-around of the logic for building lambda expressions (in particular, the LambdaExpr wouldn't be built until the whole body had been parsed), but I think the result would be better.
+protected:
+ LambdaExpr(CXXRecordDecl *ClosureType, QualType T,
+ Capture *Explicits, unsigned NumExplicits)
+ : Expr(LambdaExprClass, T, VK_RValue, OK_Ordinary,
This would be a great place to use ArrayRef.
+ : Expr(LambdaExprClass, T, VK_RValue, OK_Ordinary,
+ // FIXME: Support templates and parameter packs later.
+ /*TypeDependent=*/false,
+ /*ValueDependent=*/false,
+ /*InstantiationDependent=*/false,
It seems to me that if T is dependent (i.e., if the lambda is anywhere inside any template), then the expression is type/value/instantiation-dependent, because the instantiated type for the lambda will be different for each instantiation of the template.
+ /*ContainsUnexpandedParameterPack=*/false),
This will always be false, FWIW; no need for a FIXME here.
@@ -673,8 +673,20 @@ bool Parser::TryParseLambdaIntroducer(LambdaIntroducer &Intro) {
/// expression.
ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
LambdaIntroducer &Intro) {
+ ExprResult MaybeLambda = Actions.ActOnLambdaIntroducer(getCurScope(), Intro);
+ if (!MaybeLambda.isUsable())
+ return ExprError();
+
+ LambdaExpr *Lambda = reinterpret_cast<LambdaExpr*>(MaybeLambda.take());
+
Hrm. I don't particularly love the idea of having the parser know about LambdaExpr.
+ UnqualifiedId Name;
+ CXXScopeSpec ScopeSpec;
+ Name.setIdentifier(C->Id, C->Loc);
+ DeclRefExpr *VarRef
+ = static_cast<DeclRefExpr*>(
+ ActOnIdExpression(S, ScopeSpec, Name,
+ /*HasTrailingLParen=*/false,
+ /*IsAddressOfOperand=*/false).take());
+
This should check whether the result of ActOnIdExpression is (1) actually valid, and (2) actually a DeclRefExpr.
+ CXXRecordDecl *Class = CXXRecordDecl::Create(Context,
+ TTK_Class,
+ DC,
+ Intro.Range.getBegin(),
+ /*IdLoc=*/SourceLocation(),
+ /*Id=*/0);
+ Class->startDefinition();
+ // FIXME: This is how we track the end of the lambda. It is set repeatedly as
+ // we move along. Is it ok?
It's probably best just to set it once, at the end of parsing.
+ // FIXME: We've got Decl::getSourceRange (that splits into Decl::getLocStart
+ // and Decl::getLocEnd), Decl::getLocation, and Decl::getBodyRBrace;
+ // DeclaratorDecl::getInnerLocStart and DeclaratorDecl::getOuterLocStart;
+ // TagDecl::getRBraceLoc. Some of these do not have corresponding setters,
+ // some do---even in other subclasses, e.g., FunctionDecl::setRangeEnd.
+ // How are they related? I have a feeling some redundancy exists.
Generally, the ones that have setters are the ones that it makes sense to mutate.
+ Method->setAccess(AS_public);
+ Method->setLexicalDeclContext(CurContext);
+ // FIXME: How to add Method as a member of Class? Cannot use
+ // DeclContext::addDecl. Apparently, that is only for lexical contexts. How
+ // should we handle DeclContexts for Class, Method, Field, and Ctor?
You want both the lexical and semantic contexts of the method to be the lambda's class. Then you can addDecl on the class to add the method to the class.
+ // FIXME: Remove this check later.
+ bool Redeclaration = false;
+ LookupResult Previous(*this,
+ MethodName,
+ SourceLocation(),
+ LookupOrdinaryName,
+ ForRedeclaration);
+ CheckFunctionDeclaration(S, Method, Previous,
+ /*IsExplicitSpecialization=*/false,
+ Redeclaration);
+ assert(!Method->isInvalidDecl() && "bad lambda constructor");
+
+ Lambda->setFunction(Method);
It's fine to leave this check in. I'm going back and forth in my head about whether it's useful or not.
+void Sema::ActOnLambdaBody(Scope *S, LambdaExpr *Lambda, Stmt *Body) {
+ CXXRecordDecl *Class = Lambda->getClosureType();
+ Class->setRBraceLoc(Body->getLocEnd());
+
+ CanQualType ClassType
+ = Context.getCanonicalType(Context.getRecordType(Class));
+
+ CXXMethodDecl *Method = Lambda->getFunction();
+ Method->setBody(Body);
+
+ DeclarationName CtorName
+ = Context.DeclarationNames.getCXXConstructorName(ClassType);
+ CXXConstructorDecl *Ctor
+ = CXXConstructorDecl::Create(Context,
+ Class,
+ /*StartLoc=*/SourceLocation(),
+ DeclarationNameInfo(CtorName,
+ /*NameLoc=*/SourceLocation()),
+ // Chicken and egg problem: we need the Ctor to
+ // build the CtorParams and the CtorParams to
+ // build the Ctor. Favor one call on the Ctor
+ // to set its type after construction instead
+ // of one call on each CtorParam to set their
+ // DeclContext.
+ QualType(),
+ /*TInfo=*/0,
+ /*isExplicit=*/true,
+ /*isInline=*/true,
+ /*isImplicitlyDeclared=*/true,
+ /*isConstExpr=*/false);
+
+ // FIXME: When we support implicit captures later, this number will need to be
+ // updated.
+ unsigned NumCaptures = Lambda->getNumExplicits();
FWIW, this issue with having to update the implicit captures will go away if we build the lambda expression very late, in this routine.
+ // FIXME: Find the correct type. There are capture type transformations
+ // remaining to be implemented. What is the difference between CanQualType
+ // and QualType? How is template instantiation handled? If some of these
+ // have dependent type, how is that treated?
+ QualType CaptureType = Init->getType();
+ if (C->isReferenceCapture()) {
+ // FIXME: Compare with Sema::BuildReferenceType.
+ CaptureType = Context.getLValueReferenceType(CaptureType,
+ /*SpelledAsLValue=*/true);
CanQualType is a "canonical" type, an idea that is described at:
http://clang.llvm.org/docs/InternalsManual.html#Type
As for template instantiation, we'll just end up instantiating each "Init" and recomputing the type from the instantiation. It should be a piece of cake.
+ FieldDecl *Field = CheckFieldDecl(CaptureName,
+ CaptureType,
+ /*TInfo=*/0,
+ Class,
+ CaptureLoc,
+ /*Mutable=*/false,
+ /*BitWidth=*/0,
+ /*HasInit=*/false,
+ // FIXME: Pass IdLoc? We haven't for other
+ // declarations so far, but fields are the
+ // only ones we have names for in the
+ // source.
+ CaptureLoc,
+ AS_private,
+ /*PrevDecl=*/0);
'twould probably be fine to just build the FieldDecl directly, but this is more pedantic (and we can do that later if we need to).
+ {
+ // FIXME: Have to use Expr* instead of DeclRefExpr* because we cannot
+ // convert from DeclRefExpr** to Expr**. Why?
… because then someone could write a ParenExpr* through the Expr**, and the DeclRefExpr** would no longer be pointing to a DeclRefExpr*.
+ // FIXME: Compare with Sema::CheckParmsForFunctionDef.
+ Ctor->setParams(CtorParams.data(), CtorParams.size());
Yes, CheckParmsForFunctionDef will do some extra checking we need.
+ // FIXME: Compare with Sema::ActOnMemInitializers.
+ // FIXME: Compare with Sema::SetCtorInitializers.
Since you're cooking up the exact ctor-initializer list, we probably don't need any of the extra checking there.
Despite my pile of comments, your patch is getting fairly close. I tweaked your patch a bit to implement a few of my suggestions above (mostly in compressing the storage of LambdaExpr down a bit). At this point, I'm pondering the CXXConstructExpr issue; the other things could be dealt with incrementally, but I'd rather know which way we're going with CXXConstructExpr before committing.
- Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lambda-expr-tweaked.patch
Type: application/octet-stream
Size: 31874 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110923/a1d71518/attachment.obj>
-------------- next part --------------
More information about the cfe-commits
mailing list