An insomniac stab at making block declarations list the variables they close
on, as well as more reliably limiting invalid references to locals from
nested scopes.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124721 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index b1da849..db69e08 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -2104,8 +2104,21 @@
}
}
-unsigned BlockDecl::getNumParams() const {
- return NumParams;
+void BlockDecl::setCapturedDecls(ASTContext &Context,
+ VarDecl * const *begin,
+ VarDecl * const *end,
+ bool capturesCXXThis) {
+ CapturesCXXThis = capturesCXXThis;
+
+ if (begin == end) {
+ NumCapturedDecls = 0;
+ CapturedDecls = 0;
+ return;
+ }
+
+ NumCapturedDecls = end - begin;
+ CapturedDecls = new (Context) VarDecl*[NumCapturedDecls];
+ memcpy(CapturedDecls, begin, NumCapturedDecls * sizeof(VarDecl*));
}
SourceRange BlockDecl::getSourceRange() const {
diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 7c3fc63..0a7a9d2 100644
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -462,7 +462,7 @@
{ return Success(E); }
bool VisitCallExpr(CallExpr *E);
bool VisitBlockExpr(BlockExpr *E) {
- if (!E->hasBlockDeclRefExprs())
+ if (!E->getBlockDecl()->hasCaptures())
return Success(E);
return false;
}
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 0ca1ce5..24ce720 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1935,7 +1935,7 @@
}
case Stmt::BlockExprClass:
- if (cast<BlockExpr>(E)->hasBlockDeclRefExprs())
+ if (cast<BlockExpr>(E)->getBlockDecl()->hasCaptures())
return E; // local block.
return NULL;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index ec5e3e0..49637f2 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -733,58 +733,106 @@
StringTokLocs.size()));
}
-/// ShouldSnapshotBlockValueReference - Return true if a reference inside of
-/// CurBlock to VD should cause it to be snapshotted (as we do for auto
-/// variables defined outside the block) or false if this is not needed (e.g.
-/// for values inside the block or for globals).
+enum CaptureResult {
+ /// No capture is required.
+ CR_NoCapture,
+
+ /// A capture is required.
+ CR_Capture,
+
+ /// An error occurred when trying to capture the given variable.
+ CR_Error
+};
+
+/// Diagnose an uncapturable value reference.
///
-/// This also keeps the 'hasBlockDeclRefExprs' in the BlockScopeInfo records
-/// up-to-date.
-///
-static bool ShouldSnapshotBlockValueReference(Sema &S, BlockScopeInfo *CurBlock,
- ValueDecl *VD) {
- // If the value is defined inside the block, we couldn't snapshot it even if
- // we wanted to.
- if (CurBlock->TheDecl == VD->getDeclContext())
- return false;
+/// \param var - the variable referenced
+/// \param DC - the context which we couldn't capture through
+static CaptureResult
+DiagnoseUncapturableValueReference(Sema &S, SourceLocation loc,
+ VarDecl *var, DeclContext *DC) {
+ switch (S.ExprEvalContexts.back().Context) {
+ case Sema::Unevaluated:
+ // The argument will never be evaluated, so don't complain.
+ return CR_NoCapture;
- // If this is an enum constant or function, it is constant, don't snapshot.
- if (isa<EnumConstantDecl>(VD) || isa<FunctionDecl>(VD))
- return false;
+ case Sema::PotentiallyEvaluated:
+ case Sema::PotentiallyEvaluatedIfUsed:
+ break;
- // If this is a reference to an extern, static, or global variable, no need to
- // snapshot it.
- // FIXME: What about 'const' variables in C++?
- if (const VarDecl *Var = dyn_cast<VarDecl>(VD))
- if (!Var->hasLocalStorage())
- return false;
-
- // Blocks that have these can't be constant.
- CurBlock->hasBlockDeclRefExprs = true;
-
- // If we have nested blocks, the decl may be declared in an outer block (in
- // which case that outer block doesn't get "hasBlockDeclRefExprs") or it may
- // be defined outside all of the current blocks (in which case the blocks do
- // all get the bit). Walk the nesting chain.
- for (unsigned I = S.FunctionScopes.size() - 1; I; --I) {
- BlockScopeInfo *NextBlock = dyn_cast<BlockScopeInfo>(S.FunctionScopes[I]);
-
- if (!NextBlock)
- continue;
-
- // If we found the defining block for the variable, don't mark the block as
- // having a reference outside it.
- if (NextBlock->TheDecl == VD->getDeclContext())
- break;
-
- // Otherwise, the DeclRef from the inner block causes the outer one to need
- // a snapshot as well.
- NextBlock->hasBlockDeclRefExprs = true;
+ case Sema::PotentiallyPotentiallyEvaluated:
+ // FIXME: delay these!
+ break;
}
- return true;
+ // Don't diagnose about capture if we're not actually in code right
+ // now; in general, there are more appropriate places that will
+ // diagnose this.
+ if (!S.CurContext->isFunctionOrMethod()) return CR_NoCapture;
+
+ // This particular madness can happen in ill-formed default
+ // arguments; claim it's okay and let downstream code handle it.
+ if (isa<ParmVarDecl>(var) &&
+ S.CurContext == var->getDeclContext()->getParent())
+ return CR_NoCapture;
+
+ DeclarationName functionName;
+ if (FunctionDecl *fn = dyn_cast<FunctionDecl>(var->getDeclContext()))
+ functionName = fn->getDeclName();
+ // FIXME: variable from enclosing block that we couldn't capture from!
+
+ S.Diag(loc, diag::err_reference_to_local_var_in_enclosing_function)
+ << var->getIdentifier() << functionName;
+ S.Diag(var->getLocation(), diag::note_local_variable_declared_here)
+ << var->getIdentifier();
+
+ return CR_Error;
}
+/// ShouldCaptureValueReference - Determine if a reference to the
+/// given value in the current context requires a variable capture.
+///
+/// This also keeps the captures set in the BlockScopeInfo records
+/// up-to-date.
+static CaptureResult ShouldCaptureValueReference(Sema &S, SourceLocation loc,
+ ValueDecl *value) {
+ // Only variables ever require capture.
+ VarDecl *var = dyn_cast<VarDecl>(value);
+ if (!var || isa<NonTypeTemplateParmDecl>(var)) return CR_NoCapture;
+
+ // Fast path: variables from the current context never require capture.
+ DeclContext *DC = S.CurContext;
+ if (var->getDeclContext() == DC) return CR_NoCapture;
+
+ // Only variables with local storage require capture.
+ // FIXME: What about 'const' variables in C++?
+ if (!var->hasLocalStorage()) return CR_NoCapture;
+
+ // Otherwise, we need to capture.
+
+ unsigned functionScopesIndex = S.FunctionScopes.size() - 1;
+
+ do {
+ // Only blocks (and eventually C++0x closures) can capture; other
+ // scopes don't work.
+ if (!isa<BlockDecl>(DC))
+ return DiagnoseUncapturableValueReference(S, loc, var, DC);
+
+ BlockScopeInfo *blockScope =
+ cast<BlockScopeInfo>(S.FunctionScopes[functionScopesIndex]);
+ assert(blockScope->TheDecl == static_cast<BlockDecl*>(DC));
+
+ // Try to capture it in this block. If we've already captured at
+ // this level, we're done.
+ if (!blockScope->Captures.insert(var))
+ return CR_Capture;
+
+ functionScopesIndex--;
+ DC = cast<BlockDecl>(DC)->getDeclContext();
+ } while (var->getDeclContext() != DC);
+
+ return CR_Capture;
+}
ExprResult
Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK,
@@ -811,17 +859,6 @@
// Non-type template parameters can be referenced anywhere they are
// visible.
Ty = Ty.getNonLValueExprType(Context);
- } else if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(CurContext)) {
- if (const FunctionDecl *FD = MD->getParent()->isLocalClass()) {
- if (VD->hasLocalStorage() && VD->getDeclContext() != CurContext) {
- Diag(NameInfo.getLoc(),
- diag::err_reference_to_local_var_in_enclosing_function)
- << D->getIdentifier() << FD->getDeclName();
- Diag(D->getLocation(), diag::note_local_variable_declared_here)
- << D->getIdentifier();
- return ExprError();
- }
- }
// This ridiculousness brought to you by 'extern void x;' and the
// GNU compiler collection.
@@ -2250,66 +2287,74 @@
// We do not do this for things like enum constants, global variables, etc,
// as they do not get snapshotted.
//
- if (getCurBlock() &&
- ShouldSnapshotBlockValueReference(*this, getCurBlock(), VD)) {
- if (VD->getType().getTypePtr()->isVariablyModifiedType()) {
- Diag(Loc, diag::err_ref_vm_type);
- Diag(D->getLocation(), diag::note_declared_at);
- return ExprError();
- }
+ switch (ShouldCaptureValueReference(*this, NameInfo.getLoc(), VD)) {
+ case CR_Error:
+ return ExprError();
- if (VD->getType()->isArrayType()) {
- Diag(Loc, diag::err_ref_array_type);
- Diag(D->getLocation(), diag::note_declared_at);
- return ExprError();
- }
+ case CR_NoCapture:
+ // If this reference is not in a block or if the referenced
+ // variable is within the block, create a normal DeclRefExpr.
+ return BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(), VK,
+ NameInfo, &SS);
- MarkDeclarationReferenced(Loc, VD);
- QualType ExprTy = VD->getType().getNonReferenceType();
+ case CR_Capture:
+ break;
+ }
- // The BlocksAttr indicates the variable is bound by-reference.
- bool byrefVar = (VD->getAttr<BlocksAttr>() != 0);
- QualType T = VD->getType();
- BlockDeclRefExpr *BDRE;
+ // If we got here, we need to capture.
+
+ if (VD->getType().getTypePtr()->isVariablyModifiedType()) {
+ Diag(Loc, diag::err_ref_vm_type);
+ Diag(D->getLocation(), diag::note_declared_at);
+ return ExprError();
+ }
+
+ if (VD->getType()->isArrayType()) {
+ Diag(Loc, diag::err_ref_array_type);
+ Diag(D->getLocation(), diag::note_declared_at);
+ return ExprError();
+ }
+
+ MarkDeclarationReferenced(Loc, VD);
+ QualType ExprTy = VD->getType().getNonReferenceType();
+
+ // The BlocksAttr indicates the variable is bound by-reference.
+ bool byrefVar = (VD->getAttr<BlocksAttr>() != 0);
+ QualType T = VD->getType();
+ BlockDeclRefExpr *BDRE;
- if (!byrefVar) {
- // This is to record that a 'const' was actually synthesize and added.
- bool constAdded = !ExprTy.isConstQualified();
- // Variable will be bound by-copy, make it const within the closure.
- ExprTy.addConst();
- BDRE = new (Context) BlockDeclRefExpr(VD, ExprTy, VK,
- Loc, false, constAdded);
- }
- else
- BDRE = new (Context) BlockDeclRefExpr(VD, ExprTy, VK, Loc, true);
+ if (!byrefVar) {
+ // This is to record that a 'const' was actually synthesize and added.
+ bool constAdded = !ExprTy.isConstQualified();
+ // Variable will be bound by-copy, make it const within the closure.
+ ExprTy.addConst();
+ BDRE = new (Context) BlockDeclRefExpr(VD, ExprTy, VK,
+ Loc, false, constAdded);
+ }
+ else
+ BDRE = new (Context) BlockDeclRefExpr(VD, ExprTy, VK, Loc, true);
- if (getLangOptions().CPlusPlus) {
- if (!T->isDependentType() && !T->isReferenceType()) {
- Expr *E = new (Context)
- DeclRefExpr(const_cast<ValueDecl*>(BDRE->getDecl()), T,
- VK, SourceLocation());
- if (T->getAs<RecordType>())
- if (!T->isUnionType()) {
- ExprResult Res = PerformCopyInitialization(
- InitializedEntity::InitializeBlock(VD->getLocation(),
+ if (getLangOptions().CPlusPlus) {
+ if (!T->isDependentType() && !T->isReferenceType()) {
+ Expr *E = new (Context)
+ DeclRefExpr(const_cast<ValueDecl*>(BDRE->getDecl()), T,
+ VK, SourceLocation());
+ if (T->isStructureOrClassType()) {
+ ExprResult Res = PerformCopyInitialization(
+ InitializedEntity::InitializeBlock(VD->getLocation(),
T, false),
- SourceLocation(),
- Owned(E));
- if (!Res.isInvalid()) {
- Res = MaybeCreateExprWithCleanups(Res);
- Expr *Init = Res.takeAs<Expr>();
- BDRE->setCopyConstructorExpr(Init);
- }
+ SourceLocation(),
+ Owned(E));
+ if (!Res.isInvalid()) {
+ Res = MaybeCreateExprWithCleanups(Res);
+ Expr *Init = Res.takeAs<Expr>();
+ BDRE->setCopyConstructorExpr(Init);
}
}
}
- return Owned(BDRE);
}
- // If this reference is not in a block or if the referenced variable is
- // within the block, create a normal DeclRefExpr.
- return BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(), VK,
- NameInfo, &SS);
+ return Owned(BDRE);
}
ExprResult Sema::ActOnPredefinedExpr(SourceLocation Loc,
@@ -8486,6 +8531,12 @@
bool NoReturn = BSI->TheDecl->getAttr<NoReturnAttr>();
QualType BlockTy;
+ // Set the captured variables on the block.
+ BSI->TheDecl->setCapturedDecls(Context,
+ BSI->Captures.begin(),
+ BSI->Captures.end(),
+ BSI->CapturesCXXThis);
+
// If the user wrote a function type in some form, try to use that.
if (!BSI->FunctionType.isNull()) {
const FunctionType *FTy = BSI->FunctionType->getAs<FunctionType>();
@@ -8557,8 +8608,7 @@
return ExprError();
}
- BlockExpr *Result = new (Context) BlockExpr(BSI->TheDecl, BlockTy,
- BSI->hasBlockDeclRefExprs);
+ BlockExpr *Result = new (Context) BlockExpr(BSI->TheDecl, BlockTy);
// Issue any analysis-based warnings.
const sema::AnalysisBasedWarnings::Policy &WP =
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 7bc9af1..73e9778 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -16,6 +16,7 @@
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/ParsedTemplate.h"
+#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/TemplateDeduction.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/CXXInheritance.h"
@@ -563,14 +564,23 @@
/// is a non-lvalue expression whose value is the address of the object for
/// which the function is called.
- DeclContext *DC = getFunctionLevelDeclContext();
- if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(DC))
- if (MD->isInstance())
- return Owned(new (Context) CXXThisExpr(ThisLoc,
- MD->getThisType(Context),
- /*isImplicit=*/false));
+ // Ignore block scopes (but nothing else).
+ DeclContext *DC = CurContext;
+ while (isa<BlockDecl>(DC)) DC = cast<BlockDecl>(DC)->getDeclContext();
- return ExprError(Diag(ThisLoc, diag::err_invalid_this_use));
+ // If we're not an instance method, error out.
+ CXXMethodDecl *method = dyn_cast<CXXMethodDecl>(DC);
+ if (!method || !method->isInstance())
+ return ExprError(Diag(ThisLoc, diag::err_invalid_this_use));
+
+ // Mark that we're closing on 'this' in all the block scopes, if applicable.
+ for (unsigned idx = FunctionScopes.size() - 1;
+ isa<BlockScopeInfo>(FunctionScopes[idx]);
+ --idx)
+ cast<BlockScopeInfo>(FunctionScopes[idx])->CapturesCXXThis = true;
+
+ return Owned(new (Context) CXXThisExpr(ThisLoc, method->getThisType(Context),
+ /*isImplicit=*/false));
}
ExprResult
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 26baee4..311ce7b 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -696,6 +696,15 @@
for (unsigned I = 0; I != NumParams; ++I)
Params.push_back(cast<ParmVarDecl>(Reader.GetDecl(Record[Idx++])));
BD->setParams(Params.data(), NumParams);
+
+ bool capturesCXXThis = Record[Idx++];
+ unsigned numCapturedDecls = Record[Idx++];
+ llvm::SmallVector<VarDecl*, 16> capturedDecls;
+ capturedDecls.reserve(numCapturedDecls);
+ for (unsigned i = 0; i != numCapturedDecls; ++i)
+ capturedDecls.push_back(cast<VarDecl>(Reader.GetDecl(Record[Idx++])));
+ BD->setCapturedDecls(*Reader.getContext(), capturedDecls.begin(),
+ capturedDecls.end(), capturesCXXThis);
}
void ASTDeclReader::VisitLinkageSpecDecl(LinkageSpecDecl *D) {
diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 1a2284b..c704eb2 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -791,7 +791,6 @@
void ASTStmtReader::VisitBlockExpr(BlockExpr *E) {
VisitExpr(E);
E->setBlockDecl(cast_or_null<BlockDecl>(Reader.GetDecl(Record[Idx++])));
- E->setHasBlockDeclRefExprs(Record[Idx++]);
}
void ASTStmtReader::VisitBlockDeclRefExpr(BlockDeclRefExpr *E) {
diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index 507c669..1fe6398 100644
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -623,6 +623,12 @@
for (FunctionDecl::param_iterator P = D->param_begin(), PEnd = D->param_end();
P != PEnd; ++P)
Writer.AddDeclRef(*P, Record);
+ Record.push_back(D->capturesCXXThis());
+ Record.push_back(D->getNumCapturedDecls());
+ for (BlockDecl::capture_iterator
+ i = D->capture_begin(), e = D->capture_end(); i != e; ++i)
+ Writer.AddDeclRef(*i, Record);
+
Code = serialization::DECL_BLOCK;
}
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index 601de6d..15a3670 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -761,7 +761,6 @@
void ASTStmtWriter::VisitBlockExpr(BlockExpr *E) {
VisitExpr(E);
Writer.AddDeclRef(E->getBlockDecl(), Record);
- Record.push_back(E->hasBlockDeclRefExprs());
Code = serialization::EXPR_BLOCK;
}
diff --git a/lib/StaticAnalyzer/CFRefCount.cpp b/lib/StaticAnalyzer/CFRefCount.cpp
index 3278c4a..2790d54 100644
--- a/lib/StaticAnalyzer/CFRefCount.cpp
+++ b/lib/StaticAnalyzer/CFRefCount.cpp
@@ -3417,7 +3417,7 @@
// Scan the BlockDecRefExprs for any object the retain/release checker
// may be tracking.
- if (!BE->hasBlockDeclRefExprs())
+ if (!BE->getBlockDecl()->hasCaptures())
return;
const GRState *state = C.getState();
diff --git a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
index 1aaaa68..561777c 100644
--- a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
@@ -56,7 +56,7 @@
void
UndefCapturedBlockVarChecker::PostVisitBlockExpr(CheckerContext &C,
const BlockExpr *BE) {
- if (!BE->hasBlockDeclRefExprs())
+ if (!BE->getBlockDecl()->hasCaptures())
return;
const GRState *state = C.getState();