squiid/llvm
1
0
Fork 0
Code Issues Pull requests Activity

Defer capture initialization for blocks until after we've left the

Browse source 
function scope.

This removes one of the last few cases where we build expressions in the
wrong function scope context. No functionality change intended.

llvm-svn: 362178
This commit is contained in:
Richard Smith 2019-05-31 00:45:09 +00:00
parent 4a585a3edd
commit 2fdd95c1c8
7 changed files with 130 additions and 87 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
clang/include/clang/Sema/AnalysisBasedWarnings.h
View file

@ -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; }

2
clang/include/clang/Sema/ScopeInfo.h
View file

@ -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 {

20
clang/include/clang/Sema/Sema.h
View file

@ -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();

9
clang/lib/Sema/AnalysisBasedWarnings.cpp
View file

@ -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 &S, const Decl *D, const Stmt *Body,
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, const sema::FunctionScopeInfo *fscope) {
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(sema::AnalysisBasedWarnings::Policy P,
: (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

47
clang/lib/Sema/Sema.cpp
View file

@ -176,8 +176,6 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
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(DeclContext *Ctx) {
/// 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 FunctionScopeInfo &FSI, Sema &S) {
}
}
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;
}

130
clang/lib/Sema/SemaExpr.cpp
View file

@ -13823,8 +13823,6 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
if (BSI->HasImplicitReturnType)
deduceClosureReturnType(*BSI);
PopDeclContext();
QualType RetTy = Context.VoidTy;
if (!BSI->ReturnType.isNull())
RetTy = BSI->ReturnType;
@ -13832,17 +13830,6 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
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(SourceLocation CaretLoc,
!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(BlockScopeInfo *BSI, VarDecl *Var,
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(BlockScopeInfo *BSI, VarDecl *Var,
// 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;
}

7
clang/test/Analysis/blocks.mm
View file

@ -52,9 +52,10 @@ void testBlockWithCopyExpression(StructWithCopyConstructor s) {
// 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