Merge pull request #80911 from gottesmm/pr-52ba504b4ba7185b4e3f34ed03b36721f4ad3d82

[rbi] Simplify some logic that got confused so that passing an actor isolated value to a callee that is isolated ot the same actor is not considered a send.

Author: gottesmm

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1438,15 +1438,18 @@ struct PartitionOpEvaluator {
       }
       std::tie(sentRegionIsolation, regionHasClosureCapturedElt) = *pairOpt;
 
-      // If we merged anything, we need to handle an attempt to send a
-      // never-sent value unless our value has the same isolation info as our
-      // callee.
       auto calleeIsolationInfo = getIsolationInfo(op);
-      if (!(calleeIsolationInfo &&
-            sentRegionIsolation.hasSameIsolation(calleeIsolationInfo)) &&
-          !sentRegionIsolation.isDisconnected()) {
-        return handleSendNeverSentHelper(op, op.getOpArg1(),
-                                         sentRegionIsolation);
+
+      // If our callee and region are both actor isolated and part of the same
+      // isolation domain, do not treat this as a send.
+      if (calleeIsolationInfo.isActorIsolated() &&
+          sentRegionIsolation.hasSameIsolation(calleeIsolationInfo))
+        return;
+
+      // At this point, check if our sent value is not disconnected. If so, emit
+      // a sent never sendable helper.
+      if (sentRegionIsolation && !sentRegionIsolation.isDisconnected()) {
+        return handleSendNeverSentHelper(op, op.getOpArg1(), sentRegionIsolation);
       }
 
       // Next see if we are disconnected and have the same isolation. In such a

test/Concurrency/concurrent_value_checking.swift

@@ -141,9 +141,8 @@ func globalTest() async {
   // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
   // expected-note@+1 {{property access is 'async'}}
   let a = globalValue // expected-warning{{non-sendable type 'NotConcurrent?' of let 'globalValue' cannot exit global actor 'SomeGlobalActor'-isolated context}}
-  await globalAsync(a) // expected-tns-warning {{sending 'a' risks causing data races}}
-  // expected-tns-note @-1 {{sending global actor 'SomeGlobalActor'-isolated 'a' to global actor 'SomeGlobalActor'-isolated global function 'globalAsync' risks causing data races between global actor 'SomeGlobalActor'-isolated and local nonisolated uses}}
-  await globalSync(a) // expected-tns-note {{access can happen concurrently}}
+  await globalAsync(a)
+  await globalSync(a)
 
   // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
   // expected-note@+1 {{property access is 'async'}}
@@ -178,9 +177,8 @@ func globalTestMain(nc: NotConcurrent) async {
   // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
   // expected-note@+1 {{property access is 'async'}}
   let a = globalValue // expected-warning {{non-sendable type 'NotConcurrent?' of let 'globalValue' cannot exit global actor 'SomeGlobalActor'-isolated context}}
-  await globalAsync(a) // expected-tns-warning {{sending 'a' risks causing data races}}
-  // expected-tns-note @-1 {{sending global actor 'SomeGlobalActor'-isolated 'a' to global actor 'SomeGlobalActor'-isolated global function 'globalAsync' risks causing data races between global actor 'SomeGlobalActor'-isolated and local main actor-isolated uses}}
-  await globalSync(a) // expected-tns-note {{access can happen concurrently}}
+  await globalAsync(a)
+  await globalSync(a)
   _ = await ClassWithGlobalActorInits(nc)
   // expected-tns-warning @-1 {{non-Sendable 'ClassWithGlobalActorInits'-typed result can not be returned from global actor 'SomeGlobalActor'-isolated initializer 'init(_:)' to main actor-isolated context}}
   // expected-tns-warning @-2 {{sending 'nc' risks causing data races}}

test/Concurrency/transfernonsendable.sil

@@ -158,13 +158,14 @@ bb0:
   %f = function_ref @transferIndirectWithOutResult : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
   apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f<NonSendableKlass>(%1, %0) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 // expected-warning {{}}
 
+  // We do not error here on use after send since %1 is already isolated to the
+  // same global actor as transferIndirect which is not considered to be a
+  // send. We leave the actual error to be the error about returning an isolated
+  // value into a non-isolated context.
   %f2 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
   apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f2<NonSendableKlass>(%1) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
-  // expected-warning @-1 {{}}
-  // expected-note @-2 {{}}
   %useIndirect = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
   apply %useIndirect<NonSendableKlass>(%1) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
-  // expected-note @-1 {{access can happen concurrently}}
 
   destroy_addr %1 : $*NonSendableKlass
   dealloc_stack %1 : $*NonSendableKlass

test/Concurrency/transfernonsendable.swift

@@ -2027,3 +2027,30 @@ func testIsolatedParamInference() {
     }
   }
 }
+
+// We shouldn't error here since test2 is isolated to B since we are capturing
+// self and funcParam is also exposed to B's isolated since it is a parameter to
+// one of B's methods.
+func sendIsolatedValueToItsOwnIsolationDomain() {
+  class A {
+    func useValue() {}
+  }
+
+  func helper(_ x: @escaping () -> A) {}
+
+  actor B {
+    let field = A()
+
+    private func test(funcParam: A?) async {
+      helper {
+        if let funcParam {
+          return funcParam
+        } else {
+          return self.field
+        }
+      }
+
+      funcParam?.useValue()
+    }
+  }
+}