Compute VarDeclScopeMap up front.

The previous lazy discovery did not always work because sometimes a debug_value is emitted before the first SIL instruction in the variable's scope. rdar://110841130
2025-12-14 20:36:38 +01:00 · 2023-06-16 12:36:10 -07:00
parent 874ba0868b
commit 329e42849a
10 changed files with 83 additions and 51 deletions
--- a/lib/SILGen/SILGenFunction.cpp
+++ b/lib/SILGen/SILGenFunction.cpp
@@ -55,10 +55,51 @@ SILGenFunction::SILGenFunction(SILGenModule &SGM, SILFunction &F,
  assert(DC && "creating SGF without a DeclContext?");
  B.setInsertionPoint(createBasicBlock());
  B.setCurrentDebugScope(F.getDebugScope());
+
+  // Populate VarDeclScopeMap.
  SourceLoc SLoc = F.getLocation().getSourceLoc();
  if (SF && SLoc) {
    FnASTScope = ast_scope::ASTScopeImpl::findStartingScopeForLookup(SF, SLoc);
    ScopeMap.insert({{FnASTScope, nullptr}, F.getDebugScope()});
+
+    // Collect all variable declarations in this scope.
+    struct Consumer : public namelookup::AbstractASTScopeDeclConsumer {
+      const ast_scope::ASTScopeImpl *ASTScope;
+      VarDeclScopeMapTy &VarDeclScopeMap;
+      Consumer(const ast_scope::ASTScopeImpl *ASTScope,
+               VarDeclScopeMapTy &VarDeclScopeMap)
+          : ASTScope(ASTScope), VarDeclScopeMap(VarDeclScopeMap) {}
+
+      bool consume(ArrayRef<ValueDecl *> values,
+                   NullablePtr<DeclContext> baseDC) override {
+        LLVM_DEBUG(ASTScope->print(llvm::errs(), 0, false, false));
+        for (auto &value : values) {
+          LLVM_DEBUG({
+            if (value->hasName())
+              llvm::dbgs() << "+ " << value->getBaseIdentifier() << "\n";
+          });
+
+          assert((VarDeclScopeMap.count(value) == 0 ||
+                  VarDeclScopeMap[value] == ASTScope) &&
+                 "VarDecl appears twice");
+          VarDeclScopeMap.insert({value, ASTScope});
+        }
+        return false;
+      }
+      bool lookInMembers(const DeclContext *) const override { return false; }
+#ifndef NDEBUG
+      void startingNextLookupStep() override {}
+      void finishingLookup(std::string) const override {}
+      bool isTargetLookup() const override { return false; }
+#endif
+    };
+    const_cast<ast_scope::ASTScopeImpl *>(FnASTScope)
+        ->preOrderChildrenDo([&](ast_scope::ASTScopeImpl *ASTScope) {
+          if (!ASTScope->ignoreInDebugInfo()) {
+            Consumer consumer(ASTScope, VarDeclScopeMap);
+            ASTScope->lookupLocalsOrMembers(consumer);
+          }
+        });
  }
 }

