Derive the SILDebugScope for a variable declaration from its owning ASTScope.

The previous code made the assumption that the ASTScope for a variable
declaration should be the one of the declaration's source location. That is not
necessarily the case, in some cases it should be an ancestor scope. This patch
introduces a map from ValueDecl -> ASTScope that is derived from querying each
ASTScope for its locals, which matches also what happens in name lookup.  This
patch also fixes the nesting of SILDebugScopes created for guard statement
bodies, which are incorrectly nested in the ASTScope hierarchy.

rdar://108940570

Author: adrian-prantl

lib/SILGen/SILGenFunction.cpp

@@ -32,6 +32,7 @@
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/AST/Types.h"
+#include "swift/Basic/Defer.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILProfiler.h"
 #include "swift/SIL/SILUndef.h"
@@ -201,7 +202,17 @@ const SILDebugScope *SILGenFunction::getScopeOrNull(SILLocation Loc,
   SourceLoc SLoc = Loc.getSourceLoc();
   if (!SF || LastSourceLoc == SLoc)
     return nullptr;
-  return getOrCreateScope(SLoc);
+  // 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
+      // that the variable resides in for name lookup purposes.
+      auto ValueScope = VarDeclScopeMap.find(ValDecl);
+      if (ValueScope != VarDeclScopeMap.end())
+        return getOrCreateScope(ValueScope->second, F.getDebugScope());
+    }
+  return Scope;
 }
 
 const SILDebugScope *SILGenFunction::getOrCreateScope(SourceLoc SLoc) {
@@ -378,9 +389,14 @@ SILGenFunction::getOrCreateScope(const ast_scope::ASTScopeImpl *ASTScope,
   if (It != ScopeMap.end())
     return It->second;
 
-  LLVM_DEBUG( ASTScope->print(llvm::errs(), 0, false, false) );
+  LLVM_DEBUG(ASTScope->print(llvm::errs(), 0, false, false));
 
-  SILDebugScope *SILScope = nullptr;
+  auto cache = [&](const SILDebugScope *SILScope) {
+    ScopeMap.insert({{ASTScope, InlinedAt}, SILScope});
+    assert(SILScope->getParentFunction() == &F &&
+           "inlinedAt points to other function");
+    return SILScope;
+  };
 
   // Decide whether to pick a parent scope instead.
   if (ASTScope->ignoreInDebugInfo()) {
@@ -390,44 +406,68 @@ 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() ==
         ASTScope->getSourceRangeOfThisASTNode())
-      return getOrCreateScope(Parent, FnScope, InlinedAt);
+      return cache(getOrCreateScope(Parent, FnScope, InlinedAt));
 
   // The calls to defer closures have cleanup source locations pointing to the
   // defer. Reparent them into the current debug scope.
   auto *AncestorScope = ASTScope->getParent().getPtrOrNull();
   while (AncestorScope && AncestorScope != FnASTScope &&
          !ScopeMap.count({AncestorScope, InlinedAt})) {
     if (auto *FD = dyn_cast_or_null<FuncDecl>(
-          AncestorScope->getDeclIfAny().getPtrOrNull())) {
+            AncestorScope->getDeclIfAny().getPtrOrNull())) {
       if (cast<DeclContext>(FD) != FunctionDC)
-        return B.getCurrentDebugScope();
+        return cache(B.getCurrentDebugScope());
 
       // This is this function's own scope.
       // If this is the outermost BraceStmt scope, ignore it.
       if (AncestorScope == ASTScope->getParent().getPtrOrNull())
-        return FnScope;
+        return cache(FnScope);
       break;
     }
 
     AncestorScope = AncestorScope->getParent().getPtrOrNull();
   };
 
+  // Create the scope and recursively its parents.  getLookupParent implements a
+  // special case for GuardBlockStmt, which is nested incorrectly.
+  auto *ParentScope = ASTScope->getLookupParent().getPtrOrNull();
   const SILDebugScope *Parent =
-    getOrCreateScope(ASTScope->getParent().getPtrOrNull(), FnScope, InlinedAt);
+      getOrCreateScope(ParentScope, FnScope, InlinedAt);
   SourceLoc SLoc = ASTScope->getSourceRangeOfThisASTNode().Start;
   RegularLocation Loc(SLoc);
-  SILScope = new (SGM.M)
+  auto *SILScope = new (SGM.M)
       SILDebugScope(Loc, FnScope->getParentFunction(), Parent, InlinedAt);
-  ScopeMap.insert({{ASTScope, InlinedAt}, SILScope});
-
-  assert(SILScope->getParentFunction() == &F &&
-         "inlinedAt points to other function");
-
-  return SILScope;
+  return cache(SILScope);
 }
 
 void SILGenFunction::enterDebugScope(SILLocation Loc, bool isBindingScope) {

lib/SILGen/SILGenFunction.h

@@ -366,6 +366,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   SourceLoc LastSourceLoc;
   using ASTScopeTy = ast_scope::ASTScopeImpl;
   const ASTScopeTy *FnASTScope = nullptr;
+  using VarDeclScopeMapTy =
+      llvm::SmallDenseMap<ValueDecl *, const ASTScopeTy *, 8>;
+  /// The ASTScope each variable declaration belongs to.
+  VarDeclScopeMapTy VarDeclScopeMap;
   /// Caches one SILDebugScope for each ASTScope.
   llvm::SmallDenseMap<std::pair<const ASTScopeTy *, const SILDebugScope *>,
                       const SILDebugScope *, 16>

test/DebugInfo/for-scope.swift

@@ -8,14 +8,15 @@ public func f(_ xs: [String?]) {
     sink(x)
   }
 }
