Skip to content

Enter a new debug scope for every let binding #39345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
std::vector<PatternMatchContext*> SwitchStack;
/// Keep track of our current nested scope.
///
/// The boolean tracks whether this is a 'guard' scope, which should be
/// The boolean tracks whether this is a binding scope, which should be
/// popped automatically when we leave the innermost BraceStmt scope.
std::vector<llvm::PointerIntPair<const SILDebugScope *, 1>> DebugScopeStack;

Expand Down Expand Up @@ -583,19 +583,20 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction

/// Enter the debug scope for \p Loc, creating it if necessary.
///
/// \param isGuardScope If true, this is a scope for the bindings introduced by
/// a 'guard' statement. This scope ends when the next innermost BraceStmt ends.
void enterDebugScope(SILLocation Loc, bool isGuardScope=false) {
auto *Parent =
DebugScopeStack.size() ? DebugScopeStack.back().getPointer() : F.getDebugScope();
/// \param isBindingScope If true, this is a scope for the bindings introduced
/// by a let expression. This scope ends when the next innermost BraceStmt
/// ends.
void enterDebugScope(SILLocation Loc, bool isBindingScope = false) {
auto *Parent = DebugScopeStack.size() ? DebugScopeStack.back().getPointer()
: F.getDebugScope();
auto *DS = Parent;
// Don't create a pointless scope for the function body's BraceStmt.
if (!DebugScopeStack.empty())
// Don't nest a scope for Loc under Parent unless it's actually different.
if (RegularLocation(DS->getLoc()) != RegularLocation(Loc))
DS = new (SGM.M)
SILDebugScope(RegularLocation(Loc), &getFunction(), DS);
DebugScopeStack.emplace_back(DS, isGuardScope);
DebugScopeStack.emplace_back(DS, isBindingScope);
B.setCurrentDebugScope(DS);
}

Expand Down
3 changes: 0 additions & 3 deletions lib/SILGen/SILGenStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -784,9 +784,6 @@ void StmtEmitter::visitGuardStmt(GuardStmt *S) {
// Emit the condition bindings, branching to the bodyBB if they fail.
auto NumFalseTaken = SGF.loadProfilerCount(S->getBody());
auto NumNonTaken = SGF.loadProfilerCount(S);
// Begin a new 'guard' scope, which is popped when the next innermost debug
// scope ends.
SGF.enterDebugScope(S, /*isGuardScope=*/true);
SGF.emitStmtCondition(S->getCond(), bodyBB, S, NumNonTaken, NumFalseTaken);
}

Expand Down
5 changes: 5 additions & 0 deletions lib/SILGen/SwitchEnumBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ void SwitchEnumBuilder::emit() && {
// Don't allow cleanups to escape the conditional block.
SwitchCaseFullExpr presentScope(builder.getSILGenFunction(),
CleanupLocation(loc), branchDest);
// Begin a new binding scope, which is popped when the next innermost debug
// scope ends. The cleanup location loc isn't the perfect source location
// but it's close enough.
builder.getSILGenFunction().enterDebugScope(loc,
/*isBindingScope=*/true);

builder.emitBlock(caseBlock);

Expand Down
2 changes: 1 addition & 1 deletion test/DebugInfo/guard-let-scope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
func f(c: AnyObject?) {
let x = c
// CHECK: sil_scope [[S1:[0-9]+]] { {{.*}} parent @{{.*}}1f
// CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":[[@LINE+3]]:3 parent [[S1]] }
// CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":[[@LINE+3]]:17 parent [[S1]] }
// CHECK: debug_value %{{.*}} : $Optional<AnyObject>, let, name "x"{{.*}} scope [[S1]]
// CHECK: debug_value %{{.*}} : $AnyObject, let, name "x", {{.*}} scope [[S2]]
guard let x = x else {
Expand Down
8 changes: 4 additions & 4 deletions test/SILOptimizer/constantprop-wrongscope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
// Make sure that the destroy_addr instruction has the same scope of the
// instructions surrounding it.

// CHECK: destroy_addr %7 : $*Any, loc {{.*}}:22:19, scope 1
// CHECK: dealloc_stack %12 : $*Optional<Any>, loc {{.*}}:22:23, scope 1
// CHECK: dealloc_stack %7 : $*Any, loc {{.*}}:22:23, scope 1
// CHECK: dealloc_stack %6 : $*A, loc {{.*}}:22:7, scope 1
// CHECK: destroy_addr %7 : $*Any, loc {{.*}}:22:19, scope 2
// CHECK: dealloc_stack %12 : $*Optional<Any>, loc {{.*}}:22:23, scope 2
// CHECK: dealloc_stack %7 : $*Any, loc {{.*}}:22:23, scope 2
// CHECK: dealloc_stack %6 : $*A, loc {{.*}}:22:7, scope 2

import Foundation
func indexedSubscripting(b b: B, idx: Int, a: A) {
Expand Down
14 changes: 7 additions & 7 deletions test/SILOptimizer/definite-init-wrongscope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ public class M {

// CHECK-LABEL: sil [ossa] @$s3del1MC4fromAcA12WithDelegate_p_tKcfc : $@convention(method) (@in WithDelegate, @owned M) -> (@owned M, @error Error)

// CHECK: [[I:%.*]] = integer_literal $Builtin.Int2, 1, loc {{.*}}:23:12, scope 2
// CHECK: [[V:%.*]] = load [trivial] %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 2
// CHECK: [[OR:%.*]] = builtin "or_Int2"([[V]] : $Builtin.Int2, [[I]] : $Builtin.Int2) : $Builtin.Int2, loc {{.*}}:23:12, scope 2
// CHECK: store [[OR]] to [trivial] %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 2
// CHECK: store %{{.*}} to [init] %{{.*}} : $*C, loc {{.*}}:26:20, scope 2
// CHECK: [[I:%.*]] = integer_literal $Builtin.Int2, 1, loc {{.*}}:23:12, scope 4
// CHECK: [[V:%.*]] = load [trivial] %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 4
// CHECK: [[OR:%.*]] = builtin "or_Int2"([[V]] : $Builtin.Int2, [[I]] : $Builtin.Int2) : $Builtin.Int2, loc {{.*}}:23:12, scope 4
// CHECK: store [[OR]] to [trivial] %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 4
// CHECK: store %{{.*}} to [init] %{{.*}} : $*C, loc {{.*}}:26:20, scope 4

// Make sure the dealloc_stack gets the same scope of the instructions surrounding it.

// CHECK: destroy_addr %0 : $*WithDelegate, loc {{.*}}:29:5, scope 2
// CHECK: dealloc_stack %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 2
// CHECK: destroy_addr %0 : $*WithDelegate, loc {{.*}}:29:5, scope 4
// CHECK: dealloc_stack %2 : $*Builtin.Int2, loc {{.*}}:23:12, scope 4
// CHECK: throw %{{.*}} : $Error, loc {{.*}}:23:12, scope 1