Merge pull request #71536 from hborla/isolated-closure-capture

[Concurrency] Don't allow non-`Sendable`, isolated closures to capture non-`Sendable` values that may be isolated to a different actor.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5436,6 +5436,10 @@ ERROR(non_sendable_capture,none,
       "capture of %1 with non-sendable type %0 in a `@Sendable` "
       "%select{local function|closure}2",
       (Type, DeclName, bool))
+ERROR(non_sendable_isolated_capture,none,
+      "capture of %1 with non-sendable type %0 in an isolated "
+      "%select{local function|closure}2",
+      (Type, DeclName, bool))
 ERROR(implicit_async_let_non_sendable_capture,none,
       "capture of %1 with non-sendable type %0 in 'async let' binding",
       (Type, DeclName))

lib/Sema/TypeCheckConcurrency.cpp

@@ -2460,6 +2460,7 @@ namespace {
         // which the declaration occurred, it's okay.
         auto decl = capture.getDecl();
         auto *context = localFunc.getAsDeclContext();
+        auto fnType = localFunc.getType()->getAs<AnyFunctionType>();
         if (!mayExecuteConcurrentlyWith(context, decl->getDeclContext()))
           continue;
 
@@ -2492,11 +2493,19 @@ namespace {
                                      diag::implicit_non_sendable_capture,
                                      decl->getName());
           }
+        } else if (fnType->isSendable()) {
+          diagnoseNonSendableTypes(type, getDeclContext(),
+                                   /*inDerivedConformance*/Type(),
+                                   capture.getLoc(),
+                                   diag::non_sendable_capture,
+                                   decl->getName(),
+                                   /*closure=*/closure != nullptr);
         } else {
           diagnoseNonSendableTypes(type, getDeclContext(),
                                    /*inDerivedConformance*/Type(),
                                    capture.getLoc(),
-                                   diag::non_sendable_capture, decl->getName(),
+                                   diag::non_sendable_isolated_capture,
+                                   decl->getName(),
                                    /*closure=*/closure != nullptr);
         }
       }
@@ -4026,15 +4035,30 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
   if (useContext == defContext)
     return false;
 
-  // If both contexts are isolated to the same actor, then they will not
-  // execute concurrently.
+  bool isolatedStateMayEscape = false;
+
   auto useIsolation = getActorIsolationOfContext(
       const_cast<DeclContext *>(useContext), getClosureActorIsolation);
   if (useIsolation.isActorIsolated()) {
     auto defIsolation = getActorIsolationOfContext(
         const_cast<DeclContext *>(defContext), getClosureActorIsolation);
+    // If both contexts are isolated to the same actor, then they will not
+    // execute concurrently.
     if (useIsolation == defIsolation)
       return false;
+
+    // If the local function is not Sendable, its isolation differs
+    // from that of the context, and both contexts are actor isolated,
+    // then capturing non-Sendable values allows the closure to stash
+    // those values into actor isolated state. The original context
+    // may also stash those values into isolated state, enabling concurrent
+    // access later on.
+    auto &ctx = useContext->getASTContext();
+    bool regionIsolationEnabled =
+        ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation);
+    isolatedStateMayEscape =
+        (!regionIsolationEnabled &&
+        useIsolation.isActorIsolated() && defIsolation.isActorIsolated());
   }
 
   // Walk the context chain from the use to the definition.
@@ -4043,13 +4067,19 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
     if (auto closure = dyn_cast<AbstractClosureExpr>(useContext)) {
       if (isSendableClosure(closure, /*forActorIsolation=*/false))
         return true;
+
+      if (isolatedStateMayEscape)
+        return true;
     }
 
     if (auto func = dyn_cast<FuncDecl>(useContext)) {
       if (func->isLocalCapture()) {
         // If the function is @Sendable... it can be run concurrently.
         if (func->isSendable())
           return true;
+
+        if (isolatedStateMayEscape)
+          return true;
       }
     }
 

test/Concurrency/isolated_captures.swift

@@ -0,0 +1,98 @@
+// RUN: %target-swift-frontend -verify -disable-availability-checking -strict-concurrency=complete -verify-additional-prefix complete- -emit-sil -o /dev/null %s
+// RUN: %target-swift-frontend -verify -disable-availability-checking -strict-concurrency=complete -verify-additional-prefix region-isolation- -emit-sil -o /dev/null %s -enable-experimental-feature RegionBasedIsolation
+
+// REQUIRES: concurrency
+// REQUIRES: asserts
+
+@globalActor
+actor MyActor {
+  static let shared = MyActor()
+  @MyActor static var ns: NotSendable?
+  @MyActor static func ohNo() { ns!.x += 1 }
+}
+
+@globalActor
+actor YourActor {
+  static let shared = YourActor()
+  @YourActor static var ns: NotSendable?
+  @YourActor static func ohNo() { ns!.x += 1 }
+}
+
+// expected-complete-note@+1 3{{class 'NotSendable' does not conform to the 'Sendable' protocol}}
+class NotSendable {
+  var x: Int = 0
+
+  @MyActor init() {
+    MyActor.ns = self
+  }
+
+  init(x: Int) {
+    self.x = x
+  }
+
+  @MyActor func stash() {
+    MyActor.ns = self
+  }
+}
+
+@MyActor func exhibitRace1() async {
+  let ns = NotSendable(x: 0)
+  MyActor.ns = ns
+
+  // expected-region-isolation-warning@+1 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller; this is an error in Swift 6}}
+  await { @YourActor in
+    // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}}
+    YourActor.ns = ns
+  }()
+
+  await withTaskGroup(of: Void.self) {
+    $0.addTask {
+      await MyActor.ohNo()
+    }
+
+    $0.addTask {
+      await YourActor.ohNo()
+    }
+  }
+}
+
+@MyActor func exhibitRace2() async {
+  let ns = NotSendable(x: 0)
+  ns.stash()
+
+  // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
+  await { @YourActor in
+    // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}}
+    YourActor.ns = ns
+  }()
+
+  await withTaskGroup(of: Void.self) {
+    $0.addTask {
+      await MyActor.ohNo()
+    }
+
+    $0.addTask {
+      await YourActor.ohNo()
+    }
+  }
+}
+
+@MyActor func exhibitRace3() async {
+  let ns = NotSendable()
+
+  // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533)
+  await { @YourActor in
+    // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}}
+    YourActor.ns = ns
+  }()
+
+  await withTaskGroup(of: Void.self) {
+    $0.addTask {
+      await MyActor.ohNo()
+    }
+
+    $0.addTask {
+      await YourActor.ohNo()
+    }
+  }
+}