+
 // CHECK: sil_scope [[F:[0-9]+]] { loc "{{.*}}":5:13 parent @$s1a1fyySaySSSgGF
-// CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":6:3 parent [[F]] }
-// CHECK: sil_scope [[S1:[0-9]+]] { loc "{{.*}}":6:15 parent [[S0]] }
-// CHECK: sil_scope [[S3:[0-9]+]] { loc "{{.*}}":7:9 parent [[S1]] }
-// CHECK: sil_scope [[S4:[0-9]+]] { loc "{{.*}}":7:13 parent [[S3]] }
+// CHECK: sil_scope [[S3:[0-9]+]] { loc "{{.*}}":6:3 parent [[F]] }
+// CHECK: sil_scope [[S4:[0-9]+]] { loc "{{.*}}":6:15 parent [[S3]] }
+// 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 [[S0]]
-// CHECK: retain_value %[[X]] : $Optional<String>, {{.*}}, scope [[S4]]
-// CHECK: debug_value %[[X1:[0-9]+]] : $String, let, name "x", {{.*}}, scope [[S3]]
-// CHECK:  release_value %[[X1]] : $String, {{.*}}, scope [[S3]]
-// CHECK:  release_value %[[X]] : $Optional<String>, {{.*}}, scope [[S3]]
+// CHECK: debug_value %[[X:.*]] : $Optional<String>, let, name "x", {{.*}}, scope [[S3]]
+// 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]]
+// CHECK:  release_value %[[X]] : $Optional<String>, {{.*}}, scope [[S6]]

test/DebugInfo/guard-let-scope.swift

