Merge pull request #35332 from slavapestov/debug-scope-guard-stmt

SILGen: Introduce a new debug scope for GuardStmts

Author: slavapestov

lib/SILGen/SILGenFunction.h

@@ -329,7 +329,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   std::vector<BreakContinueDest> BreakContinueDestStack;
   std::vector<PatternMatchContext*> SwitchStack;
   /// Keep track of our current nested scope.
-  std::vector<const SILDebugScope *> DebugScopeStack;
+  ///
+  /// The boolean tracks whether this is a 'guard' scope, which should be
+  /// popped automatically when we leave the innermost BraceStmt scope.
+  std::vector<llvm::PointerIntPair<const SILDebugScope *, 1>> DebugScopeStack;
 
   /// The cleanup depth and BB for when the operand of a
   /// BindOptionalExpr is a missing value.
@@ -590,23 +593,32 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   StringRef getMagicFunctionString();
 
   /// Enter the debug scope for \p Loc, creating it if necessary.
-  void enterDebugScope(SILLocation Loc) {
+  ///
+  /// \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() : F.getDebugScope();
+        DebugScopeStack.size() ? DebugScopeStack.back().getPointer() : F.getDebugScope();
     auto *DS = Parent;
     // Don't nest a scope for Loc under Parent unless it's actually different.
-    if (Parent->getLoc().getAsRegularLocation() != Loc.getAsRegularLocation())
-      DS = DS = new (SGM.M)
-          SILDebugScope(Loc.getAsRegularLocation(), &getFunction(), Parent);
-    DebugScopeStack.push_back(DS);
+    if (DS->getLoc().getAsRegularLocation() != Loc.getAsRegularLocation()) {
+      DS = new (SGM.M)
+          SILDebugScope(Loc.getAsRegularLocation(), &getFunction(), DS);
+    }
+    DebugScopeStack.emplace_back(DS, isGuardScope);
     B.setCurrentDebugScope(DS);
   }
 
   /// Return to the previous debug scope.
   void leaveDebugScope() {
+    // Pop any 'guard' scopes first.
+    while (DebugScopeStack.back().getInt())
+      DebugScopeStack.pop_back();
+
+    // Pop the scope we're leaving now.
     DebugScopeStack.pop_back();
     if (DebugScopeStack.size())
-      B.setCurrentDebugScope(DebugScopeStack.back());
+      B.setCurrentDebugScope(DebugScopeStack.back().getPointer());
     // Don't reset the debug scope after leaving the outermost scope,
     // because the debugger is not expecting the function epilogue to
     // be in a different scope.

lib/SILGen/SILGenStmt.cpp

@@ -724,11 +724,14 @@ void StmtEmitter::visitGuardStmt(GuardStmt *S) {
       SGF.B.createUnreachable(S);
   }
 
-  // Emit the condition bindings, branching to the bodyBB if they fail.  Since
-  // we didn't push a scope, the bound variables are live after this statement.
+  // Emit the condition bindings, branching to the bodyBB if they fail.
   auto NumFalseTaken = SGF.loadProfilerCount(S->getBody());
   auto NumNonTaken = SGF.loadProfilerCount(S);
   SGF.emitStmtCondition(S->getCond(), bodyBB, S, NumNonTaken, NumFalseTaken);
+
+  // Begin a new 'guard' scope, which is popped when the next innermost debug
+  // scope ends.
+  SGF.enterDebugScope(S, /*isGuardScope=*/true);
 }
 
 void StmtEmitter::visitWhileStmt(WhileStmt *S) {

test/DebugInfo/guard-let.swift

@@ -2,9 +2,19 @@
 // RUN:   %FileCheck %s --check-prefix=CHECK1
 // RUN: %target-swift-frontend %s -c -emit-ir -g -o - | \
 // RUN:   %FileCheck %s --check-prefix=CHECK2
+// RUN: %target-swift-frontend %s -c -emit-ir -g -o - | \
+// RUN:   %FileCheck %s --check-prefix=CHECK3
 
 // UNSUPPORTED: OS=watchos
 
+// With large type optimizations the string is passed indirectly on the
+// following architectures so there is no shadow copy happening. As this
+// tests that we're emitting the DI correctly, we can skip running on them.
+// UNSUPPORTED: CPU=i386
+// UNSUPPORTED: CPU=armv7
+// UNSUPPORTED: CPU=armv7s
+// UNSUPPORTED: CPU=armv7k
+
 func use<T>(_ t: T) {}
 
 public func f(_ i : Int?)
@@ -25,14 +35,6 @@ public func f(_ i : Int?)
   use(val)
 }
 
