r362178 - Defer capture initialization for blocks until after we've left the
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu May 30 17:45:09 PDT 2019
Author: rsmith
Date: Thu May 30 17:45:09 2019
New Revision: 362178
URL: http://llvm.org/viewvc/llvm-project?rev=362178&view=rev
Log:
Defer capture initialization for blocks until after we've left the
function scope.
This removes one of the last few cases where we build expressions in the
wrong function scope context. No functionality change intended.
Modified:
cfe/trunk/include/clang/Sema/AnalysisBasedWarnings.h
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Analysis/blocks.mm
Modified: cfe/trunk/include/clang/Sema/AnalysisBasedWarnings.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AnalysisBasedWarnings.h?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/AnalysisBasedWarnings.h (original)
+++ cfe/trunk/include/clang/Sema/AnalysisBasedWarnings.h Thu May 30 17:45:09 2019
@@ -90,7 +90,7 @@ public:
AnalysisBasedWarnings(Sema &s);
void IssueWarnings(Policy P, FunctionScopeInfo *fscope,
- const Decl *D, const BlockExpr *blkExpr);
+ const Decl *D, QualType BlockType);
Policy getDefaultPolicy() { return DefaultPolicy; }
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Thu May 30 17:45:09 2019
@@ -487,6 +487,8 @@ public:
/// Clear out the information in this function scope, making it
/// suitable for reuse.
void Clear();
+
+ bool isPlainFunction() const { return Kind == SK_Function; }
};
class Capture {
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu May 30 17:45:09 2019
@@ -595,7 +595,7 @@ public:
using MaybeODRUseExprSet = llvm::SmallPtrSet<Expr *, 2>;
MaybeODRUseExprSet MaybeODRUseExprs;
- std::unique_ptr<sema::FunctionScopeInfo> PreallocatedFunctionScope;
+ std::unique_ptr<sema::FunctionScopeInfo> CachedFunctionScope;
/// Stack containing information about each of the nested
/// function, block, and method scopes that are currently active.
@@ -1408,10 +1408,24 @@ public:
void PushCapturedRegionScope(Scope *RegionScope, CapturedDecl *CD,
RecordDecl *RD,
CapturedRegionKind K);
- void
+
+ /// Custom deleter to allow FunctionScopeInfos to be kept alive for a short
+ /// time after they've been popped.
+ class PoppedFunctionScopeDeleter {
+ Sema *Self;
+
+ public:
+ explicit PoppedFunctionScopeDeleter(Sema *Self) : Self(Self) {}
+ void operator()(sema::FunctionScopeInfo *Scope) const;
+ };
+
+ using PoppedFunctionScopePtr =
+ std::unique_ptr<sema::FunctionScopeInfo, PoppedFunctionScopeDeleter>;
+
+ PoppedFunctionScopePtr
PopFunctionScopeInfo(const sema::AnalysisBasedWarnings::Policy *WP = nullptr,
const Decl *D = nullptr,
- const BlockExpr *blkExpr = nullptr);
+ QualType BlockType = QualType());
sema::FunctionScopeInfo *getCurFunction() const {
return FunctionScopes.empty() ? nullptr : FunctionScopes.back();
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu May 30 17:45:09 2019
@@ -620,7 +620,7 @@ struct CheckFallThroughDiagnostics {
/// of a noreturn function. We assume that functions and blocks not marked
/// noreturn will return.
static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
- const BlockExpr *blkExpr,
+ QualType BlockType,
const CheckFallThroughDiagnostics &CD,
AnalysisDeclContext &AC,
sema::FunctionScopeInfo *FSI) {
@@ -641,9 +641,8 @@ static void CheckFallThroughForBody(Sema
HasNoReturn = MD->hasAttr<NoReturnAttr>();
}
else if (isa<BlockDecl>(D)) {
- QualType BlockTy = blkExpr->getType();
if (const FunctionType *FT =
- BlockTy->getPointeeType()->getAs<FunctionType>()) {
+ BlockType->getPointeeType()->getAs<FunctionType>()) {
if (FT->getReturnType()->isVoidType())
ReturnsVoid = true;
if (FT->getNoReturnAttr())
@@ -2012,7 +2011,7 @@ static void flushDiagnostics(Sema &S, co
void clang::sema::
AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
sema::FunctionScopeInfo *fscope,
- const Decl *D, const BlockExpr *blkExpr) {
+ const Decl *D, QualType BlockType) {
// We avoid doing analysis-based warnings when there are errors for
// two reasons:
@@ -2138,7 +2137,7 @@ AnalysisBasedWarnings::IssueWarnings(sem
: (fscope->isCoroutine()
? CheckFallThroughDiagnostics::MakeForCoroutine(D)
: CheckFallThroughDiagnostics::MakeForFunction(D)));
- CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC, fscope);
+ CheckFallThroughForBody(S, D, Body, BlockType, CD, AC, fscope);
}
// Warning: check for unreachable code
Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Thu May 30 17:45:09 2019
@@ -176,8 +176,6 @@ Sema::Sema(Preprocessor &pp, ASTContext
ExpressionEvaluationContext::PotentiallyEvaluated, 0, CleanupInfo{},
nullptr, ExpressionEvaluationContextRecord::EK_Other);
- PreallocatedFunctionScope.reset(new FunctionScopeInfo(Diags));
-
// Initialization of data sharing attributes stack for OpenMP
InitDataSharingAttributesStack();
@@ -354,8 +352,7 @@ Sema::~Sema() {
// Kill all the active scopes.
for (sema::FunctionScopeInfo *FSI : FunctionScopes)
- if (FSI != PreallocatedFunctionScope.get())
- delete FSI;
+ delete FSI;
// Tell the SemaConsumer to forget about us; we're going out of scope.
if (SemaConsumer *SC = dyn_cast<SemaConsumer>(&Consumer))
@@ -1596,10 +1593,10 @@ Scope *Sema::getScopeForContext(DeclCont
/// Enter a new function scope
void Sema::PushFunctionScope() {
- if (FunctionScopes.empty()) {
- // Use PreallocatedFunctionScope to avoid allocating memory when possible.
- PreallocatedFunctionScope->Clear();
- FunctionScopes.push_back(PreallocatedFunctionScope.get());
+ if (FunctionScopes.empty() && CachedFunctionScope) {
+ // Use CachedFunctionScope to avoid allocating memory when possible.
+ CachedFunctionScope->Clear();
+ FunctionScopes.push_back(CachedFunctionScope.release());
} else {
FunctionScopes.push_back(new FunctionScopeInfo(getDiagnostics()));
}
@@ -1680,30 +1677,42 @@ static void markEscapingByrefs(const Fun
}
}
-void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
- const Decl *D, const BlockExpr *blkExpr) {
+/// Pop a function (or block or lambda or captured region) scope from the stack.
+///
+/// \param WP The warning policy to use for CFG-based warnings, or null if such
+/// warnings should not be produced.
+/// \param D The declaration corresponding to this function scope, if producing
+/// CFG-based warnings.
+/// \param BlockType The type of the block expression, if D is a BlockDecl.
+Sema::PoppedFunctionScopePtr
+Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
+ const Decl *D, QualType BlockType) {
assert(!FunctionScopes.empty() && "mismatched push/pop!");
- // This function shouldn't be called after popping the current function scope.
- // markEscapingByrefs calls PerformMoveOrCopyInitialization, which can call
- // PushFunctionScope, which can cause clearing out PreallocatedFunctionScope
- // when FunctionScopes is empty.
markEscapingByrefs(*FunctionScopes.back(), *this);
- FunctionScopeInfo *Scope = FunctionScopes.pop_back_val();
+ PoppedFunctionScopePtr Scope(FunctionScopes.pop_back_val(),
+ PoppedFunctionScopeDeleter(this));
if (LangOpts.OpenMP)
- popOpenMPFunctionRegion(Scope);
+ popOpenMPFunctionRegion(Scope.get());
// Issue any analysis-based warnings.
if (WP && D)
- AnalysisWarnings.IssueWarnings(*WP, Scope, D, blkExpr);
+ AnalysisWarnings.IssueWarnings(*WP, Scope.get(), D, BlockType);
else
for (const auto &PUD : Scope->PossiblyUnreachableDiags)
Diag(PUD.Loc, PUD.PD);
- // Delete the scope unless its our preallocated scope.
- if (Scope != PreallocatedFunctionScope.get())
+ return Scope;
+}
+
+void Sema::PoppedFunctionScopeDeleter::
+operator()(sema::FunctionScopeInfo *Scope) const {
+ // Stash the function scope for later reuse if it's for a normal function.
+ if (Scope->isPlainFunction() && !Self->CachedFunctionScope)
+ Self->CachedFunctionScope.reset(Scope);
+ else
delete Scope;
}
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu May 30 17:45:09 2019
@@ -13823,8 +13823,6 @@ ExprResult Sema::ActOnBlockStmtExpr(Sour
if (BSI->HasImplicitReturnType)
deduceClosureReturnType(*BSI);
- PopDeclContext();
-
QualType RetTy = Context.VoidTy;
if (!BSI->ReturnType.isNull())
RetTy = BSI->ReturnType;
@@ -13832,17 +13830,6 @@ ExprResult Sema::ActOnBlockStmtExpr(Sour
bool NoReturn = BD->hasAttr<NoReturnAttr>();
QualType BlockTy;
- // Set the captured variables on the block.
- SmallVector<BlockDecl::Capture, 4> Captures;
- for (Capture &Cap : BSI->Captures) {
- if (Cap.isInvalid() || Cap.isThisCapture())
- continue;
- BlockDecl::Capture NewCap(Cap.getVariable(), Cap.isBlockCapture(),
- Cap.isNested(), Cap.getInitExpr());
- Captures.push_back(NewCap);
- }
- BD->setCaptures(Context, Captures, BSI->CXXThisCaptureIndex != 0);
-
// 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>();
@@ -13898,9 +13885,80 @@ ExprResult Sema::ActOnBlockStmtExpr(Sour
!BD->isDependentContext())
computeNRVO(Body, BSI);
- BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
+ PopDeclContext();
+
+ // Pop the block scope now but keep it alive to the end of this function.
AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
- PopFunctionScopeInfo(&WP, Result->getBlockDecl(), Result);
+ PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);
+
+ // Set the captured variables on the block.
+ SmallVector<BlockDecl::Capture, 4> Captures;
+ for (Capture &Cap : BSI->Captures) {
+ if (Cap.isInvalid() || Cap.isThisCapture())
+ continue;
+
+ VarDecl *Var = Cap.getVariable();
+ Expr *CopyExpr = nullptr;
+ if (getLangOpts().CPlusPlus && Cap.isCopyCapture()) {
+ if (const RecordType *Record =
+ Cap.getCaptureType()->getAs<RecordType>()) {
+ // The capture logic needs the destructor, so make sure we mark it.
+ // Usually this is unnecessary because most local variables have
+ // their destructors marked at declaration time, but parameters are
+ // an exception because it's technically only the call site that
+ // actually requires the destructor.
+ if (isa<ParmVarDecl>(Var))
+ FinalizeVarWithDestructor(Var, Record);
+
+ // Enter a separate potentially-evaluated context while building block
+ // initializers to isolate their cleanups from those of the block
+ // itself.
+ // FIXME: Is this appropriate even when the block itself occurs in an
+ // unevaluated operand?
+ EnterExpressionEvaluationContext EvalContext(
+ *this, ExpressionEvaluationContext::PotentiallyEvaluated);
+
+ SourceLocation Loc = Cap.getLocation();
+
+ ExprResult Result = BuildDeclarationNameExpr(
+ CXXScopeSpec(), DeclarationNameInfo(Var->getDeclName(), Loc), Var);
+
+ // According to the blocks spec, the capture of a variable from
+ // the stack requires a const copy constructor. This is not true
+ // of the copy/move done to move a __block variable to the heap.
+ if (!Result.isInvalid() &&
+ !Result.get()->getType().isConstQualified()) {
+ Result = ImpCastExprToType(Result.get(),
+ Result.get()->getType().withConst(),
+ CK_NoOp, VK_LValue);
+ }
+
+ if (!Result.isInvalid()) {
+ Result = PerformCopyInitialization(
+ InitializedEntity::InitializeBlock(Var->getLocation(),
+ Cap.getCaptureType(), false),
+ Loc, Result.get());
+ }
+
+ // Build a full-expression copy expression if initialization
+ // succeeded and used a non-trivial constructor. Recover from
+ // errors by pretending that the copy isn't necessary.
+ if (!Result.isInvalid() &&
+ !cast<CXXConstructExpr>(Result.get())->getConstructor()
+ ->isTrivial()) {
+ Result = MaybeCreateExprWithCleanups(Result);
+ CopyExpr = Result.get();
+ }
+ }
+ }
+
+ BlockDecl::Capture NewCap(Var, Cap.isBlockCapture(), Cap.isNested(),
+ CopyExpr);
+ Captures.push_back(NewCap);
+ }
+ BD->setCaptures(Context, Captures, BSI->CXXThisCaptureIndex != 0);
+
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
// If the block isn't obviously global, i.e. it captures anything at
// all, then we need to do a few things in the surrounding context:
@@ -15192,7 +15250,6 @@ static bool captureInBlock(BlockScopeInf
QualType &DeclRefType,
const bool Nested,
Sema &S, bool Invalid) {
- Expr *CopyExpr = nullptr;
bool ByRef = false;
// Blocks are not allowed to capture arrays, excepting OpenCL.
@@ -15264,51 +15321,12 @@ static bool captureInBlock(BlockScopeInf
// Block capture by copy introduces 'const'.
CaptureType = CaptureType.getNonReferenceType().withConst();
DeclRefType = CaptureType;
-
- if (S.getLangOpts().CPlusPlus && BuildAndDiagnose) {
- if (const RecordType *Record = DeclRefType->getAs<RecordType>()) {
- // The capture logic needs the destructor, so make sure we mark it.
- // Usually this is unnecessary because most local variables have
- // their destructors marked at declaration time, but parameters are
- // an exception because it's technically only the call site that
- // actually requires the destructor.
- if (isa<ParmVarDecl>(Var))
- S.FinalizeVarWithDestructor(Var, Record);
-
- // Enter a new evaluation context to insulate the copy
- // full-expression.
- EnterExpressionEvaluationContext scope(
- S, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
-
- // According to the blocks spec, the capture of a variable from
- // the stack requires a const copy constructor. This is not true
- // of the copy/move done to move a __block variable to the heap.
- Expr *DeclRef = new (S.Context) DeclRefExpr(
- S.Context, Var, Nested, DeclRefType.withConst(), VK_LValue, Loc);
-
- ExprResult Result
- = S.PerformCopyInitialization(
- InitializedEntity::InitializeBlock(Var->getLocation(),
- CaptureType, false),
- Loc, DeclRef);
-
- // Build a full-expression copy expression if initialization
- // succeeded and used a non-trivial constructor. Recover from
- // errors by pretending that the copy isn't necessary.
- if (!Result.isInvalid() &&
- !cast<CXXConstructExpr>(Result.get())->getConstructor()
- ->isTrivial()) {
- Result = S.MaybeCreateExprWithCleanups(Result);
- CopyExpr = Result.get();
- }
- }
- }
}
// Actually capture the variable.
if (BuildAndDiagnose)
BSI->addCapture(Var, HasBlocksAttr, ByRef, Nested, Loc, SourceLocation(),
- CaptureType, CopyExpr, Invalid);
+ CaptureType, nullptr, Invalid);
return !Invalid;
}
Modified: cfe/trunk/test/Analysis/blocks.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/blocks.mm?rev=362178&r1=362177&r2=362178&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/blocks.mm (original)
+++ cfe/trunk/test/Analysis/blocks.mm Thu May 30 17:45:09 2019
@@ -52,9 +52,10 @@ void testBlockWithCopyExpression(StructW
// CHECK: [B1]
// CHECK-NEXT: 1: s
-// CHECK-NEXT: 2: [B1.1] (CXXConstructExpr, const struct StructWithCopyConstructor)
-// CHECK-NEXT: 3: ^{ }
-// CHECK-NEXT: 4: (void)([B1.3]) (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const struct StructWithCopyConstructor)
+// CHECK-NEXT: 3: [B1.2] (CXXConstructExpr, const struct StructWithCopyConstructor)
+// CHECK-NEXT: 4: ^{ }
+// CHECK-NEXT: 5: (void)([B1.4]) (CStyleCastExpr, ToVoid, void)
// CHECK-NEXT: Preds (1): B2
// CHECK-NEXT: Succs (1): B0
More information about the cfe-commits
mailing list