@@ -7,13 +7,13 @@ func f(c: AnyObject??) {
   guard let x = x, let x = x else {
   // CHECK: sil_scope [[S3:[0-9]+]] { {{.*}} parent @{{.*}}1f
   // CHECK: sil_scope [[S4:[0-9]+]] { {{.*}} parent [[S3]] }
-  // CHECK: sil_scope [[S5:[0-9]+]] { {{.*}} parent [[S4]] }
-  // CHECK: sil_scope [[S6:[0-9]+]] { loc "{{.*}}":7:3 parent [[S4]] }
+  // CHECK: sil_scope [[S5:[0-9]+]] { {{.*}} parent [[S3]] }
+  // CHECK: sil_scope [[S6:[0-9]+]] { loc "{{.*}}":7:3 parent [[S5]] }
   // CHECK: sil_scope [[S7:[0-9]+]] { loc "{{.*}}":7:17 parent [[S6]] }
   // CHECK: sil_scope [[S8:[0-9]+]] { loc "{{.*}}":7:28 parent [[S7]] }
-  // CHECK: debug_value %{{.*}} : $Optional<Optional<AnyObject>>, let, name "x"{{.*}} scope [[S4]]
-  // CHECK: debug_value %{{.*}} : $Optional<AnyObject>, let, name "x", {{.*}} scope [[S6]]
-  // CHECK: debug_value %{{.*}} : $AnyObject, let, name "x", {{.*}} scope [[S7]]
+  // CHECK: debug_value %{{.*}} : $Optional<Optional<AnyObject>>, let, name "x"{{.*}} scope [[S5]]
+  // CHECK: debug_value %{{.*}} : $Optional<AnyObject>, let, name "x", {{.*}} scope [[S7]]
+  // CHECK: debug_value %{{.*}} : $AnyObject, let, name "x", {{.*}} scope [[S8]]
     fatalError()
   }
   // CHECK: function_ref {{.*3use.*}} scope [[S8]]

test/DebugInfo/guard-let-scope2.swift

@@ -18,7 +18,7 @@ public func f(x: String?) throws {
   }
   // CHECK: sil_scope [[S1:[0-9]+]] { {{.*}} parent @{{.*}}1f
   // CHECK: sil_scope [[S2:[0-9]+]] { {{.*}} parent [[S1]] }
-  // CHECK: sil_scope [[S3:[0-9]+]] { {{.*}} parent [[S2]] }
+  // 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]]
   guard let s = s else {

test/DebugInfo/guard-let-scope3.swift

@@ -7,12 +7,15 @@ public class S {
   private var c = [Int : C?]()
   public func f(_ i: Int) throws  -> C {
     guard let x = c[i], let x else {
-    // CHECK: sil_scope [[X1:[0-9]+]] { loc "{{.*}}":[[@LINE-1]]:5
-    // CHECK: sil_scope [[X2:[0-9]+]] { loc "{{.*}}":[[@LINE-2]]:29
-    // CHECK: debug_value {{.*}} : $Optional<C>, let, name "x", {{.*}}, scope [[X1]]
-    // CHECK: debug_value %29 : $C, let, name "x", {{.*}}, scope [[X2]]
-    // CHECK-NEXT:  scope [[X2]]
+      // CHECK: sil_scope [[P:[0-9]+]] { loc "{{.*}}":[[@LINE-1]]:5
+      // CHECK: sil_scope [[X1:[0-9]+]] { loc "{{.*}}":[[@LINE-2]]:19 parent [[P]]
+      // CHECK: sil_scope [[X2:[0-9]+]] { loc "{{.*}}":[[@LINE-3]]:29 parent [[X1]]
+      // CHECK: sil_scope [[GUARD:[0-9]+]] { loc "{{.*}}":[[@LINE-4]]:36 parent [[P]]
+      // CHECK: debug_value {{.*}} : $Optional<C>, let, name "x", {{.*}}, scope [[X1]]
+      // CHECK: debug_value {{.*}} : $C, let, name "x", {{.*}}, scope [[X2]]
+      // CHECK-NEXT:  scope [[X2]]
       throw MyError()
+      // CHECK: function_ref {{.*}}MyError{{.*}}:[[@LINE-1]]:13, scope [[GUARD]]
     }
     return x
   }

test/DebugInfo/if-let-scope.swift

@@ -0,0 +1,13 @@
+// RUN: %target-swift-frontend -g -emit-sil %s -parse-as-library -module-name a | %FileCheck %s
+func use<T>(_ t: T) {}
+public func f(value: String?) {
+  // CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":[[@LINE-1]]:13
+  if let value, let value = Int(value) {
+    // CHECK: sil_scope [[S1:[0-9]+]] { loc "{{.*}}":[[@LINE-1]]:10
+    // CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":[[@LINE-2]]:29 parent [[S1]] }
+    // 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]]
+    use((value))
+  }
+}

test/DebugInfo/inlinescopes.swift

@@ -27,10 +27,9 @@ func inlined(_ x: Int64) -> Int64 {
   let result = transparent(x)
 // CHECK-DAG: ![[CALL]] = !DILocation(line: [[@LINE-1]], column: {{.*}}, scope: ![[INLINED1:.*]], inlinedAt: ![[INLINEDAT:.*]])
 // CHECK-DAG: ![[INLINEDAT]] = !DILocation({{.*}}scope: ![[INLINEDAT1:[0-9]+]]
-// CHECK-DAG: ![[INLINED1]] = distinct !DILexicalBlock(scope: ![[INLINED2:[0-9]+]]
-// CHECK-DAG: ![[INLINED2]] = distinct !DILexicalBlock(scope: ![[INLINED3:[0-9]+]]
+// CHECK-DAG: ![[INLINED1]] = distinct !DILexicalBlock(scope: ![[INLINED:[0-9]+]]
 // Check if the inlined and removed function still has the correct linkage name.
-// CHECK-DAG: ![[INLINED3]] = distinct !DISubprogram(name: "inlined", linkageName: "$s4main7inlinedys5Int64VADF"
+// CHECK-DAG: ![[INLINED]] = distinct !DISubprogram(name: "inlined", linkageName: "$s4main7inlinedys5Int64VADF"
 // TRANSPARENT-CHECK-NOT: !DISubprogram(name: "transparent"
   return result
 }

test/DebugInfo/let-scope.swift

@@ -0,0 +1,8 @@
+// RUN: %target-swift-frontend -g -emit-sil %s -parse-as-library -module-name a | %FileCheck %s
+func use<T>(_ t: T) {}
+public func f(value: (Int, Int)) {
+  let (x, y) = value
+  // CHECK: debug_value {{.*}}let, name "x", {{.*}}, scope [[LET:[0-9]+]]
+  // CHECK: debug_value {{.*}}let, name "y", {{.*}}, scope [[LET]]
+  use((x,y))
+}

test/DebugInfo/scopes.swift

@@ -17,5 +17,6 @@ public func f(i: Int) {
 // CHECK: ![[S3]] = distinct !DILexicalBlock(scope: ![[S1]],
 // SIL: sil_scope [[S1:[0-9]+]] { {{.*}} parent @$s4main1f1iySi_tF
 // SIL: sil_scope [[S2:[0-9]+]] { {{.*}} parent [[S1]] }
+// SIL: sil_scope [[S3:[0-9]+]] { {{.*}} parent [[S1]] }
 // SIL: debug_value %0 : $Int, let, name "i", argno 1,{{.*}}, scope [[S1]]
-// SIL: debug_value {{.*}} : $Array<Int>, let, name "i", {{.*}}, scope [[S2]]
+// SIL: debug_value {{.*}} : $Array<Int>, let, name "i", {{.*}}, scope [[S3]]

test/DebugInfo/shadowed-arg.swift

@@ -20,8 +20,7 @@ func multiStatementInference() -> Int {
 // The closure intruduced by the macro expansion should not contain any inline
 // locations, but instead point directly into the macro buffer.
 // CHECK-SIL: sil_scope [[S0:[0-9]+]] { loc "@__swiftmacro_9MacroUser23multiStatementInferenceSiyF0cD0fMf_.swift":1:1 parent @$s9MacroUser23multiStatementInferenceSiyFSiyXEfU_
-// CHECK-SIL: sil_scope [[S1:[0-9]+]] { loc "@__swiftmacro_9MacroUser23multiStatementInferenceSiyF0cD0fMf_.swift":2:7 parent [[S0]] }
-// CHECK-SIL: sil_scope [[S2:[0-9]+]] { loc "@__swiftmacro_9MacroUser23multiStatementInferenceSiyF0cD0fMf_.swift":2:14 parent [[S1]] }
+// CHECK-SIL: sil_scope [[S2:[0-9]+]] { loc "@__swiftmacro_9MacroUser23multiStatementInferenceSiyF0cD0fMf_.swift":2:14 parent [[S0]] }
 
 // CHECK-SIL: sil {{.*}} @$s9MacroUser23multiStatementInferenceSiyFSiyXEfU_
 // CHECK-SIL-NOT: return

test/Macros/macro_expand_closure.swift

@@ -19,12 +19,12 @@
 // CHECK:   destroy_value [[BOX_COPY]] : ${ var Int }, loc {{.*}}:33:11, scope 4
 // CHECK:   [[CLOSURE:%[^,]+]] = partial_apply [callee_guaranteed] [[SPECIALIZED_F]]([[REGISTER_11]]) : $@convention(thin) (Int) -> Int, loc {{.*}}:33:11, scope 4
 // CHECK:   [[BORROW:%.*]] = begin_borrow [lexical] [[CLOSURE]]
-// CHECK:   debug_value [[BORROW]] : $@callee_guaranteed () -> Int, let, name "f", loc {{.*}}:33:7, scope 5
-// CHECK:   [[CLOSURE_COPY:%[^,]+]] = copy_value [[BORROW]] : $@callee_guaranteed () -> Int, loc {{.*}}:34:10, scope 5
+// CHECK:   debug_value [[BORROW]] : $@callee_guaranteed () -> Int, let, name "f", loc {{.*}}:33:7, scope 6
+// CHECK:   [[CLOSURE_COPY:%[^,]+]] = copy_value [[BORROW]] : $@callee_guaranteed () -> Int, loc {{.*}}:34:10, scope 6
 // There used to be an end_borrow here. We leave an emptyline here to preserve line numbers.
-// CHECK:   destroy_value [[CLOSURE]] : $@callee_guaranteed () -> Int, loc {{.*}}:35:1, scope 5
-// CHECK:   destroy_value [[BOX]] : ${ var Int }, loc {{.*}}:35:1, scope 5
-// CHECK:   return [[CLOSURE_COPY]] : $@callee_guaranteed () -> Int, loc {{.*}}:34:3, scope 5
+// CHECK:   destroy_value [[CLOSURE]] : $@callee_guaranteed () -> Int, loc {{.*}}:35:1, scope 6
+// CHECK:   destroy_value [[BOX]] : ${ var Int }, loc {{.*}}:35:1, scope 6
+// CHECK:   return [[CLOSURE_COPY]] : $@callee_guaranteed () -> Int, loc {{.*}}:34:3, scope 6
 // CHECK: }