r361891 - If capturing a variable fails, add a capture anyway (and mark it

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 16:09:44 PDT 2019


Author: rsmith
Date: Tue May 28 16:09:44 2019
New Revision: 361891

URL: http://llvm.org/viewvc/llvm-project?rev=361891&view=rev
Log:
If capturing a variable fails, add a capture anyway (and mark it
invalid) so that we can avoid repeated diagnostics for the same capture.

Modified:
    cfe/trunk/include/clang/Sema/ScopeInfo.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/lib/Sema/SemaLambda.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
    cfe/trunk/test/Sema/captured-statements.c
    cfe/trunk/test/SemaCXX/lambda-expressions.cpp
    cfe/trunk/test/SemaObjCXX/capturing-flexible-array-in-block.mm

Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Tue May 28 16:09:44 2019
@@ -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
                                    Expr *Cpy,
                                    const bool ByCopy) {
   Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc, QualType(),
-                             Cpy, ByCopy));
+                             Cpy, ByCopy, /*Invalid*/ false));
   CXXThisCaptureIndex = Captures.size();
 }
 

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May 28 16:09:44 2019
@@ -12939,7 +12939,7 @@ static void RebuildLambdaScopeInfo(CXXMe
           /*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(),

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue May 28 16:09:44 2019
@@ -13853,10 +13853,9 @@ ExprResult Sema::ActOnBlockStmtExpr(Sour
   QualType BlockTy;
 
   // Set the captured variables on the block.
-  // FIXME: Share capture structure between BlockDecl and CapturingScopeInfo!
   SmallVector<BlockDecl::Capture, 4> 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(BlockScopeInf
                                  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(BlockScopeInf
 
     QualType PointeeTy = PT->getPointeeType();
 
-    if (PointeeTy->getAs<ObjCObjectPointerType>() &&
+    if (!Invalid && PointeeTy->getAs<ObjCObjectPointerType>() &&
         PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing &&
         !IsObjCOwnershipAttributedType(PointeeTy)) {
       if (BuildAndDiagnose) {
@@ -15323,11 +15327,10 @@ static bool captureInBlock(BlockScopeInf
 
   // 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(Capt
                                     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(Capt
 
   // 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(LambdaScopeI
                             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(LambdaScopeI
     }
 
     // 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(LambdaScopeI
   // 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<CapturingScopeInfo>(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<BlockScopeInfo>(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<CapturedRegionScopeInfo>(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<LambdaScopeInfo>(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,

Modified: cfe/trunk/lib/Sema/SemaLambda.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLambda.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLambda.cpp Tue May 28 16:09:44 2019
@@ -854,7 +854,7 @@ FieldDecl *Sema::buildInitCaptureField(L
 
   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(SourceL
     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;
 

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue May 28 16:09:44 2019
@@ -4228,6 +4228,9 @@ buildCapturedStmtCaptureList(SmallVector
                              SmallVectorImpl<Expr *> &CaptureInits,
                              ArrayRef<sema::Capture> Candidates) {
   for (const sema::Capture &Cap : Candidates) {
+    if (Cap.isInvalid())
+      continue;
+
     if (Cap.isThisCapture()) {
       Captures.push_back(CapturedStmt::Capture(Cap.getLocation(),
                                                CapturedStmt::VCK_This));

Modified: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm (original)
+++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm Tue May 28 16:09:44 2019
@@ -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}}
       }();
     }();

Modified: cfe/trunk/test/Sema/captured-statements.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/captured-statements.c?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/test/Sema/captured-statements.c (original)
+++ cfe/trunk/test/Sema/captured-statements.c Tue May 28 16:09:44 2019
@@ -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)}}

Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
+++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Tue May 28 16:09:44 2019
@@ -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]; };

Modified: cfe/trunk/test/SemaObjCXX/capturing-flexible-array-in-block.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/capturing-flexible-array-in-block.mm?rev=361891&r1=361890&r2=361891&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjCXX/capturing-flexible-array-in-block.mm (original)
+++ cfe/trunk/test/SemaObjCXX/capturing-flexible-array-in-block.mm Tue May 28 16:09:44 2019
@@ -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}}
 }




More information about the cfe-commits mailing list