[sil-verifier] Use the scope result rather than the scope instruction when verifying that we end scope instructions.

We do this since we need to be able to handle instructions that may have two
different results that independently need their scopes lifetime to be ended.

Author: gottesmm

lib/SIL/Verifier/SILVerifier.cpp

@@ -7083,8 +7083,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     struct BBState {
       std::vector<SILValue> Stack;
 
-      /// Contents: BeginAccessInst*, BeginApplyInst*.
-      std::set<SILInstruction*> ActiveOps;
+      /// Contents: Results of BeginAccessInst*, BeginApplyInst*.
+      std::set<SILValue> ActiveOps;
 
       CFGState CFG = Normal;
 
@@ -7237,19 +7237,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
         }
       }
 
-      void handleScopeInst(SILInstruction &i) {
-        if (!ActiveOps.insert(&i).second) {
+      void handleScopeInst(SILValue i) {
+        if (!ActiveOps.insert(i).second) {
           verificationFailure("operation was not ended before re-beginning it",
-                              &i, nullptr);
+                              i, nullptr);
         }
       }
 
-      void handleScopeEndingInst(SILInstruction &i) {
-        if (auto beginOp = i.getOperand(0)->getDefiningInstruction()) {
-          if (!ActiveOps.erase(beginOp)) {
-            verificationFailure("operation has already been ended", &i,
-                                nullptr);
-          }
+      void handleScopeEndingInst(const SILInstruction &i) {
+        if (!ActiveOps.erase(i.getOperand(0))) {
+          verificationFailure("operation has already been ended", &i, nullptr);
         }
       }
     };
@@ -7263,31 +7260,40 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     };
   };
 
-  static bool isScopeInst(SILInstruction *i) {
-    if (isa<BeginAccessInst>(i) ||
-        isa<BeginApplyInst>(i) ||
-        isa<StoreBorrowInst>(i)) {
-      return true;
-    } else if (auto bi = dyn_cast<BuiltinInst>(i)) {
+  /// If this is a scope ending inst, return the result from the instruction
+  /// that provides the scoped value whose lifetime must be ended by some other
+  /// scope ending instruction.
+  static SILValue isScopeInst(SILInstruction *i) {
+    if (auto *bai = dyn_cast<BeginAccessInst>(i))
+      return bai;
+    if (auto *bai = dyn_cast<BeginApplyInst>(i))
+      return bai->getTokenResult();
+    if (auto *sbi = dyn_cast<StoreBorrowInst>(i))
+      return sbi;
+
+    if (auto bi = dyn_cast<BuiltinInst>(i)) {
       if (auto bk = bi->getBuiltinKind()) {
         switch (*bk) {
         case BuiltinValueKind::StartAsyncLetWithLocalBuffer:
-          return true;
+          return bi->getResult(0);
 
         default:
-          return false;
+          return SILValue();
         }
       }
     }
-    return false;
+
+    return SILValue();
   }
 
   static bool isScopeEndingInst(SILInstruction *i) {
     if (isa<EndAccessInst>(i) ||
         isa<AbortApplyInst>(i) ||
         isa<EndApplyInst>(i)) {
       return true;
-    } else if (auto bi = dyn_cast<BuiltinInst>(i)) {
+    }
+
+    if (auto bi = dyn_cast<BuiltinInst>(i)) {
       if (auto bk = bi->getBuiltinKind()) {
         switch (*bk) {
         case BuiltinValueKind::FinishAsyncLet:
@@ -7359,10 +7365,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
             state.Stack.push_back(i.getStackAllocation());
 
             // Also track begin_apply as a scope instruction.
-            if (isa<BeginApplyInst>(i))
-              state.handleScopeInst(i);
+            if (auto *bai = dyn_cast<BeginApplyInst>(&i)) {
+              state.handleScopeInst(bai->getStackAllocation());
+              state.handleScopeInst(bai->getTokenResult());
+            }
           } else {
-            state.handleScopeInst(i);
+            state.handleScopeInst(i.getStackAllocation());
           }
         } else if (i.isDeallocatingStack()) {
           SILValue op = i.getOperand(0);
@@ -7388,8 +7396,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                   // failure.
                 });
           }
-        } else if (isScopeInst(&i)) {
-          state.handleScopeInst(i);
+        } else if (auto scopeEndingValue = isScopeInst(&i)) {
+          state.handleScopeInst(scopeEndingValue);
         } else if (isScopeEndingInst(&i)) {
           state.handleScopeEndingInst(i);
         } else if (auto *endBorrow = dyn_cast<EndBorrowInst>(&i)) {

test/SIL/verifier_failures.sil

@@ -89,3 +89,18 @@ failure(%error : @owned $any Error):
   dealloc_stack %allocation : $*Builtin.SILToken
   throw %error : $any Error
 }
+
+sil [ossa] @yield_test : $@yield_once @convention(thin) (@inout Builtin.Int32) -> @yields @inout Builtin.Int32 {
+bb0(%0 : $*Builtin.Int32):
+  (%2, %3) = begin_apply undef(%0) : $@yield_once @convention(method) (@inout Builtin.Int32) -> @yields @inout Builtin.Int32
+  yield %2, resume bb1, unwind bb2
+
+bb1:
+  %5 = end_apply %3 as $()
+  %6 = tuple ()
+  return %6
+
+bb2:
+  abort_apply %3
+  unwind
+}