Compute VarDeclScopeMap up front.

adrian-prantl · adrian-prantl · commit a52f90056656 · 2023-06-17T10:38:39.000-07:00
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 (cherry picked from commit 329e428)
diff --git 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() ==
diff --git 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))
   }
 }
diff --git 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
diff --git 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]]
diff --git 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
diff --git 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]]
     }
diff --git 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()
+    }()
+  }
+}
diff --git 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))
   }
 }
diff --git 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

Original file line number	Diff line number	Diff line change
`@@ -9,10 +9,10 @@ func consume<T>(_ t: T) {}`
`9`	`9`	`public func f(_ s1: Set<Int>?, _ s2: Set<Int>?) {`
`10`	`10`	`switch (s1, s2) {`
`11`	`11`	`case (nil, let a), (let a, nil):`
`12`		`- // CHECK: debug_value {{.}} $Optional<Set<Int>>, let, name "a", {{.}}:[[@LINE-1]]:18, scope [[S1]]`
	`12`	`+ // CHECK: debug_value {{.}} $Optional<Set<Int>>, let, name "a", {{.}}:11:18, scope [[S2]]`
`13`	`13`	`consume(a)`
`14`	`14`	`case (let a?, _):`
`15`		`- // CHECK: debug_value {{.}} $Set<Int>, let, name "a", {{.}}:[[@LINE-1]]:13, scope [[S3]]`
	`15`	`+ // CHECK: debug_value {{.}} $Set<Int>, let, name "a", {{.}}:14:13, scope [[S3]]`
`16`	`16`	`consume((a))`
`17`	`17`	`}`
`18`	`18`	`}`