From 8465d5be63dbbd4ddb6adbab087b2a532bdc9e2b Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 28 May 2019 23:09:44 +0000 Subject: [PATCH] If capturing a variable fails, add a capture anyway (and mark it invalid) so that we can avoid repeated diagnostics for the same capture. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@361891 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/ScopeInfo.h | 31 +++-- lib/Sema/SemaDecl.cpp | 2 +- lib/Sema/SemaExpr.cpp | 108 ++++++++++-------- lib/Sema/SemaLambda.cpp | 5 +- lib/Sema/SemaStmt.cpp | 3 + .../expr/expr.prim/expr.prim.lambda/blocks.mm | 7 ++ test/Sema/captured-statements.c | 9 +- test/SemaCXX/lambda-expressions.cpp | 4 +- .../capturing-flexible-array-in-block.mm | 5 +- 9 files changed, 107 insertions(+), 67 deletions(-) diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h index 2aa1caf699..9fd34d147d 100644 --- a/include/clang/Sema/ScopeInfo.h +++ b/include/clang/Sema/ScopeInfo.h @@ -539,23 +539,28 @@ class Capture { /// the lambda. bool NonODRUsed = false; + /// Whether the capture is invalid (a capture was required but the entity is + /// non-capturable). + bool Invalid = false; + public: Capture(VarDecl *Var, bool Block, bool ByRef, bool IsNested, - SourceLocation Loc, SourceLocation EllipsisLoc, - QualType CaptureType, Expr *Cpy) + SourceLocation Loc, SourceLocation EllipsisLoc, QualType CaptureType, + Expr *Cpy, bool Invalid) : VarAndNestedAndThis(Var, IsNested ? IsNestedCapture : 0), InitExprAndCaptureKind( - Cpy, !Var ? Cap_VLA : Block ? Cap_Block : ByRef ? Cap_ByRef - : Cap_ByCopy), - Loc(Loc), EllipsisLoc(EllipsisLoc), CaptureType(CaptureType) {} + Cpy, !Var ? Cap_VLA + : Block ? Cap_Block : ByRef ? Cap_ByRef : Cap_ByCopy), + Loc(Loc), EllipsisLoc(EllipsisLoc), CaptureType(CaptureType), + Invalid(Invalid) {} enum IsThisCapture { ThisCapture }; Capture(IsThisCapture, bool IsNested, SourceLocation Loc, - QualType CaptureType, Expr *Cpy, const bool ByCopy) + QualType CaptureType, Expr *Cpy, const bool ByCopy, bool Invalid) : VarAndNestedAndThis( nullptr, (IsThisCaptured | (IsNested ? IsNestedCapture : 0))), - InitExprAndCaptureKind(Cpy, ByCopy ? Cap_ByCopy : Cap_ByRef), - Loc(Loc), CaptureType(CaptureType) {} + InitExprAndCaptureKind(Cpy, ByCopy ? Cap_ByCopy : Cap_ByRef), Loc(Loc), + CaptureType(CaptureType), Invalid(Invalid) {} bool isThisCapture() const { return VarAndNestedAndThis.getInt() & IsThisCaptured; @@ -585,6 +590,8 @@ public: return VarAndNestedAndThis.getInt() & IsNestedCapture; } + bool isInvalid() const { return Invalid; } + bool isODRUsed() const { return ODRUsed; } bool isNonODRUsed() const { return NonODRUsed; } void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } @@ -650,9 +657,9 @@ public: void addCapture(VarDecl *Var, bool isBlock, bool isByref, bool isNested, SourceLocation Loc, SourceLocation EllipsisLoc, - QualType CaptureType, Expr *Cpy) { + QualType CaptureType, Expr *Cpy, bool Invalid) { Captures.push_back(Capture(Var, isBlock, isByref, isNested, Loc, - EllipsisLoc, CaptureType, Cpy)); + EllipsisLoc, CaptureType, Cpy, Invalid)); CaptureMap[Var] = Captures.size(); } @@ -660,7 +667,7 @@ public: Captures.push_back(Capture(/*Var*/ nullptr, /*isBlock*/ false, /*isByref*/ false, /*isNested*/ false, Loc, /*EllipsisLoc*/ SourceLocation(), CaptureType, - /*Cpy*/ nullptr)); + /*Cpy*/ nullptr, /*Invalid*/ false)); } // Note, we do not need to add the type of 'this' since that is always @@ -1016,7 +1023,7 @@ CapturingScopeInfo::addThisCapture(bool isNested, SourceLocation Loc, Expr *Cpy, const bool ByCopy) { Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc, QualType(), - Cpy, ByCopy)); + Cpy, ByCopy, /*Invalid*/ false)); CXXThisCaptureIndex = Captures.size(); } diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index a035f200fc..188e801b4c 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -12939,7 +12939,7 @@ static void RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator, /*RefersToEnclosingVariableOrCapture*/true, C.getLocation(), /*EllipsisLoc*/C.isPackExpansion() ? C.getEllipsisLoc() : SourceLocation(), - CaptureType, /*Expr*/ nullptr); + CaptureType, /*Expr*/ nullptr, /*Invalid*/false); } else if (C.capturesThis()) { LSI->addThisCapture(/*Nested*/ false, C.getLocation(), diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index cc3dea9ead..95be7af8b6 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -13853,10 +13853,9 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc, QualType BlockTy; // Set the captured variables on the block. - // FIXME: Share capture structure between BlockDecl and CapturingScopeInfo! SmallVector Captures; for (Capture &Cap : BSI->Captures) { - if (Cap.isThisCapture()) + if (Cap.isInvalid() || Cap.isThisCapture()) continue; BlockDecl::Capture NewCap(Cap.getVariable(), Cap.isBlockCapture(), Cap.isNested(), Cap.getInitExpr()); @@ -15212,31 +15211,36 @@ static bool captureInBlock(BlockScopeInfo *BSI, VarDecl *Var, QualType &CaptureType, QualType &DeclRefType, const bool Nested, - Sema &S) { + Sema &S, bool Invalid) { Expr *CopyExpr = nullptr; bool ByRef = false; // Blocks are not allowed to capture arrays, excepting OpenCL. // OpenCL v2.0 s1.12.5 (revision 40): arrays are captured by reference // (decayed to pointers). - if (!S.getLangOpts().OpenCL && CaptureType->isArrayType()) { + if (!Invalid && !S.getLangOpts().OpenCL && CaptureType->isArrayType()) { if (BuildAndDiagnose) { S.Diag(Loc, diag::err_ref_array_type); S.Diag(Var->getLocation(), diag::note_previous_decl) << Var->getDeclName(); + Invalid = true; + } else { + return false; } - return false; } // Forbid the block-capture of autoreleasing variables. - if (CaptureType.getObjCLifetime() == Qualifiers::OCL_Autoreleasing) { + if (!Invalid && + CaptureType.getObjCLifetime() == Qualifiers::OCL_Autoreleasing) { if (BuildAndDiagnose) { S.Diag(Loc, diag::err_arc_autoreleasing_capture) << /*block*/ 0; S.Diag(Var->getLocation(), diag::note_previous_decl) << Var->getDeclName(); + Invalid = true; + } else { + return false; } - return false; } // Warn about implicitly autoreleasing indirect parameters captured by blocks. @@ -15259,7 +15263,7 @@ static bool captureInBlock(BlockScopeInfo *BSI, VarDecl *Var, QualType PointeeTy = PT->getPointeeType(); - if (PointeeTy->getAs() && + if (!Invalid && PointeeTy->getAs() && PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing && !IsObjCOwnershipAttributedType(PointeeTy)) { if (BuildAndDiagnose) { @@ -15323,11 +15327,10 @@ static bool captureInBlock(BlockScopeInfo *BSI, VarDecl *Var, // Actually capture the variable. if (BuildAndDiagnose) - BSI->addCapture(Var, HasBlocksAttr, ByRef, Nested, Loc, - SourceLocation(), CaptureType, CopyExpr); - - return true; + BSI->addCapture(Var, HasBlocksAttr, ByRef, Nested, Loc, SourceLocation(), + CaptureType, CopyExpr, Invalid); + return !Invalid; } @@ -15339,7 +15342,7 @@ static bool captureInCapturedRegion(CapturedRegionScopeInfo *RSI, QualType &CaptureType, QualType &DeclRefType, const bool RefersToCapturedVariable, - Sema &S) { + Sema &S, bool Invalid) { // By default, capture variables by reference. bool ByRef = true; // Using an LValue reference type is consistent with Lambdas (see below). @@ -15384,11 +15387,11 @@ static bool captureInCapturedRegion(CapturedRegionScopeInfo *RSI, // Actually capture the variable. if (BuildAndDiagnose) - RSI->addCapture(Var, /*isBlock*/false, ByRef, RefersToCapturedVariable, Loc, - SourceLocation(), CaptureType, CopyExpr); + RSI->addCapture(Var, /*isBlock*/ false, ByRef, RefersToCapturedVariable, + Loc, SourceLocation(), CaptureType, CopyExpr, + Invalid); - - return true; + return !Invalid; } /// Create a field within the lambda class for the variable @@ -15435,8 +15438,7 @@ static bool captureInLambda(LambdaScopeInfo *LSI, const Sema::TryCaptureKind Kind, SourceLocation EllipsisLoc, const bool IsTopScope, - Sema &S) { - + Sema &S, bool Invalid) { // Determine whether we are capturing by reference or by value. bool ByRef = false; if (IsTopScope && Kind != Sema::TryCapture_Implicit) { @@ -15477,31 +15479,33 @@ static bool captureInLambda(LambdaScopeInfo *LSI, } // Forbid the lambda copy-capture of autoreleasing variables. - if (CaptureType.getObjCLifetime() == Qualifiers::OCL_Autoreleasing) { + if (!Invalid && + CaptureType.getObjCLifetime() == Qualifiers::OCL_Autoreleasing) { if (BuildAndDiagnose) { S.Diag(Loc, diag::err_arc_autoreleasing_capture) << /*lambda*/ 1; S.Diag(Var->getLocation(), diag::note_previous_decl) << Var->getDeclName(); + Invalid = true; + } else { + return false; } - return false; } // Make sure that by-copy captures are of a complete and non-abstract type. - if (BuildAndDiagnose) { + if (!Invalid && BuildAndDiagnose) { if (!CaptureType->isDependentType() && S.RequireCompleteType(Loc, CaptureType, diag::err_capture_of_incomplete_type, Var->getDeclName())) - return false; - - if (S.RequireNonAbstractType(Loc, CaptureType, - diag::err_capture_of_abstract_type)) - return false; + Invalid = true; + else if (S.RequireNonAbstractType(Loc, CaptureType, + diag::err_capture_of_abstract_type)) + Invalid = true; } } // Capture this variable in the lambda. - if (BuildAndDiagnose) + if (BuildAndDiagnose && !Invalid) addAsFieldToClosureType(S, LSI, CaptureType, DeclRefType, Loc, RefersToCapturedVariable); @@ -15522,9 +15526,10 @@ static bool captureInLambda(LambdaScopeInfo *LSI, // Add the capture. if (BuildAndDiagnose) LSI->addCapture(Var, /*IsBlock=*/false, ByRef, RefersToCapturedVariable, - Loc, EllipsisLoc, CaptureType, /*CopyExpr=*/nullptr); + Loc, EllipsisLoc, CaptureType, /*CopyExpr=*/nullptr, + Invalid); - return true; + return !Invalid; } bool Sema::tryCaptureVariable( @@ -15622,11 +15627,6 @@ bool Sema::tryCaptureVariable( } return true; } - // Certain capturing entities (lambdas, blocks etc.) are not allowed to capture - // certain types of variables (unnamed, variably modified types etc.) - // so check for eligibility. - if (!isVariableCapturable(CSI, Var, ExprLoc, BuildAndDiagnose, *this)) - return true; // Try to capture variable-length arrays types. if (Var->getType()->isVariablyModifiedType()) { @@ -15697,33 +15697,45 @@ bool Sema::tryCaptureVariable( // requirements, and adding captures if requested. // If the variable had already been captured previously, we start capturing // at the lambda nested within that one. + bool Invalid = false; for (unsigned I = ++FunctionScopesIndex, N = MaxFunctionScopesIndex + 1; I != N; ++I) { CapturingScopeInfo *CSI = cast(FunctionScopes[I]); + // Certain capturing entities (lambdas, blocks etc.) are not allowed to capture + // certain types of variables (unnamed, variably modified types etc.) + // so check for eligibility. + if (!Invalid) + Invalid = + !isVariableCapturable(CSI, Var, ExprLoc, BuildAndDiagnose, *this); + + // After encountering an error, if we're actually supposed to capture, keep + // capturing in nested contexts to suppress any follow-on diagnostics. + if (Invalid && !BuildAndDiagnose) + return true; + if (BlockScopeInfo *BSI = dyn_cast(CSI)) { - if (!captureInBlock(BSI, Var, ExprLoc, - BuildAndDiagnose, CaptureType, - DeclRefType, Nested, *this)) - return true; + Invalid = !captureInBlock(BSI, Var, ExprLoc, BuildAndDiagnose, CaptureType, + DeclRefType, Nested, *this, Invalid); Nested = true; } else if (CapturedRegionScopeInfo *RSI = dyn_cast(CSI)) { - if (!captureInCapturedRegion(RSI, Var, ExprLoc, - BuildAndDiagnose, CaptureType, - DeclRefType, Nested, *this)) - return true; + Invalid = !captureInCapturedRegion(RSI, Var, ExprLoc, BuildAndDiagnose, + CaptureType, DeclRefType, Nested, + *this, Invalid); Nested = true; } else { LambdaScopeInfo *LSI = cast(CSI); - if (!captureInLambda(LSI, Var, ExprLoc, - BuildAndDiagnose, CaptureType, + Invalid = + !captureInLambda(LSI, Var, ExprLoc, BuildAndDiagnose, CaptureType, DeclRefType, Nested, Kind, EllipsisLoc, - /*IsTopScope*/I == N - 1, *this)) - return true; + /*IsTopScope*/ I == N - 1, *this, Invalid); Nested = true; } + + if (Invalid && !BuildAndDiagnose) + return true; } - return false; + return Invalid; } bool Sema::tryCaptureVariable(VarDecl *Var, SourceLocation Loc, diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp index b2055dd650..f6c9dee2a0 100644 --- a/lib/Sema/SemaLambda.cpp +++ b/lib/Sema/SemaLambda.cpp @@ -854,7 +854,7 @@ FieldDecl *Sema::buildInitCaptureField(LambdaScopeInfo *LSI, VarDecl *Var) { LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(), /*isNested*/false, Var->getLocation(), SourceLocation(), - Var->getType(), Var->getInit()); + Var->getType(), Var->getInit(), /*Invalid*/false); return Field; } @@ -1586,6 +1586,9 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) { const Capture &From = LSI->Captures[I]; + if (From.isInvalid()) + return ExprError(); + assert(!From.isBlockCapture() && "Cannot capture __block variables"); bool IsImplicit = I >= LSI->NumExplicitCaptures; diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index c7fb565451..51a72c618b 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -4228,6 +4228,9 @@ buildCapturedStmtCaptureList(SmallVectorImpl &Captures, SmallVectorImpl &CaptureInits, ArrayRef Candidates) { for (const sema::Capture &Cap : Candidates) { + if (Cap.isInvalid()) + continue; + if (Cap.isThisCapture()) { Captures.push_back(CapturedStmt::Capture(Cap.getLocation(), CapturedStmt::VCK_This)); diff --git a/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm b/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm index 96e8fcd8d3..cb56f6816a 100644 --- a/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm +++ b/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm @@ -50,6 +50,13 @@ void nesting() { [=] () mutable { ^ { int i = array[2]; // expected-error{{cannot refer to declaration with an array type inside block}} + i += array[3]; + }(); + }(); + + [=] () mutable { + ^ { + int i = 0; i += array[3]; // expected-error{{cannot refer to declaration with an array type inside block}} }(); }(); diff --git a/test/Sema/captured-statements.c b/test/Sema/captured-statements.c index 86e9273944..ac04915097 100644 --- a/test/Sema/captured-statements.c +++ b/test/Sema/captured-statements.c @@ -65,11 +65,18 @@ void test_nest_block() { int b; #pragma clang __debug captured { - __block int c; int d; ^{ a = b; // expected-error{{__block variable 'a' cannot be captured in a captured statement}} + a = b; // (duplicate diagnostic suppressed) b = d; // OK - Consistent with block inside a lambda + }(); + } + #pragma clang __debug captured + { + __block int c; + int d; + ^{ c = a; // expected-error{{__block variable 'a' cannot be captured in a captured statement}} c = d; // OK d = b; // expected-error{{variable is not assignable (missing __block type specifier)}} diff --git a/test/SemaCXX/lambda-expressions.cpp b/test/SemaCXX/lambda-expressions.cpp index 1833400be3..8b0b83078b 100644 --- a/test/SemaCXX/lambda-expressions.cpp +++ b/test/SemaCXX/lambda-expressions.cpp @@ -65,9 +65,9 @@ namespace ImplicitCapture { d = 3; [=]() { return c; }; // expected-error {{unnamed variable cannot be implicitly captured in a lambda expression}} - __block int e; // expected-note 3 {{declared}} + __block int e; // expected-note 2{{declared}} [&]() { return e; }; // expected-error {{__block variable 'e' cannot be captured in a lambda expression}} - [&e]() { return e; }; // expected-error 2 {{__block variable 'e' cannot be captured in a lambda expression}} + [&e]() { return e; }; // expected-error {{__block variable 'e' cannot be captured in a lambda expression}} int f[10]; // expected-note {{declared}} [&]() { return f[2]; }; diff --git a/test/SemaObjCXX/capturing-flexible-array-in-block.mm b/test/SemaObjCXX/capturing-flexible-array-in-block.mm index d7d888564c..cf88d4684c 100644 --- a/test/SemaObjCXX/capturing-flexible-array-in-block.mm +++ b/test/SemaObjCXX/capturing-flexible-array-in-block.mm @@ -2,7 +2,8 @@ // rdar://12655829 void f() { - struct { int x; int y[]; } a; // expected-note 2 {{'a' declared here}} + struct { int x; int y[]; } a; // expected-note 3 {{'a' declared here}} ^{return a.x;}(); // expected-error {{cannot refer to declaration of structure variable with flexible array member inside block}} - [] {return a.x;}(); // expected-error {{variable 'a' with flexible array member cannot be captured in a lambda expression}} + [=] {return a.x;}(); // expected-error {{variable 'a' with flexible array member cannot be captured in a lambda expression}} + [] {return a.x;}(); // expected-error {{variable 'a' cannot be implicitly captured in a lambda with no capture-default}} expected-note {{here}} } -- 2.40.0