Skip to content

Commit 801c0d7

Browse files
committed
Rebase SILScope generation on top of ASTScope.
This patch replaces the stateful generation of SILScope information in SILGenFunction with data derived from the ASTScope hierarchy, which should be 100% in sync with the scopes needed for local variables. The goal is to eliminate the surprising effects that the stack of cleanup operations can have on the current state of SILBuilder leading to a fully deterministic (in the sense of: predictible by a human) association of SILDebugScopes with SILInstructions. The patch also eliminates the need to many workarounds. There are still some accomodations for several Sema transformation passes such as ResultBuilders, which don't correctly update the source locations when moving around nodes. If these were implemented as macros, this problem would disappear. This necessary rewrite of the macro scope handling included in this patch also adds proper support nested macro expansions. This fixes rdar://88274783 and either fixes or at least partially addresses the following: rdar://89252827 rdar://105186946 rdar://105757810 rdar://105997826 rdar://105102288
1 parent 2da0976 commit 801c0d7

File tree

65 files changed

+832
-388
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+832
-388
lines changed

include/swift/AST/ASTScope.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ class GenericContext;
7878
class DeclName;
7979
class StmtConditionElement;
8080

81+
namespace Lowering {
82+
class SILGenFunction;
83+
}
84+
8185
namespace ast_scope {
8286
class ASTScopeImpl;
8387
class GenericTypeOrExtensionScope;
@@ -129,6 +133,7 @@ class ASTScopeImpl : public ASTAllocated<ASTScopeImpl> {
129133
friend class IterableTypeBodyPortion;
130134
friend class ScopeCreator;
131135
friend class ASTSourceFileScope;
136+
friend class Lowering::SILGenFunction;
132137

133138
#pragma mark - tree state
134139
protected:
@@ -273,6 +278,10 @@ class ASTScopeImpl : public ASTAllocated<ASTScopeImpl> {
273278
static std::pair<CaseStmt *, CaseStmt *>
274279
lookupFallthroughSourceAndDest(SourceFile *sourceFile, SourceLoc loc);
275280

281+
/// Scopes that cannot bind variables may set this to true to create more
282+
/// compact scope tree in the debug info.
283+
virtual bool ignoreInDebugInfo() const { return false; }
284+
276285
#pragma mark - - lookup- starting point
277286
private:
278287
static const ASTScopeImpl *findStartingScopeForLookup(SourceFile *,
@@ -777,6 +786,7 @@ class ParameterListScope final : public ASTScopeImpl {
777786
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
778787

779788
NullablePtr<const void> addressForPrinting() const override { return params; }
789+
bool ignoreInDebugInfo() const override { return true; }
780790
};
781791

782792
/// Body of functions, methods, constructors, destructors and accessors.
@@ -799,6 +809,7 @@ class FunctionBodyScope : public ASTScopeImpl {
799809
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
800810
virtual NullablePtr<Decl> getDeclIfAny() const override { return decl; }
801811
Decl *getDecl() const { return decl; }
812+
bool ignoreInDebugInfo() const override { return true; }
802813

803814
protected:
804815
bool lookupLocalsOrMembers(DeclConsumer) const override;
@@ -825,6 +836,7 @@ class DefaultArgumentInitializerScope final : public ASTScopeImpl {
825836
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
826837
virtual NullablePtr<Decl> getDeclIfAny() const override { return decl; }
827838
Decl *getDecl() const { return decl; }
839+
bool ignoreInDebugInfo() const override { return true; }
828840
};
829841

830842
/// Consider:
@@ -858,7 +870,7 @@ class AttachedPropertyWrapperScope final : public ASTScopeImpl {
858870
NullablePtr<DeclAttribute> getDeclAttributeIfAny() const override {
859871
return attr;
860872
}
861-
873+
bool ignoreInDebugInfo() const override { return true; }
862874
private:
863875
void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
864876
};
@@ -1049,6 +1061,7 @@ class ClosureParametersScope final : public ASTScopeImpl {
10491061
}
10501062
NullablePtr<Expr> getExprIfAny() const override { return closureExpr; }
10511063
Expr *getExpr() const { return closureExpr; }
1064+
bool ignoreInDebugInfo() const override { return true; }
10521065

10531066
protected:
10541067
ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;

include/swift/SIL/SILBuilder.h

Lines changed: 96 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#ifndef SWIFT_SIL_SILBUILDER_H
1414
#define SWIFT_SIL_SILBUILDER_H
1515

16+
#include "SILDebugVariable.h"
17+
#include "SILInstruction.h"
1618
#include "swift/Basic/ProfileCounter.h"
1719
#include "swift/SIL/SILArgument.h"
1820
#include "swift/SIL/SILDebugScope.h"
@@ -22,6 +24,7 @@
2224
#include "swift/SIL/SILUndef.h"
2325
#include "llvm/ADT/PointerUnion.h"
2426
#include "llvm/ADT/StringExtras.h"
27+
#include <type_traits>
2528

2629
namespace swift {
2730

@@ -179,27 +182,33 @@ class SILBuilder {
179182
/// location.
180183
///
181184
/// SILBuilderContext must outlive this SILBuilder instance.
182-
SILBuilder(SILInstruction *I, const SILDebugScope *DS, SILBuilderContext &C)
185+
SILBuilder(SILInstruction *I, SILBuilderContext &C,
186+
const SILDebugScope *DS = nullptr)
183187
: TempContext(C.getModule()), C(C), F(I->getFunction()) {
184-
assert(DS && "instruction has no debug scope");
185-
setCurrentDebugScope(DS);
186188
setInsertionPoint(I);
189+
if (DS)
190+
setCurrentDebugScope(DS);
187191
}
188192

189-
SILBuilder(SILBasicBlock *BB, const SILDebugScope *DS, SILBuilder &B)
190-
: SILBuilder(BB, DS, B.getBuilderContext()) {}
193+
SILBuilder(SILBasicBlock *BB, SILBuilder &B,
194+
const SILDebugScope *DS = nullptr)
195+
: SILBuilder(BB, B.getBuilderContext(), DS) {}
191196

192197
/// Build instructions before the given insertion point, inheriting the debug
193198
/// location.
194199
///
195200
/// SILBuilderContext must outlive this SILBuilder instance.
196-
SILBuilder(SILBasicBlock *BB, const SILDebugScope *DS, SILBuilderContext &C)
201+
SILBuilder(SILBasicBlock *BB, SILBuilderContext &C,
202+
const SILDebugScope *DS = nullptr)
197203
: TempContext(C.getModule()), C(C), F(BB->getParent()) {
198204
assert(DS && "block has no debug scope");
199-
setCurrentDebugScope(DS);
200205
setInsertionPoint(BB);
206+
if (DS)
207+
setCurrentDebugScope(DS);
201208
}
202209

210+
virtual ~SILBuilder() {}
211+
203212
// Allow a pass to override the current SIL module conventions. This should
204213
// only be done by a pass responsible for lowering SIL to a new stage
205214
// (e.g. AddressLowering).
@@ -249,7 +258,8 @@ class SILBuilder {
249258
}
250259

251260
/// Convenience function for building a SILDebugLocation.
252-
SILDebugLocation getSILDebugLocation(SILLocation Loc) {
261+
virtual SILDebugLocation
262+
getSILDebugLocation(SILLocation Loc, bool ForMetaInstruction = false) {
253263
// FIXME: Audit all uses and enable this assertion.
254264
// assert(getCurrentDebugScope() && "no debug scope");
255265
auto Scope = getCurrentDebugScope();
@@ -259,6 +269,17 @@ class SILBuilder {
259269
return SILDebugLocation(overriddenLoc, Scope);
260270
}
261271

272+
/// When the frontend generates synthesized conformances it generates a
273+
/// fully-typechecked AST without source locations. This means that the
274+
/// ASTScope-based mechanism to generate SILDebugScopes doesn't work, which
275+
/// means we can't disambiguate local variables in different lexical
276+
/// scopes. To avoid a verification error later in the pipeline, drop all
277+
/// variables without a proper source location.
278+
bool shouldDropVariable(SILDebugVariable Var, SILLocation Loc) {
279+
return !Var.ArgNo && Loc.isSynthesizedAST();
280+
}
281+
282+
262283
/// If we have a SILFunction, return SILFunction::hasOwnership(). If we have a
263284
/// SILGlobalVariable, just return false.
264285
bool hasOwnership() const {
@@ -310,26 +331,38 @@ class SILBuilder {
310331
/// setInsertionPoint - Set the insertion point to insert before the specified
311332
/// instruction.
312333
void setInsertionPoint(SILInstruction *I) {
334+
#ifndef NDEBUG
335+
PrevDebugScope = nullptr;
336+
#endif
313337
assert(I && "can't set insertion point to a null instruction");
314338
setInsertionPoint(I->getParent(), I->getIterator());
315339
}
316340

317341
/// setInsertionPoint - Set the insertion point to insert before the specified
318342
/// instruction.
319343
void setInsertionPoint(SILBasicBlock::iterator IIIter) {
344+
#ifndef NDEBUG
345+
PrevDebugScope = nullptr;
346+
#endif
320347
setInsertionPoint(IIIter->getParent(), IIIter);
321348
}
322349

323350
/// setInsertionPoint - Set the insertion point to insert at the end of the
324351
/// specified block.
325352
void setInsertionPoint(SILBasicBlock *BB) {
353+
#ifndef NDEBUG
354+
PrevDebugScope = nullptr;
355+
#endif
326356
assert(BB && "can't set insertion point to a null basic block");
327357
setInsertionPoint(BB, BB->end());
328358
}
329359

330360
/// setInsertionPoint - Set the insertion point to insert at the end of the
331361
/// specified block.
332362
void setInsertionPoint(SILFunction::iterator BBIter) {
363+
#ifndef NDEBUG
364+
PrevDebugScope = nullptr;
365+
#endif
333366
setInsertionPoint(&*BBIter);
334367
}
335368

@@ -388,7 +421,7 @@ class SILBuilder {
388421
SILBasicBlock *createFallthroughBlock(SILLocation loc,
389422
SILBasicBlock *targetBB) {
390423
auto *newBB = F->createBasicBlock();
391-
SILBuilder(newBB, this->getCurrentDebugScope(), this->getBuilderContext())
424+
SILBuilder(newBB, this->getBuilderContext(), this->getCurrentDebugScope())
392425
.createBranch(loc, targetBB);
393426
return newBB;
394427
}
@@ -401,6 +434,8 @@ class SILBuilder {
401434
Optional<SILDebugVariable>
402435
substituteAnonymousArgs(llvm::SmallString<4> Name,
403436
Optional<SILDebugVariable> Var, SILLocation Loc) {
437+
if (Var && shouldDropVariable(*Var, Loc))
438+
return {};
404439
if (!Var || !Var->ArgNo || !Var->Name.empty())
405440
return Var;
406441

@@ -416,14 +451,21 @@ class SILBuilder {
416451
AllocStackInst *createAllocStack(SILLocation Loc, SILType elementType,
417452
Optional<SILDebugVariable> Var = None,
418453
bool hasDynamicLifetime = false,
419-
bool isLexical = false,
420-
bool wasMoved = false) {
454+
bool isLexical = false, bool wasMoved = false
455+
#ifndef NDEBUG
456+
,
457+
bool skipVarDeclAssert = false
458+
#endif
459+
) {
421460
llvm::SmallString<4> Name;
422461
Loc.markAsPrologue();
423-
assert((!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
424-
"location is a VarDecl, but SILDebugVariable is empty");
462+
#ifndef NDEBUG
463+
if (dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()))
464+
assert((skipVarDeclAssert || Loc.isSynthesizedAST() || Var) &&
465+
"location is a VarDecl, but SILDebugVariable is empty");
466+
#endif
425467
return insert(AllocStackInst::create(
426-
getSILDebugLocation(Loc), elementType, getFunction(),
468+
getSILDebugLocation(Loc, true), elementType, getFunction(),
427469
substituteAnonymousArgs(Name, Var, Loc), hasDynamicLifetime, isLexical,
428470
wasMoved));
429471
}
@@ -474,15 +516,20 @@ class SILBuilder {
474516
Optional<SILDebugVariable> Var = None,
475517
bool hasDynamicLifetime = false,
476518
bool reflection = false,
477-
bool usesMoveableValueDebugInfo = false) {
519+
bool usesMoveableValueDebugInfo = false
520+
#ifndef NDEBUG
521+
, bool skipVarDeclAssert = false
522+
#endif
523+
) {
478524
llvm::SmallString<4> Name;
479525
Loc.markAsPrologue();
480-
assert((!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
526+
assert((skipVarDeclAssert ||
527+
!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
481528
"location is a VarDecl, but SILDebugVariable is empty");
482-
return insert(AllocBoxInst::create(getSILDebugLocation(Loc), BoxType, *F,
483-
substituteAnonymousArgs(Name, Var, Loc),
484-
hasDynamicLifetime, reflection,
485-
usesMoveableValueDebugInfo));
529+
return insert(AllocBoxInst::create(
530+
getSILDebugLocation(Loc, true), BoxType, *F,
531+
substituteAnonymousArgs(Name, Var, Loc), hasDynamicLifetime, reflection,
532+
usesMoveableValueDebugInfo));
486533
}
487534

488535
AllocExistentialBoxInst *
@@ -2937,7 +2984,21 @@ class SILBuilder {
29372984
class SILBuilderWithScope : public SILBuilder {
29382985
void inheritScopeFrom(SILInstruction *I) {
29392986
assert(I->getDebugScope() && "instruction has no debug scope");
2940-
setCurrentDebugScope(I->getDebugScope());
2987+
SILBasicBlock::iterator II(*I);
2988+
auto End = I->getParent()->end();
2989+
const SILDebugScope *DS = II->getDebugScope();
2990+
assert(DS);
2991+
// Skip over meta instructions, since debug_values may originate from outer
2992+
// scopes. Don't do any of this after inlining.
2993+
while (!DS->InlinedCallSite && II != End && II->isMetaInstruction())
2994+
++II;
2995+
if (II != End) {
2996+
auto nextScope = II->getDebugScope();
2997+
if (!nextScope->InlinedCallSite)
2998+
DS = nextScope;
2999+
}
3000+
assert(DS);
3001+
setCurrentDebugScope(DS);
29413002
}
29423003

29433004
public:
@@ -2946,29 +3007,33 @@ class SILBuilderWithScope : public SILBuilder {
29463007
///
29473008
/// Clients should prefer this constructor.
29483009
SILBuilderWithScope(SILInstruction *I, SILBuilderContext &C)
2949-
: SILBuilder(I, I->getDebugScope(), C)
2950-
{}
3010+
: SILBuilder(I, C) {
3011+
inheritScopeFrom(I);
3012+
}
29513013

29523014
/// Build instructions before the given insertion point, inheriting the debug
29533015
/// location and using the context from the passed in builder.
29543016
///
29553017
/// Clients should prefer this constructor.
29563018
SILBuilderWithScope(SILInstruction *I, SILBuilder &B)
2957-
: SILBuilder(I, I->getDebugScope(), B.getBuilderContext()) {}
3019+
: SILBuilder(I, B.getBuilderContext()) {
3020+
inheritScopeFrom(I);
3021+
}
29583022

29593023
explicit SILBuilderWithScope(
29603024
SILInstruction *I,
29613025
SmallVectorImpl<SILInstruction *> *InsertedInstrs = nullptr)
29623026
: SILBuilder(I, InsertedInstrs) {
2963-
assert(I->getDebugScope() && "instruction has no debug scope");
2964-
setCurrentDebugScope(I->getDebugScope());
3027+
inheritScopeFrom(I);
29653028
}
29663029

29673030
explicit SILBuilderWithScope(SILBasicBlock::iterator I)
29683031
: SILBuilderWithScope(&*I) {}
29693032

29703033
explicit SILBuilderWithScope(SILBasicBlock::iterator I, SILBuilder &B)
2971-
: SILBuilder(&*I, &*I->getDebugScope(), B.getBuilderContext()) {}
3034+
: SILBuilder(&*I, B.getBuilderContext()) {
3035+
inheritScopeFrom(&*I);
3036+
}
29723037

29733038
explicit SILBuilderWithScope(SILInstruction *I,
29743039
SILInstruction *InheritScopeFrom)
@@ -2990,12 +3055,13 @@ class SILBuilderWithScope : public SILBuilder {
29903055

29913056
explicit SILBuilderWithScope(SILBasicBlock *BB, SILBuilder &B,
29923057
SILInstruction *InheritScopeFrom)
2993-
: SILBuilder(BB, InheritScopeFrom->getDebugScope(),
2994-
B.getBuilderContext()) {}
3058+
: SILBuilder(BB, B.getBuilderContext()) {
3059+
inheritScopeFrom(InheritScopeFrom);
3060+
}
29953061

29963062
explicit SILBuilderWithScope(SILBasicBlock *BB, SILBuilderContext &C,
29973063
const SILDebugScope *debugScope)
2998-
: SILBuilder(BB, debugScope, C) {}
3064+
: SILBuilder(BB, C, debugScope) {}
29993065

30003066
/// Creates a new SILBuilder with an insertion point at the
30013067
/// beginning of BB and the debug scope from the first

include/swift/SIL/SILCloner.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,12 @@ SILCloner<ImplClass>::visitAllocBoxInst(AllocBoxInst *Inst) {
962962
Inst,
963963
getBuilder().createAllocBox(
964964
Loc, this->getOpType(Inst->getType()).template castTo<SILBoxType>(),
965-
VarInfo));
965+
VarInfo, false, false, false
966+
#ifndef NDEBUG
967+
,
968+
true
969+
#endif
970+
));
966971
}
967972

968973
template<typename ImplClass>

0 commit comments

Comments
 (0)