Skip to content

Commit dcd881e

Browse files
committed
SILDebugScopes: Remove a stateful workaround for Property Wrappers.
In an earlier version of the ASTScope -> SILdebugScope translation a workaround was put into place that would select the current debug scope if an ASTScope was marked as ignoreInDebugInfo. Removing this workaround makes the translation more predictable as it eliminated the dependency on the current SILBuilder state. The property wrapper tests still pass without this. This uncovers a situations with let bindings where the SIL instructions are emitted RHS before LHS, which does violate the di-hole verifier. This is addressed by relaxing the verifier for now. rdar://108736443
1 parent 3901219 commit dcd881e

File tree

3 files changed

+31
-14
lines changed

3 files changed

+31
-14
lines changed

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6283,6 +6283,15 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
62836283
// inherit the sourrounding scope in IRGenSIL.
62846284
if (SI.getLoc().getKind() == SILLocation::MandatoryInlinedKind)
62856285
continue;
6286+
// FIXME: There are situations where the execution legitimately goes
6287+
// backwards, such as
6288+
//
6289+
// while case let w: String? = Optional.some("b") {}
6290+
//
6291+
// where the RHS of the assignment gets run first and then the
6292+
// result is copied into the LHS.
6293+
if (!llvm::isa<BeginBorrowInst>(&SI) || !llvm::isa<CopyValueInst>(&SI))
6294+
continue;
62866295

62876296
// If we haven't seen this debug scope yet, update the
62886297
// map and go on.

lib/SILGen/SILGenFunction.cpp

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -370,20 +370,9 @@ SILGenFunction::getOrCreateScope(const ast_scope::ASTScopeImpl *ASTScope,
370370
// Decide whether to pick a parent scope instead.
371371
if (ASTScope->ignoreInDebugInfo()) {
372372
LLVM_DEBUG(llvm::dbgs() << "ignored\n");
373-
// FIXME: it would be more deterministic to use
374-
// getOrCreateScope(ASTScope->getParent().getPtrOrNull());
375-
// here. Unfortunately property wrappers rearrange AST
376-
// nodes without marking them as implicit, e.g.:
377-
//
378-
// @Wrapper(a) var v = b
379-
// ->
380-
// let _tmp = Constructor(a, b); var v = _tmp
381-
//
382-
// Since the arguments to Constructor aren't marked as implicit,
383-
// argument b is in the scope of v, but the call to Constructor
384-
// isn't, which correctly triggers the scope hole verifier.
385-
auto *CurScope = B.getCurrentDebugScope();
386-
return CurScope->InlinedCallSite != InlinedAt ? FnScope : CurScope;
373+
auto *ParentScope = getOrCreateScope(ASTScope->getParent().getPtrOrNull(),
374+
FnScope, InlinedAt);
375+
return ParentScope->InlinedCallSite != InlinedAt ? FnScope : ParentScope;
387376
}
388377

389378
// Collapse BraceStmtScopes whose parent is a .*BodyScope.

test/DebugInfo/guard-let-scope3.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %target-swift-frontend -g -emit-sil %s -parse-as-library -module-name a | %FileCheck %s
2+
public class C {}
3+
public enum MyError : Error {
4+
init() { self.init() }
5+
}
6+
public class S {
7+
private var c = [Int : C?]()
8+
public func f(_ i: Int) throws -> C {
9+
guard let x = c[i], let x else {
10+
// CHECK: sil_scope [[X1:[0-9]+]] { loc "{{.*}}":[[@LINE-1]]:5
11+
// CHECK: sil_scope [[X2:[0-9]+]] { loc "{{.*}}":[[@LINE-2]]:29
12+
// CHECK: debug_value {{.*}} : $Optional<C>, let, name "x", {{.*}}, scope [[X1]]
13+
// CHECK: debug_value %29 : $C, let, name "x", {{.*}}, scope [[X2]]
14+
// CHECK-NEXT: scope [[X2]]
15+
throw MyError()
16+
}
17+
return x
18+
}
19+
}

0 commit comments

Comments
 (0)