[cfe-commits] r151076 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Sema/Sema.h lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaLambda.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cp
Douglas Gregor
dgregor at apple.com
Tue Feb 21 12:17:47 PST 2012
On Feb 21, 2012, at 12:15 PM, Eli Friedman wrote:
> On Tue, Feb 21, 2012 at 11:11 AM, Douglas Gregor <dgregor at apple.com> wrote:
>> Author: dgregor
>> Date: Tue Feb 21 13:11:17 2012
>> New Revision: 151076
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=151076&view=rev
>> Log:
>> Improve our handling of lambda expressions that occur within default
>> arguments. There are two aspects to this:
>>
>> - Make sure that when marking the declarations referenced in a
>> default argument, we don't try to mark local variables, both because
>> it's a waste of time and because the semantics are wrong: we're not
>> in a place where we could capture these variables again even if it
>> did make sense.
>> - When a lambda expression occurs in a default argument of a
>> function template, make sure that the corresponding closure type is
>> considered dependent, so that it will get properly instantiated. The
>> second bit is a bit of a hack; to fix it properly, we may have to
>> rearchitect our handling of default arguments, parsing them only
>> after creating the function definition. However, I'd like to
>> separate that work from the lambdas work.
>>
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclCXX.h
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/AST/DeclBase.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/lib/Sema/SemaLambda.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=151076&r1=151075&r2=151076&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Feb 21 13:11:17 2012
>> @@ -560,18 +560,28 @@
>> struct LambdaDefinitionData : public DefinitionData {
>> typedef LambdaExpr::Capture Capture;
>>
>> - LambdaDefinitionData(CXXRecordDecl *D)
>> - : DefinitionData(D), NumCaptures(0), NumExplicitCaptures(0),
>> - ContextDecl(0), Captures(0)
>> + LambdaDefinitionData(CXXRecordDecl *D, bool Dependent)
>> + : DefinitionData(D), Dependent(Dependent), NumCaptures(0),
>> + NumExplicitCaptures(0), ManglingNumber(0), ContextDecl(0), Captures(0)
>> {
>> IsLambda = true;
>> }
>>
>> + /// \brief Whether this lambda is known to be dependent, even if its
>> + /// context isn't dependent.
>> + ///
>> + /// A lambda with a non-dependent context can be dependent if it occurs
>> + /// within the default argument of a function template, because the
>> + /// lambda will have been created with the enclosing context as its
>> + /// declaration context, rather than function. This is an unfortunate
>> + /// artifact of having to parse the default arguments before
>> + unsigned Dependent : 1;
>> +
>> /// \brief The number of captures in this lambda.
>> unsigned NumCaptures : 16;
>>
>> /// \brief The number of explicit captures in this lambda.
>> - unsigned NumExplicitCaptures : 16;
>> + unsigned NumExplicitCaptures : 15;
>>
>> /// \brief The number used to indicate this lambda expression for name
>> /// mangling in the Itanium C++ ABI.
>> @@ -689,7 +699,7 @@
>> IdentifierInfo *Id, CXXRecordDecl* PrevDecl=0,
>> bool DelayTypeCreation = false);
>> static CXXRecordDecl *CreateLambda(const ASTContext &C, DeclContext *DC,
>> - SourceLocation Loc);
>> + SourceLocation Loc, bool DependentLambda);
>> static CXXRecordDecl *CreateDeserialized(const ASTContext &C, unsigned ID);
>>
>> bool isDynamicClass() const {
>> @@ -1478,6 +1488,21 @@
>> return getLambdaData().ContextDecl;
>> }
>>
>> + /// \brief Determine whether this lambda expression was known to be dependent
>> + /// at the time it was created, even if its context does not appear to be
>> + /// dependent.
>> + ///
>> + /// This flag is a workaround for an issue with parsing, where default
>> + /// arguments are parsed before their enclosing function declarations have
>> + /// been created. This means that any lambda expressions within those
>> + /// default arguments will have as their DeclContext the context enclosing
>> + /// the function declaration, which may be non-dependent even when the
>> + /// function declaration itself is dependent. This flag indicates when we
>> + /// know that the lambda is dependent despite that.
>> + bool isDependentLambda() const {
>> + return isLambda() && getLambdaData().Dependent;
>> + }
>> +
>> static bool classof(const Decl *D) { return classofKind(D->getKind()); }
>> static bool classofKind(Kind K) {
>> return K >= firstCXXRecord && K <= lastCXXRecord;
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=151076&r1=151075&r2=151076&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Feb 21 13:11:17 2012
>> @@ -2386,7 +2386,8 @@
>> QualType getCapturedDeclRefType(VarDecl *Var, SourceLocation Loc);
>>
>> void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T);
>> - void MarkDeclarationsReferencedInExpr(Expr *E);
>> + void MarkDeclarationsReferencedInExpr(Expr *E,
>> + bool SkipLocalVariables = false);
>>
>> /// \brief Try to recover by turning the given expression into a
>> /// call. Returns true if recovery was attempted or an error was
>> @@ -3543,7 +3544,8 @@
>> void ActOnCXXExitDeclInitializer(Scope *S, Decl *Dcl);
>>
>> /// \brief Create a new lambda closure type.
>> - CXXRecordDecl *createLambdaClosureType(SourceRange IntroducerRange);
>> + CXXRecordDecl *createLambdaClosureType(SourceRange IntroducerRange,
>> + bool KnownDependent = false);
>>
>> /// \brief Start the definition of a lambda expression.
>> CXXMethodDecl *startLambdaDefinition(CXXRecordDecl *Class,
>>
>> Modified: cfe/trunk/lib/AST/DeclBase.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=151076&r1=151075&r2=151076&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Feb 21 13:11:17 2012
>> @@ -741,10 +741,14 @@
>> if (isa<ClassTemplatePartialSpecializationDecl>(this))
>> return true;
>>
>> - if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(this))
>> + if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(this)) {
>> if (Record->getDescribedClassTemplate())
>> return true;
>> -
>> +
>> + if (Record->isDependentLambda())
>> + return true;
>> + }
>> +
>> if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(this)) {
>> if (Function->getDescribedFunctionTemplate())
>> return true;
>>
>> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=151076&r1=151075&r2=151076&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Feb 21 13:11:17 2012
>> @@ -83,11 +83,11 @@
>> }
>>
>> CXXRecordDecl *CXXRecordDecl::CreateLambda(const ASTContext &C, DeclContext *DC,
>> - SourceLocation Loc) {
>> + SourceLocation Loc, bool Dependent) {
>> CXXRecordDecl* R = new (C) CXXRecordDecl(CXXRecord, TTK_Class, DC, Loc, Loc,
>> 0, 0);
>> R->IsBeingDefined = true;
>> - R->DefinitionData = new (C) struct LambdaDefinitionData(R);
>> + R->DefinitionData = new (C) struct LambdaDefinitionData(R, Dependent);
>> C.getTypeDeclType(R, /*PrevDecl=*/0);
>> return R;
>> }
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=151076&r1=151075&r2=151076&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Feb 21 13:11:17 2012
>> @@ -3146,7 +3146,8 @@
>> // We already type-checked the argument, so we know it works.
>> // Just mark all of the declarations in this potentially-evaluated expression
>> // as being "referenced".
>> - MarkDeclarationsReferencedInExpr(Param->getDefaultArg());
>> + MarkDeclarationsReferencedInExpr(Param->getDefaultArg(),
>> + /*SkipLocalVariables=*/true);
>> return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param));
>> }
>>
>> @@ -10145,13 +10146,22 @@
>> /// potentially-evaluated subexpressions as "referenced".
>> class EvaluatedExprMarker : public EvaluatedExprVisitor<EvaluatedExprMarker> {
>> Sema &S;
>> + bool SkipLocalVariables;
>>
>> public:
>> typedef EvaluatedExprVisitor<EvaluatedExprMarker> Inherited;
>>
>> - explicit EvaluatedExprMarker(Sema &S) : Inherited(S.Context), S(S) { }
>> + EvaluatedExprMarker(Sema &S, bool SkipLocalVariables)
>> + : Inherited(S.Context), S(S), SkipLocalVariables(SkipLocalVariables) { }
>>
>> void VisitDeclRefExpr(DeclRefExpr *E) {
>> + // If we were asked not to visit local variables, don't.
>> + if (SkipLocalVariables) {
>> + if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl()))
>> + if (VD->hasLocalStorage())
>> + return;
>> + }
>> +
>> S.MarkDeclRefReferenced(E);
>> }
>>
>> @@ -10193,6 +10203,10 @@
>> }
>>
>> void VisitBlockDeclRefExpr(BlockDeclRefExpr *E) {
>> + // If we were asked not to visit local variables, don't.
>> + if (SkipLocalVariables && E->getDecl()->hasLocalStorage())
>> + return;
>
> What exactly are the rules for odr-use marking inside a lambda in a
> default argument, actually?
That is an *excellent* question. We're trying to emulate what would have happened if the expression had been written in a potentially evaluated context, by walking the resulting tree, and it's fairly likely that we don't get all of the cases right.
- Doug
More information about the cfe-commits
mailing list