[region-isolation] Fix return vs break logic error in the evaluator.

I am not sure if this would ever actually ever cause a bug... but we should be
consistent. The issue here is that in the evaluator switch when processing
transfers in certain cases when we emitted an early error due to an actor
derived value we were performing a transfer and other times we weren't. With
this patch:

1. We now consistently do not actually transfer the value if it is actor
derived. It is always illegal to transfer an actor derived value... so we know
that if we always just error on transferring it, we have a safe model.

2. The switch we breaking so that we could do an assert that we canonicalized
the modified PartitionOp. Rather than do that and allow for people to
potentially return, I moved the assert into a SWIFT_DEFER and added an assert in
the continuation of the switch. This means that if an author ever breaks out of
the switch, they will hit the assert implying that the author in all cases must
explicitly return within the case statement guaranteeing this mistake does not
happen again.

Author: gottesmm

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -810,6 +810,7 @@ struct PartitionOpEvaluator {
         REGIONBASEDISOLATION_VERBOSE_LOG(llvm::dbgs() << "    After:  ";
                                          p.print(llvm::dbgs()));
       }
+      assert(p.is_canonical_correct());
     };
 
     switch (op.getKind()) {
@@ -829,13 +830,13 @@ struct PartitionOpEvaluator {
       // assignment could have invalidated canonicality of either the old region
       // of op.getOpArgs()[0] or the region of op.getOpArgs()[1], or both
       p.canonical = false;
-      break;
+      return;
     case PartitionOpKind::AssignFresh:
       assert(op.getOpArgs().size() == 1 &&
              "AssignFresh PartitionOp should be passed 1 argument");
 
       p.addElement(op.getOpArgs()[0]);
-      break;
+      return;
     case PartitionOpKind::Transfer: {
       assert(op.getOpArgs().size() == 1 &&
              "Transfer PartitionOp should be passed 1 argument");
@@ -853,8 +854,7 @@ struct PartitionOpEvaluator {
         if (!p.isTransferred(nonTransferrable) &&
             p.elementToRegionMap.at(nonTransferrable) ==
                 p.elementToRegionMap.at(op.getOpArgs()[0])) {
-          handleTransferNonTransferrable(op, nonTransferrable);
-          break;
+          return handleTransferNonTransferrable(op, nonTransferrable);
         }
       }
 
@@ -874,7 +874,7 @@ struct PartitionOpEvaluator {
 
       // Mark op.getOpArgs()[0] as transferred.
       p.markTransferred(op.getOpArgs()[0], op.getSourceOp());
-      break;
+      return;
     }
     case PartitionOpKind::UndoTransfer: {
       assert(op.getOpArgs().size() == 1 &&
@@ -884,7 +884,7 @@ struct PartitionOpEvaluator {
 
       // Mark op.getOpArgs()[0] as not transferred.
       p.undoTransfer(op.getOpArgs()[0]);
-      break;
+      return;
     }
     case PartitionOpKind::Merge:
       assert(op.getOpArgs().size() == 2 &&
@@ -900,17 +900,18 @@ struct PartitionOpEvaluator {
         handleFailure(op, op.getOpArgs()[1], transferringInst);
 
       p.merge(op.getOpArgs()[0], op.getOpArgs()[1]);
-      break;
+      return;
     case PartitionOpKind::Require:
       assert(op.getOpArgs().size() == 1 &&
              "Require PartitionOp should be passed 1 argument");
       assert(p.elementToRegionMap.count(op.getOpArgs()[0]) &&
              "Require PartitionOp's argument should already be tracked");
       if (auto *transferringInst = p.getTransferred(op.getOpArgs()[0]))
         handleFailure(op, op.getOpArgs()[0], transferringInst);
+      return;
     }
 
-    assert(p.is_canonical_correct());
+    llvm_unreachable("Covered switch isn't covered?!");
   }
 
   void apply(std::initializer_list<PartitionOp> ops) {

test/Concurrency/sendable_checking.swift

@@ -251,12 +251,11 @@ final class NonSendable {
   func call() async {
     await update()
     // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
-    // expected-tns-warning@-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
-    // expected-tns-warning@-3 {{passing argument of non-sendable type 'NonSendable' from nonisolated context to main actor-isolated context at this call site could yield a race with accesses later in this function}}
+    // expected-tns-warning @-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
 
     await self.update()
     // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
-    // expected-tns-note @-2 {{access here could race}}
+    // expected-tns-warning @-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
 
     _ = await x
     // expected-warning@-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
@@ -292,14 +291,14 @@ func callNonisolatedAsyncClosure(
   g: (NonSendable) async -> Void
 ) async {
   await g(ns)
-  // expected-targeted-and-complete-warning@-1 {{passing argument of non-sendable type 'NonSendable' outside of main actor-isolated context may introduce data races}}
-  // expected-tns-warning@-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
-  // expected-tns-warning@-3 {{passing argument of non-sendable type 'NonSendable' from main actor-isolated context to nonisolated context at this call site could yield a race with accesses later in this function}}
+  // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' outside of main actor-isolated context may introduce data races}}
+  // expected-tns-warning @-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
+  // expected-tns-warning @-3 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
 
   let f: (NonSendable) async -> () = globalSendable // okay
   await f(ns)
   // expected-targeted-and-complete-warning@-1 {{passing argument of non-sendable type 'NonSendable' outside of main actor-isolated context may introduce data races}}
-  // expected-tns-note@-2 {{access here could race}}
+  // expected-tns-warning @-2 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
 
 }