[region-isolation] Do not treat functions/class_methods that are isolated to a #isolated as being globally isolated to that.

Instead, we need to consider the isolation at the apply site.

rdar://126285681
rdar://125078448

Author: gottesmm

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -186,7 +186,7 @@ class regionanalysisimpl::TrackableValueState {
        << "][is_no_alias: " << (isNoAlias() ? "yes" : "no")
        << "][is_sendable: " << (isSendable() ? "yes" : "no")
        << "][region_value_kind: ";
-    getIsolationRegionInfo().print(os);
+    getIsolationRegionInfo().printForDiagnostics(os);
     os << "].";
   }
 

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -161,6 +161,11 @@ class SILIsolationInfo {
 
   void printForDiagnostics(llvm::raw_ostream &os) const;
 
+  SWIFT_DEBUG_DUMPER(dumpForDiagnostics()) {
+    printForDiagnostics(llvm::dbgs());
+    llvm::dbgs() << '\n';
+  }
+
   ActorIsolation getActorIsolation() const {
     assert(kind == Actor);
     return actorIsolation;

lib/SILOptimizer/Utils/PartitionUtils.cpp

@@ -21,6 +21,8 @@
 
 #include "llvm/Support/CommandLine.h"
 
+#pragma clang optimize off
+
 using namespace swift;
 using namespace swift::PatternMatch;
 using namespace swift::PartitionPrimitives;
@@ -71,14 +73,6 @@ getGlobalActorInitIsolation(SILFunction *fn) {
 }
 
 SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
-  if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>()) {
-    if (auto crossing = apply->getIsolationCrossing()) {
-      if (crossing->getCalleeIsolation().isActorIsolated())
-        return SILIsolationInfo::getActorIsolated(
-            SILValue(), SILValue(), crossing->getCalleeIsolation());
-    }
-  }
-
   if (auto fas = FullApplySite::isa(inst)) {
     if (auto crossing = fas.getIsolationCrossing()) {
       if (crossing->getCalleeIsolation().isActorIsolated()) {
@@ -149,7 +143,9 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
   // Treat function ref as either actor isolated or sendable.
   if (auto *fri = dyn_cast<FunctionRefInst>(inst)) {
     auto isolation = fri->getReferencedFunction()->getActorIsolation();
-    if (isolation.isActorIsolated()) {
+    if (isolation.isActorIsolated() &&
+        (isolation.getKind() != ActorIsolation::ActorInstance ||
+         isolation.getActorInstanceParameter() == 0)) {
       return SILIsolationInfo::getActorIsolated(fri, SILValue(), isolation);
     }
 
@@ -178,11 +174,15 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
   }
 
   if (auto *cmi = dyn_cast<ClassMethodInst>(inst)) {
+    // Ok, we know that we do not have an actor... but we might have a global
+    // actor isolated method. Use the AST to compute the actor isolation and
+    // check if we are self. If we are not self, we want this to be
+    // disconnected.
     if (auto *declRefExpr = cmi->getLoc().getAsASTNode<DeclRefExpr>()) {
-      // See if we are actor isolated. If so, treat this as non-Sendable so we
-      // propagate actor isolation.
       if (auto isolation = swift::getActorIsolation(declRefExpr->getDecl())) {
-        if (isolation.isActorIsolated()) {
+        if (isolation.isActorIsolated() &&
+            (isolation.getKind() != ActorIsolation::ActorInstance ||
+             isolation.getActorInstanceParameter() == 0)) {
           auto actor = cmi->getOperand()->getType().isActor()
                            ? cmi->getOperand()
                            : SILValue();
@@ -264,6 +264,16 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
     }
   }
 
+  // Try to infer using SIL first since we might be able to get the source name
+  // of the actor.
+  if (ApplyExpr *apply = inst->getLoc().getAsASTNode<ApplyExpr>()) {
+    if (auto crossing = apply->getIsolationCrossing()) {
+      if (crossing->getCalleeIsolation().isActorIsolated())
+        return SILIsolationInfo::getActorIsolated(
+            SILValue(), SILValue(), crossing->getCalleeIsolation());
+    }
+  }
+
   return SILIsolationInfo();
 }
 

test/Concurrency/isolated_parameters.swift

@@ -3,7 +3,6 @@
 
 // REQUIRES: concurrency
 // REQUIRES: swift_swift_parser
-// REQUIRES: rdar126006489
 
 @available(SwiftStdlib 5.1, *)
 actor A {

test/Concurrency/transfernonsendable.swift

@@ -15,17 +15,19 @@
 ////////////////////////
 
 /// Classes are always non-sendable, so this is non-sendable
-class NonSendableKlass { // expected-complete-note 41{{}}
+class NonSendableKlass { // expected-complete-note 45{{}}
   // expected-typechecker-only-note @-1 4{{}}
   // expected-tns-note @-2 2{{}}
   var field: NonSendableKlass? = nil
 
   func asyncCall() async {}
+  func asyncCallWithIsolatedParameter(isolation: isolated (any Actor)? = #isolation) async {
+  }
 }
 
 class SendableKlass : @unchecked Sendable {}
 
-actor Actor {
+actor MyActor {
   var klass = NonSendableKlass()
   // expected-typechecker-only-note @-1 7{{property declared here}}
   final var finalKlass = NonSendableKlass()
@@ -79,7 +81,7 @@ class TwoFieldKlassClassBox {
 // MARK: Actor Self Tests //
 ////////////////////////////
 
-extension Actor {
+extension MyActor {
   func warningIfCallingGetter() async {
     await self.klass.asyncCall() // expected-complete-warning {{passing argument of non-sendable type 'NonSendableKlass' outside of actor-isolated context may introduce data races}}
     // expected-tns-warning @-1 {{transferring 'self.klass' may cause a data race}}
@@ -120,7 +122,7 @@ func formClosureWithoutCrashing() {
 
 // In this test, closure is not promoted into its box form. As a result, we
 // treat assignments into contents as a merge operation rather than an assign.
-func closureInOut(_ a: Actor) async {
+func closureInOut(_ a: MyActor) async {
   var contents = NonSendableKlass()
   let ns0 = NonSendableKlass()
   let ns1 = NonSendableKlass()
@@ -146,7 +148,7 @@ func closureInOut(_ a: Actor) async {
   }
 }
 
-func closureInOutDifferentActor(_ a: Actor, _ a2: Actor) async {
+func closureInOutDifferentActor(_ a: MyActor, _ a2: MyActor) async {
   var contents = NonSendableKlass()
   let ns0 = NonSendableKlass()
   let ns1 = NonSendableKlass()
@@ -163,18 +165,17 @@ func closureInOutDifferentActor(_ a: Actor, _ a2: Actor) async {
   // expected-tns-note @-3 {{transferring disconnected 'ns0' to actor-isolated callee could cause races in between callee actor-isolated and local nonisolated uses}}
 
   // We only emit a warning on the first use we see, so make sure we do both
-  // the print and the closure.
+  // the use and the closure.
   if await booleanFlag {
-    // This is an actual use since a2 is a different actor from a1
-    await a2.useKlass(ns1)
+    await a2.useKlass(ns1) // expected-tns-note {{use here could race}}
     // expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass'}}
   } else {
     closure() // expected-tns-note {{use here could race}}
   }
 }
 
 
-func closureInOut2(_ a: Actor) async {
+func closureInOut2(_ a: MyActor) async {
   var contents = NonSendableKlass()
   let ns0 = NonSendableKlass()
   let ns1 = NonSendableKlass()
@@ -197,7 +198,7 @@ func closureInOut2(_ a: Actor) async {
   closure() // expected-tns-note {{use here could race}}
 }
 
-func closureNonInOut(_ a: Actor) async {
+func closureNonInOut(_ a: MyActor) async {
   var contents = NonSendableKlass()
   let ns0 = NonSendableKlass()
   let ns1 = NonSendableKlass()
@@ -224,7 +225,7 @@ func closureNonInOut(_ a: Actor) async {
 // method, we cannot access the inside of the actor without using an await, so
 // this is safe.
 func transferNonIsolatedNonAsyncClosureTwice() async {
-  let a = Actor()
+  let a = MyActor()
 
   // This is non-isolated and non-async... we can transfer it safely.
   var actorCaptureClosure = { print(a) }
@@ -236,7 +237,7 @@ func transferNonIsolatedNonAsyncClosureTwice() async {
   // expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 }
 
-extension Actor {
+extension MyActor {
   // Simple just capture self and access field.
   func simpleClosureCaptureSelfAndTransfer() async {
     let closure: () -> () = {
@@ -428,7 +429,7 @@ extension Actor {
 }
 
 func testSimpleLetClosureCaptureActor() async {
-  let a = Actor()
+  let a = MyActor()
   let closure = { print(a) }
   await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
   // expected-complete-note @-1 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
@@ -437,23 +438,23 @@ func testSimpleLetClosureCaptureActor() async {
 #if TYPECHECKER_ONLY
 
 func testSimpleLetClosureCaptureActorField() async {
-  let a = Actor()
+  let a = MyActor()
   let closure = { print(a.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
   // expected-complete-warning @-1 {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
   // expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 }
 
 func testSimpleLetClosureCaptureActorFieldThroughTuple() async {
-  let a = (Actor(), 0)
+  let a = (MyActor(), 0)
   let closure = { print(a.0.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
   // expected-complete-warning @-1 {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
   // expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
 }
 
 func testSimpleLetClosureCaptureActorFieldThroughOptional() async {
-  let a: Actor? = Actor()
+  let a: MyActor? = MyActor()
   let closure = { print(a!.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
   // expected-complete-warning @-1 {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
@@ -463,7 +464,7 @@ func testSimpleLetClosureCaptureActorFieldThroughOptional() async {
 #endif
 
 func testSimpleVarClosureCaptureActor() async {
-  let a = Actor()
+  let a = MyActor()
   var closure = {}
   closure = { print(a) }
   await transferToMain(closure) // expected-complete-warning {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
@@ -473,7 +474,7 @@ func testSimpleVarClosureCaptureActor() async {
 #if TYPECHECKER_ONLY
 
 func testSimpleVarClosureCaptureActorField() async {
-  let a = Actor()
+  let a = MyActor()
   var closure = {}
   closure = { print(a.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
@@ -482,7 +483,7 @@ func testSimpleVarClosureCaptureActorField() async {
 }
 
 func testSimpleVarClosureCaptureActorFieldThroughTuple() async {
-  let a = (Actor(), 0)
+  let a = (MyActor(), 0)
   var closure = {}
   closure = { print(a.0.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
@@ -491,7 +492,7 @@ func testSimpleVarClosureCaptureActorFieldThroughTuple() async {
 }
 
 func testSimpleVarClosureCaptureActorFieldThroughOptional() async {
-  let a: Actor? = Actor()
+  let a: MyActor? = MyActor()
   var closure = {}
   closure = { print(a!.klass) } // expected-typechecker-only-error {{actor-isolated property 'klass' can not be referenced from a non-isolated context}}
   await transferToMain(closure)
@@ -501,7 +502,7 @@ func testSimpleVarClosureCaptureActorFieldThroughOptional() async {
 
 #endif
 
-extension Actor {
+extension MyActor {
   // Make sure that we properly propagate actor derived from klass into field's
   // value.
   func simplePropagateNonSendableThroughMultipleProperty() async {
@@ -529,7 +530,7 @@ extension Actor {
   }
 }
 
-extension Actor {
+extension MyActor {
   func testVarReassignStopActorDerived() async {
     var closure: () -> () = {
       print(self)
@@ -566,7 +567,7 @@ extension Actor {
 
 // Make sure that we do not error on function values that are Sendable... even
 // if the function is converted to a non-Sendable form by a function conversion.
-func testConversionsAndSendable(a: Actor, f: @Sendable () -> Void, f2: () -> Void) async {
+func testConversionsAndSendable(a: MyActor, f: @Sendable () -> Void, f2: () -> Void) async {
   // No function conversion.
   await a.useSendableFunction(f)
 
@@ -580,7 +581,7 @@ func testConversionsAndSendable(a: Actor, f: @Sendable () -> Void, f2: () -> Voi
   // expected-tns-note @-3 {{transferring task-isolated 'f2' to actor-isolated callee could cause races between actor-isolated and task-isolated uses}}
 }
 
-func testSendableClosureCapturesNonSendable(a: Actor) {
+func testSendableClosureCapturesNonSendable(a: MyActor) {
   let klass = NonSendableKlass()
   let _ = { @Sendable in
     _ = klass // expected-warning {{capture of 'klass' with non-sendable type 'NonSendableKlass' in a `@Sendable` closure}}
@@ -1572,10 +1573,47 @@ func functionArgumentIntoClosure(_ x: @escaping () -> ()) async {
 }
 
 func testGetActorName() async {
-  let a = Actor()
+  let a = MyActor()
   let x = NonSendableKlass()
   await a.useKlass(x) // expected-tns-warning {{transferring 'x' may cause a data race}}
   // expected-tns-note @-1 {{transferring disconnected 'x' to actor-isolated callee could cause races in between callee actor-isolated and local nonisolated uses}}
   // expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
   useValue(x) // expected-tns-note {{use here could race}}
 }
+
+extension MyActor {
+  func testCallBangIsolatedMethod(other: MyActor) async {
+    await klass.asyncCallWithIsolatedParameter(isolation: other) // expected-tns-warning {{transferring 'self.klass' may cause a data race}}
+    // expected-tns-note @-1 {{transferring 'self'-isolated 'self.klass' to actor-isolated callee could cause races between actor-isolated and 'self'-isolated uses}}
+    // expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
+  }
+}
+
+extension FinalActor {
+  func testCallBangIsolatedMethod(other: MyActor) async {
+    await klass.asyncCallWithIsolatedParameter(isolation: other) // expected-tns-warning {{transferring 'self.klass' may cause a data race}}
+    // expected-tns-note @-1 {{transferring 'self'-isolated 'self.klass' to actor-isolated callee could cause races between actor-isolated and 'self'-isolated uses}}
+    // expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
+  }
+}
+
+extension NonSendableKlass {
+  func directAsyncCallWithIsolatedParameter(isolation: isolated (any Actor)? = #isolation) async {
+  }
+}
+
+extension MyActor {
+  func testCallBangIsolatedDirectMethod(other: MyActor) async {
+    await klass.directAsyncCallWithIsolatedParameter(isolation: other) // expected-tns-warning {{transferring 'self.klass' may cause a data race}}
+    // expected-tns-note @-1 {{transferring 'self'-isolated 'self.klass' to actor-isolated callee could cause races between actor-isolated and 'self'-isolated uses}}
+    // expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
+  }
+}
+
+extension FinalActor {
+  func testCallBangIsolatedDirectMethod(other: MyActor) async {
+    await klass.directAsyncCallWithIsolatedParameter(isolation: other) // expected-tns-warning {{transferring 'self.klass' may cause a data race}}
+    // expected-tns-note @-1 {{transferring 'self'-isolated 'self.klass' to actor-isolated callee could cause races between actor-isolated and 'self'-isolated uses}}
+    // expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
+  }
+}