[cfe-commits] [PATCH] Sema and AST for C++0x Lambda Expressions

[John Freeman]

I'm sorry you're getting this twice, Doug, I hit reply instead of 
reply-all. :/

On 8/12/11 5:26 PM, Douglas Gregor wrote:
>   #include "clang/AST/Expr.h"
> +// FIXME: Including this only for CXXThisExpr. Is that bad?
> +#include "clang/AST/ExprCXX.h"
>   #include "clang/AST/Decl.h"
>   #include "clang/AST/TypeLoc.h"
>   #include "clang/AST/UnresolvedSet.h"
>
> How about just forward-declaring CXXThisExpr, rather than #including the whole header?

I need the header for the base class information since I store the 
CXXThisExpr* as an Expr*. However, this may be a moot issue after the 
discussion below.

> +    /// The initializing Expr for the captured entity. Will be either
> +    /// CXXThisExpr or DeclRefExpr.
> +    Expr* getInit() const { return Init; }
>
> I'm not sure I understand the point of the DeclRefExpr. Why not just store the VarDecl* that's actually being captured, along with a SourceLocation showing where the capture occurred?

> +    UnqualifiedId Name;
> +    CXXScopeSpec ScopeSpec;
> +    Name.setIdentifier(C->Id, C->Loc);
> +    // FIXME: Can we do better than ActOnIdExpression?
> +    DeclRefExpr *Var
> +      = static_cast<DeclRefExpr*>(
> +          ActOnIdExpression(getCurScope(),
> +                            ScopeSpec, Name,
> +                            /*HasTrailingLParen=*/false,
> +                            /*IsAddressOfOperand=*/false).take());
> +
> +    // FIXME: Check that we have a local variable with automatic storage
> +    // duration.
>
> It's probably best just to call LookupName() directly, and then look at the result of name lookup, than to actually build an id-expression. That way, the capture list itself can just store declaration + source location information for each (successful!) capture.

> +  CXXCtorInitializer **CtorInit = CtorInits;
> +  for (LambdaDecl::Capture *C = Lambda->Explicits,
> +       *E = Lambda->Explicits + Lambda->NumExplicits; C != E; ++C) {
> +
> +    Expr *Init = C->getInit();
> +
> +    IdentifierInfo *Id;
> +    if (C->isThisCapture()) {
> +      Id = PP.getIdentifierInfo("__this");
> +    } else {
> +      Id = cast<DeclRefExpr>(Init)->getDecl()->getIdentifier();
> +    }
>
> This is one of the places where just storing the VarDecl in the capture list would make things easier.

The question here is whether to store captures as Exprs (CXXThisExpr and 
DeclRefExpr) or as Decls (CXXRecordDecl and VarDecl). We need the Exprs 
as arguments to the CXXConstructExpr that constructs the closure object, 
and I figured if we're going to do the lookup for the Decl, we might as 
well build the Expr and make use of existing semantic analysis at the 
same time. There may be a better way, though. What do you think?

- John