-// With large type optimizations the string is passed indirectly on the
-// following architectures so there is no shadow copy happening. As this
-// tests that we're emitting the DI correctly, we can skip running on them.
-// UNSUPPORTED: CPU=i386
-// UNSUPPORTED: CPU=armv7
-// UNSUPPORTED: CPU=armv7s
-// UNSUPPORTED: CPU=armv7k
-
 public func g(_ s : String?)
 {
   // CHECK2: define {{.*}}@"$s4main1gyySSSgF"
@@ -46,3 +48,32 @@ public func g(_ s : String?)
   guard let val = s else { return }
   use(val)
 }
+
+public func h(_ s : String?)
+{
+  // CHECK3: define {{.*}}@"$s4main1hyySSSgF"
+  // CHECK3: %s.debug = alloca %TSSSg
+  // CHECK3: @llvm.dbg.declare(metadata %TSSSg*
+  // CHECK3: %s.debug1 = alloca %TSS
+  // CHECK3: @llvm.dbg.declare(metadata %TSS*
+  // CHECK3: %[[BITCAST:.*]] = bitcast %TSS* %s.debug1 to i8*{{$}}
+  // CHECK3: call void @llvm.memset.{{.*}}(i8* align {{(4|8)}} %[[BITCAST]], i8 0
+  // CHECK3: ![[G:.*]] = distinct !DISubprogram(name: "h"
+  guard let s = s else { return }
+  use(s)
+}
+
+enum MyError : Error {
+  case bad
+}
+
+enum Stuff {
+  case array([Stuff])
+  case any(Any)
+  case nothing
+
+  func toArray() throws -> [Stuff] {
+    guard case .array(let array) = self else { throw MyError.bad }
+    return array
+  }
+}

test/SILOptimizer/definite-init-wrongscope.swift

@@ -28,14 +28,16 @@ public class M {
 
 // Make sure the expanded sequence gets the right scope.
 
-// CHECK:   [[I:%.*]] = integer_literal $Builtin.Int2, 1, loc {{.*}}:20:12, scope 2
-// CHECK:   [[V:%.*]] = load [trivial] %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 2
-// CHECK:   [[OR:%.*]] = builtin "or_Int2"([[V]] : $Builtin.Int2, [[I]] : $Builtin.Int2) : $Builtin.Int2, loc {{.*}}:20:12, scope 2
-// CHECK:   store [[OR]] to [trivial] %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 2
-// CHECK:   store %{{.*}} to [init] %{{.*}} : $*C, loc {{.*}}:23:20, scope 2
+// 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 {{.*}}:20:12, scope 4
+// CHECK:   [[V:%.*]] = load [trivial] %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 4
+// CHECK:   [[OR:%.*]] = builtin "or_Int2"([[V]] : $Builtin.Int2, [[I]] : $Builtin.Int2) : $Builtin.Int2, loc {{.*}}:20:12, scope 4
+// CHECK:   store [[OR]] to [trivial] %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 4
+// CHECK:   store %{{.*}} to [init] %{{.*}} : $*C, loc {{.*}}:23:20, scope 4
 
 // Make sure the dealloc_stack gets the same scope of the instructions surrounding it.
 
-// CHECK:   destroy_addr %0 : $*WithDelegate, loc {{.*}}:26:5, scope 2
-// CHECK:   dealloc_stack %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 2
-// CHECK:   throw %{{.*}} : $Error, loc {{.*}}:20:12, scope 2
+// CHECK:   destroy_addr %0 : $*WithDelegate, loc {{.*}}:26:5, scope 4
+// CHECK:   dealloc_stack %2 : $*Builtin.Int2, loc {{.*}}:20:12, scope 4
+// CHECK:   throw %{{.*}} : $Error, loc {{.*}}:20:12, scope 4