Skip to content

Commit 158772c

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 0337ea9 commit 158772c

File tree

77 files changed

+892
-426
lines changed

Some content is hidden

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

77 files changed

+892
-426
lines changed

include/swift/AST/ASTScope.h

Lines changed: 15 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
};
@@ -972,6 +984,7 @@ class ConditionalClauseInitializerScope final : public ASTScopeImpl {
972984
SourceRange
973985
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
974986
std::string getClassName() const override;
987+
bool ignoreInDebugInfo() const override { return true; }
975988

976989
private:
977990
void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
@@ -1049,6 +1062,7 @@ class ClosureParametersScope final : public ASTScopeImpl {
10491062
}
10501063
NullablePtr<Expr> getExprIfAny() const override { return closureExpr; }
10511064
Expr *getExpr() const { return closureExpr; }
1065+
bool ignoreInDebugInfo() const override { return true; }
10521066

10531067
protected:
10541068
ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;

include/swift/SIL/SILBuilder.h

Lines changed: 84 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

@@ -168,27 +171,33 @@ class SILBuilder {
168171
/// location.
169172
///
170173
/// SILBuilderContext must outlive this SILBuilder instance.
171-
SILBuilder(SILInstruction *I, const SILDebugScope *DS, SILBuilderContext &C)
174+
SILBuilder(SILInstruction *I, SILBuilderContext &C,
175+
const SILDebugScope *DS = nullptr)
172176
: TempContext(C.getModule()), C(C), F(I->getFunction()) {
173-
assert(DS && "instruction has no debug scope");
174-
setCurrentDebugScope(DS);
175177
setInsertionPoint(I);
178+
if (DS)
179+
setCurrentDebugScope(DS);
176180
}
177181

178-
SILBuilder(SILBasicBlock *BB, const SILDebugScope *DS, SILBuilder &B)
179-
: SILBuilder(BB, DS, B.getBuilderContext()) {}
182+
SILBuilder(SILBasicBlock *BB, SILBuilder &B,
183+
const SILDebugScope *DS = nullptr)
184+
: SILBuilder(BB, B.getBuilderContext(), DS) {}
180185

181186
/// Build instructions before the given insertion point, inheriting the debug
182187
/// location.
183188
///
184189
/// SILBuilderContext must outlive this SILBuilder instance.
185-
SILBuilder(SILBasicBlock *BB, const SILDebugScope *DS, SILBuilderContext &C)
190+
SILBuilder(SILBasicBlock *BB, SILBuilderContext &C,
191+
const SILDebugScope *DS = nullptr)
186192
: TempContext(C.getModule()), C(C), F(BB->getParent()) {
187193
assert(DS && "block has no debug scope");
188-
setCurrentDebugScope(DS);
189194
setInsertionPoint(BB);
195+
if (DS)
196+
setCurrentDebugScope(DS);
190197
}
191198

199+
virtual ~SILBuilder() {}
200+
192201
// Allow a pass to override the current SIL module conventions. This should
193202
// only be done by a pass responsible for lowering SIL to a new stage
194203
// (e.g. AddressLowering).
@@ -238,7 +247,8 @@ class SILBuilder {
238247
}
239248

240249
/// Convenience function for building a SILDebugLocation.
241-
SILDebugLocation getSILDebugLocation(SILLocation Loc) {
250+
virtual SILDebugLocation
251+
getSILDebugLocation(SILLocation Loc, bool ForMetaInstruction = false) {
242252
// FIXME: Audit all uses and enable this assertion.
243253
// assert(getCurrentDebugScope() && "no debug scope");
244254
auto Scope = getCurrentDebugScope();
@@ -248,6 +258,17 @@ class SILBuilder {
248258
return SILDebugLocation(overriddenLoc, Scope);
249259
}
250260

261+
/// When the frontend generates synthesized conformances it generates a
262+
/// fully-typechecked AST without source locations. This means that the
263+
/// ASTScope-based mechanism to generate SILDebugScopes doesn't work, which
264+
/// means we can't disambiguate local variables in different lexical
265+
/// scopes. To avoid a verification error later in the pipeline, drop all
266+
/// variables without a proper source location.
267+
bool shouldDropVariable(SILDebugVariable Var, SILLocation Loc) {
268+
return !Var.ArgNo && Loc.isSynthesizedAST();
269+
}
270+
271+
251272
/// If we have a SILFunction, return SILFunction::hasOwnership(). If we have a
252273
/// SILGlobalVariable, just return false.
253274
bool hasOwnership() const {
@@ -369,7 +390,7 @@ class SILBuilder {
369390
SILBasicBlock *createFallthroughBlock(SILLocation loc,
370391
SILBasicBlock *targetBB) {
371392
auto *newBB = F->createBasicBlock();
372-
SILBuilder(newBB, this->getCurrentDebugScope(), this->getBuilderContext())
393+
SILBuilder(newBB, this->getBuilderContext(), this->getCurrentDebugScope())
373394
.createBranch(loc, targetBB);
374395
return newBB;
375396
}
@@ -382,6 +403,8 @@ class SILBuilder {
382403
Optional<SILDebugVariable>
383404
substituteAnonymousArgs(llvm::SmallString<4> Name,
384405
Optional<SILDebugVariable> Var, SILLocation Loc) {
406+
if (Var && shouldDropVariable(*Var, Loc))
407+
return {};
385408
if (!Var || !Var->ArgNo || !Var->Name.empty())
386409
return Var;
387410

@@ -397,14 +420,21 @@ class SILBuilder {
397420
AllocStackInst *createAllocStack(SILLocation Loc, SILType elementType,
398421
Optional<SILDebugVariable> Var = None,
399422
bool hasDynamicLifetime = false,
400-
bool isLexical = false,
401-
bool wasMoved = false) {
423+
bool isLexical = false, bool wasMoved = false
424+
#ifndef NDEBUG
425+
,
426+
bool skipVarDeclAssert = false
427+
#endif
428+
) {
402429
llvm::SmallString<4> Name;
403430
Loc.markAsPrologue();
404-
assert((!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
405-
"location is a VarDecl, but SILDebugVariable is empty");
431+
#ifndef NDEBUG
432+
if (dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()))
433+
assert((skipVarDeclAssert || Loc.isSynthesizedAST() || Var) &&
434+
"location is a VarDecl, but SILDebugVariable is empty");
435+
#endif
406436
return insert(AllocStackInst::create(
407-
getSILDebugLocation(Loc), elementType, getFunction(),
437+
getSILDebugLocation(Loc, true), elementType, getFunction(),
408438
substituteAnonymousArgs(Name, Var, Loc), hasDynamicLifetime, isLexical,
409439
wasMoved));
410440
}
@@ -455,15 +485,20 @@ class SILBuilder {
455485
Optional<SILDebugVariable> Var = None,
456486
bool hasDynamicLifetime = false,
457487
bool reflection = false,
458-
bool usesMoveableValueDebugInfo = false) {
488+
bool usesMoveableValueDebugInfo = false
489+
#ifndef NDEBUG
490+
, bool skipVarDeclAssert = false
491+
#endif
492+
) {
459493
llvm::SmallString<4> Name;
460494
Loc.markAsPrologue();
461-
assert((!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
495+
assert((skipVarDeclAssert ||
496+
!dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()) || Var) &&
462497
"location is a VarDecl, but SILDebugVariable is empty");
463-
return insert(AllocBoxInst::create(getSILDebugLocation(Loc), BoxType, *F,
464-
substituteAnonymousArgs(Name, Var, Loc),
465-
hasDynamicLifetime, reflection,
466-
usesMoveableValueDebugInfo));
498+
return insert(AllocBoxInst::create(
499+
getSILDebugLocation(Loc, true), BoxType, *F,
500+
substituteAnonymousArgs(Name, Var, Loc), hasDynamicLifetime, reflection,
501+
usesMoveableValueDebugInfo));
467502
}
468503

469504
AllocExistentialBoxInst *
@@ -2918,7 +2953,21 @@ class SILBuilder {
29182953
class SILBuilderWithScope : public SILBuilder {
29192954
void inheritScopeFrom(SILInstruction *I) {
29202955
assert(I->getDebugScope() && "instruction has no debug scope");
2921-
setCurrentDebugScope(I->getDebugScope());
2956+
SILBasicBlock::iterator II(*I);
2957+
auto End = I->getParent()->end();
2958+
const SILDebugScope *DS = II->getDebugScope();
2959+
assert(DS);
2960+
// Skip over meta instructions, since debug_values may originate from outer
2961+
// scopes. Don't do any of this after inlining.
2962+
while (!DS->InlinedCallSite && II != End && II->isMetaInstruction())
2963+
++II;
2964+
if (II != End) {
2965+
auto nextScope = II->getDebugScope();
2966+
if (!nextScope->InlinedCallSite)
2967+
DS = nextScope;
2968+
}
2969+
assert(DS);
2970+
setCurrentDebugScope(DS);
29222971
}
29232972

29242973
public:
@@ -2927,29 +2976,33 @@ class SILBuilderWithScope : public SILBuilder {
29272976
///
29282977
/// Clients should prefer this constructor.
29292978
SILBuilderWithScope(SILInstruction *I, SILBuilderContext &C)
2930-
: SILBuilder(I, I->getDebugScope(), C)
2931-
{}
2979+
: SILBuilder(I, C) {
2980+
inheritScopeFrom(I);
2981+
}
29322982

29332983
/// Build instructions before the given insertion point, inheriting the debug
29342984
/// location and using the context from the passed in builder.
29352985
///
29362986
/// Clients should prefer this constructor.
29372987
SILBuilderWithScope(SILInstruction *I, SILBuilder &B)
2938-
: SILBuilder(I, I->getDebugScope(), B.getBuilderContext()) {}
2988+
: SILBuilder(I, B.getBuilderContext()) {
2989+
inheritScopeFrom(I);
2990+
}
29392991

29402992
explicit SILBuilderWithScope(
29412993
SILInstruction *I,
29422994
SmallVectorImpl<SILInstruction *> *InsertedInstrs = nullptr)
29432995
: SILBuilder(I, InsertedInstrs) {
2944-
assert(I->getDebugScope() && "instruction has no debug scope");
2945-
setCurrentDebugScope(I->getDebugScope());
2996+
inheritScopeFrom(I);
29462997
}
29472998

29482999
explicit SILBuilderWithScope(SILBasicBlock::iterator I)
29493000
: SILBuilderWithScope(&*I) {}
29503001

29513002
explicit SILBuilderWithScope(SILBasicBlock::iterator I, SILBuilder &B)
2952-
: SILBuilder(&*I, &*I->getDebugScope(), B.getBuilderContext()) {}
3003+
: SILBuilder(&*I, B.getBuilderContext()) {
3004+
inheritScopeFrom(&*I);
3005+
}
29533006

29543007
explicit SILBuilderWithScope(SILInstruction *I,
29553008
SILInstruction *InheritScopeFrom)
@@ -2971,12 +3024,13 @@ class SILBuilderWithScope : public SILBuilder {
29713024

29723025
explicit SILBuilderWithScope(SILBasicBlock *BB, SILBuilder &B,
29733026
SILInstruction *InheritScopeFrom)
2974-
: SILBuilder(BB, InheritScopeFrom->getDebugScope(),
2975-
B.getBuilderContext()) {}
3027+
: SILBuilder(BB, B.getBuilderContext()) {
3028+
inheritScopeFrom(InheritScopeFrom);
3029+
}
29763030

29773031
explicit SILBuilderWithScope(SILBasicBlock *BB, SILBuilderContext &C,
29783032
const SILDebugScope *debugScope)
2979-
: SILBuilder(BB, debugScope, C) {}
3033+
: SILBuilder(BB, C, debugScope) {}
29803034

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

include/swift/SIL/SILCloner.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,11 @@ SILCloner<ImplClass>::visitAllocStackInst(AllocStackInst *Inst) {
882882
auto *NewInst = getBuilder().createAllocStack(
883883
Loc, getOpType(Inst->getElementType()), VarInfo,
884884
Inst->hasDynamicLifetime(), Inst->isLexical(),
885-
Inst->getUsesMoveableValueDebugInfo());
885+
Inst->getUsesMoveableValueDebugInfo()
886+
#ifndef NDEBUG
887+
, true
888+
#endif
889+
);
886890
remapDebugVarInfo(DebugVarCarryingInst(NewInst));
887891
recordClonedInstruction(Inst, NewInst);
888892
}
@@ -950,7 +954,12 @@ SILCloner<ImplClass>::visitAllocBoxInst(AllocBoxInst *Inst) {
950954
Inst,
951955
getBuilder().createAllocBox(
952956
Loc, this->getOpType(Inst->getType()).template castTo<SILBoxType>(),
953-
VarInfo));
957+
VarInfo, false, false, false
958+
#ifndef NDEBUG
959+
,
960+
true
961+
#endif
962+
));
954963
}
955964

956965
template<typename ImplClass>

0 commit comments

Comments
 (0)