@@ -202,8 +243,6 @@ const SILDebugScope *SILGenFunction::getScopeOrNull(SILLocation Loc,
  SourceLoc SLoc = Loc.getSourceLoc();
  if (!SF || LastSourceLoc == SLoc)
    return nullptr;
-  // Prime VarDeclScopeMap.
-  auto Scope = getOrCreateScope(SLoc);
  if (ForMetaInstruction)
    if (ValueDecl *ValDecl = Loc.getAsASTNode<ValueDecl>()) {
      // The source location of a VarDecl isn't necessarily in the same scope
@@ -212,7 +251,7 @@ const SILDebugScope *SILGenFunction::getScopeOrNull(SILLocation Loc,
      if (ValueScope != VarDeclScopeMap.end())
        return getOrCreateScope(ValueScope->second, F.getDebugScope());
    }
-  return Scope;
+  return getOrCreateScope(SLoc);
 }

 const SILDebugScope *SILGenFunction::getOrCreateScope(SourceLoc SLoc) {
@@ -406,32 +445,6 @@ SILGenFunction::getOrCreateScope(const ast_scope::ASTScopeImpl *ASTScope,
    return ParentScope->InlinedCallSite != InlinedAt ? FnScope : ParentScope;
  }

-  // Collect all variable declarations in this scope.
-  struct Consumer : public namelookup::AbstractASTScopeDeclConsumer {
-    const ast_scope::ASTScopeImpl *ASTScope;
-    VarDeclScopeMapTy &VarDeclScopeMap;
-    Consumer(const ast_scope::ASTScopeImpl *ASTScope,
-             VarDeclScopeMapTy &VarDeclScopeMap)
-        : ASTScope(ASTScope), VarDeclScopeMap(VarDeclScopeMap) {}
-
-    bool consume(ArrayRef<ValueDecl *> values,
-                 NullablePtr<DeclContext> baseDC) override {
-      for (auto &value : values) {
-        assert(VarDeclScopeMap.count(value) == 0 && "VarDecl appears twice");
-        VarDeclScopeMap.insert({value, ASTScope});
-      }
-      return false;
-    }
-    bool lookInMembers(const DeclContext *) const override { return false; }
-#ifndef NDEBUG
-    void startingNextLookupStep() override {}
-    void finishingLookup(std::string) const override {}
-    bool isTargetLookup() const override { return false; }
-#endif
-  };
-  Consumer consumer(ASTScope, VarDeclScopeMap);
-  ASTScope->lookupLocalsOrMembers(consumer);
-
  // Collapse BraceStmtScopes whose parent is a .*BodyScope.
  if (auto Parent = ASTScope->getParent().getPtrOrNull())
    if (Parent->getSourceRangeOfThisASTNode() ==
--- a/test/DebugInfo/case-scope2.swift
+++ b/test/DebugInfo/case-scope2.swift
@@ -9,10 +9,10 @@ func consume<T>(_ t: T) {}
 public func f(_ s1: Set<Int>?, _ s2: Set<Int>?) {
  switch (s1, s2) {
  case (nil, let a), (let a, nil):
-  // CHECK: debug_value {{.*}} $Optional<Set<Int>>, let, name "a", {{.*}}:[[@LINE-1]]:18, scope [[S1]]
+  // CHECK: debug_value {{.*}} $Optional<Set<Int>>, let, name "a", {{.*}}:11:18, scope [[S2]]
    consume(a)
  case (let a?, _):
-  // CHECK: debug_value {{.*}} $Set<Int>, let, name "a", {{.*}}:[[@LINE-1]]:13, scope [[S3]]
+  // CHECK: debug_value {{.*}} $Set<Int>, let, name "a", {{.*}}:14:13, scope [[S3]]
    consume((a))
  }
 }
--- a/test/DebugInfo/case-scope3.swift
+++ b/test/DebugInfo/case-scope3.swift
@@ -14,8 +14,8 @@ func f() -> Int {
 // CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":10:44 parent [[S0]] }
 // CHECK: sil_scope [[S3:[0-9]+]] { loc "{{.*}}":10:68 parent [[S2]] }
 // CHECK: sil_scope [[S4:[0-9]+]] { loc "{{.*}}":10:78 parent [[S3]] }
-// CHECK: debug_value {{.*}}: $Optional<Int>, let, name "nextValue", {{.*}}:10:31, scope [[S0]]
-// CHECK: debug_value {{.*}}: $Int, let, name "nextValue", {{.*}}:10:56, scope [[S2]]
+// CHECK: debug_value {{.*}}: $Optional<Int>, let, name "nextValue", {{.*}}:10:31, scope [[S2]]
+// CHECK: debug_value {{.*}}: $Int, let, name "nextValue", {{.*}}:10:56, scope [[S3]]
        return nextValue
    }
    return 0
--- a/test/DebugInfo/for-scope.swift
+++ b/test/DebugInfo/for-scope.swift
@@ -15,7 +15,7 @@ public func f(_ xs: [String?]) {
 // CHECK: sil_scope [[S5:[0-9]+]] { loc "{{.*}}":7:13 parent [[S4]] }
 // CHECK: sil_scope [[S6:[0-9]+]] { loc "{{.*}}":7:9 parent [[S4]] }

-// CHECK: debug_value %[[X:.*]] : $Optional<String>, let, name "x", {{.*}}, scope [[S3]]
+// CHECK: debug_value %[[X:.*]] : $Optional<String>, let, name "x", {{.*}}, scope [[S4]]
 // CHECK: retain_value %[[X]] : $Optional<String>, {{.*}}, scope [[S5]]
 // CHECK: debug_value %[[X1:[0-9]+]] : $String, let, name "x", {{.*}}, scope [[S6]]
 // CHECK:  release_value %[[X1]] : $String, {{.*}}, scope [[S6]]
--- a/test/DebugInfo/guard-let-scope.swift
+++ b/test/DebugInfo/guard-let-scope.swift
@@ -14,8 +14,8 @@ func f(c: AnyObject??) {
  // CHECK: sil_scope [[S9:[0-9]+]] { loc "{{.*}}":7:28 parent [[S8]] }
  // CHECK: sil_scope [[S10:[0-9]+]] { loc "{{.*}}":7:28 parent [[S8]] }
  // CHECK: debug_value %{{.*}} : $Optional<Optional<AnyObject>>, let, name "x"{{.*}} scope [[S5]]
-  // CHECK: debug_value %{{.*}} : $Optional<AnyObject>, let, name "x", {{.*}} scope [[S6]]
-  // CHECK: debug_value %{{.*}} : $AnyObject, let, name "x", {{.*}} scope [[S8]]
+  // CHECK: debug_value %{{.*}} : $Optional<AnyObject>, let, name "x", {{.*}} scope [[S8]]
+  // CHECK: debug_value %{{.*}} : $AnyObject, let, name "x", {{.*}} scope [[S10]]
    fatalError()
  }
  // CHECK: function_ref {{.*3use.*}} scope [[S10]]
--- a/test/DebugInfo/guard-let-scope2.swift
+++ b/test/DebugInfo/guard-let-scope2.swift
@@ -16,11 +16,12 @@ public func f(x: String?) throws {
    s = SomeObject()
    return s != nil
  }
-  // CHECK: sil_scope [[S1:[0-9]+]] { {{.*}} parent @{{.*}}1f
-  // CHECK: sil_scope [[S2:[0-9]+]] { {{.*}} parent [[S1]] }
-  // CHECK: sil_scope [[S3:[0-9]+]] { {{.*}} parent [[S1]] }
-  // CHECK: sil_scope [[S4:[0-9]+]] { {{.*}} parent [[S2]] }
-  // CHECK: alloc_stack {{.*}} $SomeObject, let, name "s", {{.*}} scope [[S4]]
+  // CHECK: sil_scope [[S1:[0-9]+]] { {{.*}}:13:13 parent @{{.*}}1f
+  // CHECK: sil_scope [[S2:[0-9]+]] { {{.*}}:14:7  parent [[S1]] }
+  // CHECK: sil_scope [[S3:[0-9]+]] { {{.*}}:14:26 parent [[S1]] }
+  // CHECK: sil_scope [[S4:[0-9]+]] { {{.*}}:25:3 parent [[S2]] }
+  // CHECK: sil_scope [[S5:[0-9]+]] { {{.*}}:25:17 parent [[S4]] }
+  // CHECK: alloc_stack {{.*}} $SomeObject, let, name "s", {{.*}} scope [[S5]]
  guard let s = s else {
    assert(false)
    return
--- a/test/DebugInfo/guard-let-scope3.swift
+++ b/test/DebugInfo/guard-let-scope3.swift
@@ -8,15 +8,14 @@ public class S {
  public func f(_ i: Int) throws  -> C {
    guard let x = c[i], let x else {
      // CHECK: sil_scope [[P:[0-9]+]] { loc "{{.*}}":9:5
-      // CHECK: sil_scope [[X1_:[0-9]+]] { loc "{{.*}}":9:19 parent [[P]]
+      // CHECK: sil_scope [[X1_RHS:[0-9]+]] { loc "{{.*}}":9:19 parent [[P]]
      // CHECK: sil_scope [[X1:[0-9]+]] { loc "{{.*}}":9:19 parent [[P]]
      // CHECK: sil_scope [[X2:[0-9]+]] { loc "{{.*}}":9:29 parent [[X1]]
-      // CHECK: sil_scope [[X2_:[0-9]+]] { loc "{{.*}}":9:29 parent [[X1]]
      // CHECK: sil_scope [[GUARD:[0-9]+]] { loc "{{.*}}":9:36 parent [[P]]
-      // CHECK: debug_value {{.*}} : $Optional<C>, let, name "x", {{.*}}, scope [[P]]
+      // CHECK: debug_value {{.*}} : $Optional<C>, let, name "x", {{.*}}, scope [[X1]]
      // CHECK: debug_value {{.*}} : $C, let, name "x", {{.*}}, scope [[X2]]
-      // FIXME: This source location & scope are a little wild.
-      // CHECK-NEXT: strong_retain{{.*}}:[[@LINE+4]]:12, scope [[X2_]]
+      // FIXME: This source location is a little wild.
+      // CHECK-NEXT: strong_retain{{.*}}:[[@LINE+4]]:12, scope [[X2]]
      throw MyError()
      // CHECK: function_ref {{.*}}MyError{{.*}}:[[@LINE-1]]:13, scope [[GUARD]]
    }
--- a/test/DebugInfo/guard-let-scope4.swift
+++ b/test/DebugInfo/guard-let-scope4.swift
@@ -0,0 +1,18 @@
+// RUN: %target-swift-frontend -g -emit-sil %s -parse-as-library -module-name a | %FileCheck %s
+open class C {
+  public func fun() {}
+
+  public func run() {
+    { [weak self] in
+      guard let self else { fatalError("cannot happen") }
+      // CHECK: sil_scope [[LAMBDA:[0-9]+]] { loc "{{.*}}":6:5
+      // CHECK: sil_scope [[BODY:[0-9]+]] { loc "{{.*}}":6:19 parent [[LAMBDA]]
+      // CHECK: sil_scope [[LET:[0-9]+]] { loc "{{.*}}":7:7 parent [[BODY]]
+      // CHECK: sil_scope [[GUARD:[0-9]+]] { loc "{{.*}}":7:17 parent [[LET]]
+      // CHECK: debug_value {{.*}} : $C, let, name "self", {{.*}}, scope [[GUARD]]
+      // CHECK: function_ref {{.*}}3fun{{.*}}, scope [[GUARD]]
+      // CHECK-NEXT: apply {{.*}}, scope [[GUARD]]
+      self.fun()
+    }()
+  }
+}
--- a/test/DebugInfo/if-let-scope.swift
+++ b/test/DebugInfo/if-let-scope.swift
@@ -3,13 +3,14 @@ func use<T>(_ t: T) {}
 public func f(value: String?) {
  // CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":3:13
  if let value, let value = Int(value) {
-    // CHECK: sil_scope [[S1:[0-9]+]] { loc "{{.*}}":5:10
+    // CHECK: sil_scope [[S1:[0-9]+]] { loc "{{.*}}":5:3
    // CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":5:10
    // CHECK: sil_scope [[S3:[0-9]+]] { loc "{{.*}}":5:29 parent [[S2]] }
    // CHECK: sil_scope [[S4:[0-9]+]] { loc "{{.*}}":5:29 parent [[S2]] }
+    // CHECK: sil_scope [[S5:[0-9]+]] { loc "{{.*}}":5:40 parent [[S4]] }
    // CHECK: debug_value {{.*}} : $Optional<String>, let, name "value", {{.*}}, scope [[S0]]
-    // CHECK: debug_value {{.*}} : $String, let, name "value", {{.*}}, scope [[S1]]
-    // CHECK: debug_value {{.*}} : $Int, let, name "value", {{.*}}, scope [[S2]]
+    // CHECK: debug_value {{.*}} : $String, let, name "value", {{.*}}, scope [[S2]]
+    // CHECK: debug_value {{.*}} : $Int, let, name "value", {{.*}}, scope [[S4]]
    use((value))
  }
 }
--- a/test/SILGen/switch-case-debug-descriptions.swift
+++ b/test/SILGen/switch-case-debug-descriptions.swift
@@ -9,7 +9,7 @@ enum E {
 func test1(_ e: E) {
  switch e {                                  // SCOPE: sil_scope [[test1_switch:[0-9]+]] {{.*}}:[[@LINE]]:3
  case .one(let payload), .two(let payload):  // SCOPE-NEXT: sil_scope [[test1_case1:[0-9]+]] {{.*}}:[[@LINE]]:3 parent [[test1_switch]]
-    print(payload)
+    print(payload)                            // SCOPE-NEXT: sil_scope                        {{.*}}:[[@LINE]]:5 parent [[test1_case1]]
  case .three(let payload):                   // SCOPE-NEXT: sil_scope [[test1_case2:[0-9]+]] {{.*}}:[[@LINE]]:3 parent [[test1_switch]]
    print(payload)
  }
@@ -43,7 +43,7 @@ func test4(_ e: E) {
    print(x)                                  // SCOPE-NEXT: sil_scope {{.*}}:[[@LINE]]:5 parent [[test4_case1]]
    fallthrough
  case .two(let x):                           // SCOPE-NEXT: sil_scope [[test4_case2:[0-9]+]] {{.*}}:[[@LINE]]:3 parent [[test4_switch]]
-    print(x)
+    print(x)                                  // SCOPE-NEXT:                                    {{.*}}:[[@LINE]]:5 parent [[test4_case2]]
    fallthrough
  default:                                    // SCOPE-NEXT: sil_scope [[test4_default:[0-9]+]] {{.*}}:[[@LINE]]:3 parent [[test4_switch]]
    print("default")                          // SCOPE:      sil_scope [[test4_default1:[0-9]+]] {{.*}}:[[@LINE]]:5