Merge pull request #80924 from xedin/rdar-149107104-6.2

[6.2][TypeChecker] Allow closures to assume `nonisolated(nonsending)`

Author: xedin

lib/SILGen/SILGenConcurrency.cpp

@@ -206,10 +206,14 @@ void SILGenFunction::emitExpectedExecutorProlog() {
     switch (actorIsolation.getKind()) {
     case ActorIsolation::Unspecified:
     case ActorIsolation::Nonisolated:
-    case ActorIsolation::CallerIsolationInheriting:
     case ActorIsolation::NonisolatedUnsafe:
       break;
 
+    case ActorIsolation::CallerIsolationInheriting:
+      assert(F.isAsync());
+      setExpectedExecutorForParameterIsolation(*this, actorIsolation);
+      break;
+
     case ActorIsolation::Erased:
       llvm_unreachable("closure cannot have erased isolation");
 

lib/Sema/CSApply.cpp

@@ -6501,18 +6501,31 @@ ArgumentList *ExprRewriter::coerceCallArguments(
   return ArgumentList::createTypeChecked(ctx, args, newArgs);
 }
 
-/// Whether the given expression is a closure that should inherit
-/// the actor context from where it was formed.
-static bool closureInheritsActorContext(Expr *expr) {
+/// Looks through any non-semantic expressions and a capture list
+/// to find out whether the given expression is an explicit closure.
+static ClosureExpr *isExplicitClosureExpr(Expr *expr) {
   if (auto IE = dyn_cast<IdentityExpr>(expr))
-    return closureInheritsActorContext(IE->getSubExpr());
+    return isExplicitClosureExpr(IE->getSubExpr());
 
   if (auto CLE = dyn_cast<CaptureListExpr>(expr))
-    return closureInheritsActorContext(CLE->getClosureBody());
+    return isExplicitClosureExpr(CLE->getClosureBody());
+
+  return dyn_cast<ClosureExpr>(expr);
+}
 
-  if (auto CE = dyn_cast<ClosureExpr>(expr))
+/// Whether the given expression is a closure that should inherit
+/// the actor context from where it was formed.
+static bool closureInheritsActorContext(Expr *expr) {
+  if (auto *CE = isExplicitClosureExpr(expr))
     return CE->inheritsActorContext();
+  return false;
+}
 
+/// Determine whether the given expression is a closure that
+/// is explicitly marked as `@concurrent`.
+static bool isClosureMarkedAsConcurrent(Expr *expr) {
+  if (auto *CE = isExplicitClosureExpr(expr))
+    return CE->getAttrs().hasAttribute<ConcurrentAttr>();
   return false;
 }
 
@@ -7754,6 +7767,23 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
       }
     }
 
+    // If we have a ClosureExpr, then we can safely propagate the
+    // 'nonisolated(nonsending)' isolation if it's not explicitly
+    // marked as `@concurrent`.
+    if (toEI.getIsolation().isNonIsolatedCaller() &&
+        (fromEI.getIsolation().isNonIsolated() &&
+         !isClosureMarkedAsConcurrent(expr))) {
+      auto newFromFuncType = fromFunc->withIsolation(
+          FunctionTypeIsolation::forNonIsolatedCaller());
+      if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
+        fromFunc = newFromFuncType->castTo<FunctionType>();
+        // Propagating 'nonisolated(nonsending)' might have satisfied the entire
+        // conversion. If so, we're done, otherwise keep converting.
+        if (fromFunc->isEqual(toType))
+          return expr;
+      }
+    }
+
     if (ctx.LangOpts.isDynamicActorIsolationCheckingEnabled()) {
       // Passing a synchronous global actor-isolated function value and
       // parameter that expects a synchronous non-isolated function type could

lib/Sema/TypeCheckConcurrency.cpp

@@ -4701,6 +4701,13 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
       }
     }
 
+    // `nonisolated(nonsending)` inferred from the context makes
+    // the closure caller isolated.
+    if (auto *closureTy = getType(closure)->getAs<FunctionType>()) {
+      if (closureTy->getIsolation().isNonIsolatedCaller())
+        return ActorIsolation::forCallerIsolationInheriting();
+    }
+
     // If a closure has an isolated parameter, it is isolated to that
     // parameter.
     for (auto param : *closure->getParameters()) {

test/Concurrency/Runtime/startSynchronously.swift

@@ -64,6 +64,23 @@ actor MyGlobalActor {
   static let shared: MyGlobalActor = MyGlobalActor()
 }
 
+final class NaiveQueueExecutor: SerialExecutor {
+  let queue: DispatchQueue
+
+  init(queue: DispatchQueue) {
+    self.queue = queue
+  }
+
+  public func enqueue(_ job: consuming ExecutorJob) {
+    let unowned = UnownedJob(job)
+    print("NaiveQueueExecutor(\(self.queue.label)) enqueue... [thread:\(getCurrentThreadID())]")
+    queue.async {
+    print("NaiveQueueExecutor(\(self.queue.label)) enqueue: run [thread:\(getCurrentThreadID())]")
+      unowned.runSynchronously(on: self.asUnownedSerialExecutor())
+    }
+  }
+}
+
 // Test on all platforms
 func syncOnMyGlobalActor() -> [Task<Void, Never>] {
   MyGlobalActor.shared.preconditionIsolated("Should be executing on the global actor here")
@@ -189,7 +206,7 @@ syncOnNonTaskThread(synchronousTask: behavior)
 // CHECK: after startSynchronously, outside; cancel (wakeup) the synchronous task!  [thread:[[CALLING_THREAD3]]]
 
 print("\n\n==== ------------------------------------------------------------------")
-print("callActorFromStartSynchronousTask()")
+print("callActorFromStartSynchronousTask() - not on specific queue")
 callActorFromStartSynchronousTask(recipient: .recipient(Recipient()))
 
 // CHECK: callActorFromStartSynchronousTask()
@@ -203,11 +220,6 @@ callActorFromStartSynchronousTask(recipient: .recipient(Recipient()))
 // CHECK-NOT: ERROR!
 // CHECK: inside startSynchronously, call rec.sync() done
 
-// CHECK-NOT: ERROR!
-// CHECK: inside startSynchronously, call rec.async()
-// CHECK-NOT: ERROR!
-// CHECK: inside startSynchronously, call rec.async() done
-
 // CHECK-NOT: ERROR!
 // CHECK: inside startSynchronously, done
 
@@ -219,35 +231,20 @@ enum TargetActorToCall {
 }
 
 protocol RecipientProtocol where Self: Actor {
-  func sync(syncTaskThreadID: ThreadID) async
-  func async(syncTaskThreadID: ThreadID) async
+  func callAndSuspend(syncTaskThreadID: ThreadID) async
 }
 
 // default actor, must not declare an 'unownedExecutor'
-actor Recipient {
-  func sync(syncTaskThreadID: ThreadID) {
-    self.preconditionIsolated()
-
-    print("\(Recipient.self)/\(#function) Current actor thread id = \(getCurrentThreadID()) @ :\(#line)")
-    if compareThreadIDs(syncTaskThreadID, .equal, getCurrentThreadID()) {
-      print("NOTICE: Actor must not run on the synchronous task's thread :\(#line)")
-    }
-  }
-
-  func async(syncTaskThreadID: ThreadID) async {
+actor Recipient: RecipientProtocol {
+  func callAndSuspend(syncTaskThreadID: ThreadID) async {
     self.preconditionIsolated()
 
-    // Dispatch may end up reusing the thread used to service the queue so we
-    // cannot truly assert exact thread identity in such tests.
-    // Usually this will be on a different thread by now though.
     print("\(Recipient.self)/\(#function) Current actor thread id = \(getCurrentThreadID()) @ :\(#line)")
     if compareThreadIDs(syncTaskThreadID, .equal, getCurrentThreadID()) {
       print("NOTICE: Actor must not run on the synchronous task's thread :\(#line)")
     }
 
-    await Task {
-      self.preconditionIsolated()
-    }.value
+    try? await Task.sleep(for: .milliseconds(100))
   }
 }
 
@@ -274,8 +271,8 @@ func callActorFromStartSynchronousTask(recipient rec: TargetActorToCall) {
 
       print("inside startSynchronously, call rec.sync() [thread:\(getCurrentThreadID())] @ :\(#line)")
       switch rec {
-      case .recipient(let recipient): await recipient.sync(syncTaskThreadID: innerTID)
-      case .recipientOnQueue(let recipient): await recipient.sync(syncTaskThreadID: innerTID)
+      case .recipient(let recipient): await recipient.callAndSuspend(syncTaskThreadID: innerTID)
+      case .recipientOnQueue(let recipient): await recipient.callAndSuspend(syncTaskThreadID: innerTID)
       }
       print("inside startSynchronously, call rec.sync() done [thread:\(getCurrentThreadID())] @ :\(#line)")
 
@@ -290,22 +287,6 @@ func callActorFromStartSynchronousTask(recipient rec: TargetActorToCall) {
         print("NOTICE: Task resumed on same thread as it entered the synchronous task!")
       }
 
-      print("inside startSynchronously, call rec.async() [thread:\(getCurrentThreadID())] @ :\(#line)")
-      switch rec {
-      case .recipient(let recipient): await recipient.async(syncTaskThreadID: innerTID)
-      case .recipientOnQueue(let recipient): await recipient.async(syncTaskThreadID: innerTID)
-      }
-      print("inside startSynchronously, call rec.async() done [thread:\(getCurrentThreadID())] @ :\(#line)")
-
-      print("Inner thread id = \(innerTID)")
-      print("Current thread id = \(getCurrentThreadID())")
-      // Dispatch may end up reusing the thread used to service the queue so we
-      // cannot truly assert exact thread identity in such tests.
-      // Usually this will be on a different thread by now though.
-      if compareThreadIDs(innerTID, .equal, getCurrentThreadID()) {
-        print("NOTICE: Task resumed on same thread as it entered the synchronous task!")
-      }
-
       print("inside startSynchronously, done [thread:\(getCurrentThreadID())] @ :\(#line)")
       sem1.signal()
     }
@@ -323,6 +304,28 @@ print("callActorFromStartSynchronousTask() - actor in custom executor with its o
 let actorQueue = DispatchQueue(label: "recipient-actor-queue")
 callActorFromStartSynchronousTask(recipient: .recipientOnQueue(RecipientOnQueue(queue: actorQueue)))
 
+
+//            50: callActorFromStartSynchronousTask()
+//            51: before startSynchronously [thread:0x00007000054f5000] @ :366
+//            52: inside startSynchronously [thread:0x00007000054f5000] @ :372
+//            53: inside startSynchronously, call rec.sync() [thread:0x00007000054f5000] @ :380
+//            54: Recipient/sync(syncTaskThreadID:) Current actor thread id = 0x000070000567e000 @ :336
+//            55: inside startSynchronously, call rec.sync() done [thread:0x000070000567e000] @ :385
+//            56: Inner thread id = 0x00007000054f5000
+//            57: Current thread id = 0x000070000567e000
+//            60: after startSynchronously [thread:0x00007000054f5000] @ :418
+//            61: - async work on queue
+//            62: - async work on queue
+//            63: - async work on queue
+//            64: - async work on queue
+//            65: - async work on queue
+//            67: - async work on queue
+//            68: - async work on queue
+//            69: - async work on queue
+//            71: Inner thread id = 0x00007000054f5000
+//            72: Current thread id = 0x000070000567e000
+//            73: inside startSynchronously, done [thread:0x000070000567e000] @ :414
+
 // CHECK-LABEL: callActorFromStartSynchronousTask() - actor in custom executor with its own queue
 // No interleaving allowed between "before" and "inside":
 // CHECK: before startSynchronously [thread:[[CALLING_THREAD4:.*]]]
@@ -333,38 +336,14 @@ callActorFromStartSynchronousTask(recipient: .recipientOnQueue(RecipientOnQueue(
 // allowing the 'after startSynchronously' to run.
 //
 // CHECK-NEXT: inside startSynchronously, call rec.sync() [thread:[[CALLING_THREAD4]]]
-// CHECK: NaiveQueueExecutor(recipient-actor-queue) enqueue
 // CHECK: after startSynchronously
 // CHECK-NOT: ERROR!
 // CHECK: inside startSynchronously, call rec.sync() done
 
-// CHECK-NOT: ERROR!
-// CHECK: inside startSynchronously, call rec.async()
-// CHECK: NaiveQueueExecutor(recipient-actor-queue) enqueue
-// CHECK-NOT: ERROR!
-// CHECK: inside startSynchronously, call rec.async() done
-
 // CHECK-NOT: ERROR!
 // CHECK: inside startSynchronously, done
 
-final class NaiveQueueExecutor: SerialExecutor {
-  let queue: DispatchQueue
-
-  init(queue: DispatchQueue) {
-    self.queue = queue
-  }
-
-  public func enqueue(_ job: consuming ExecutorJob) {
-    let unowned = UnownedJob(job)
-    print("NaiveQueueExecutor(\(self.queue.label)) enqueue... [thread:\(getCurrentThreadID())]")
-    queue.async {
-    print("NaiveQueueExecutor(\(self.queue.label)) enqueue: run [thread:\(getCurrentThreadID())]")
-      unowned.runSynchronously(on: self.asUnownedSerialExecutor())
-    }
-  }
-}
-
-actor RecipientOnQueue {
+actor RecipientOnQueue: RecipientProtocol {
   let executor: NaiveQueueExecutor
   nonisolated let unownedExecutor: UnownedSerialExecutor
 
@@ -373,30 +352,15 @@ actor RecipientOnQueue {
     self.unownedExecutor = executor.asUnownedSerialExecutor()
   }
 
-  func sync(syncTaskThreadID: ThreadID) {
-    self.preconditionIsolated()
-    dispatchPrecondition(condition: .onQueue(self.executor.queue))
-
-    print("\(Recipient.self)/\(#function) Current actor thread id = \(getCurrentThreadID()) @ :\(#line)")
-    if compareThreadIDs(syncTaskThreadID, .equal, getCurrentThreadID()) {
-      print("NOTICE: Actor must not run on the synchronous task's thread :\(#line)")
-    }
-  }
-
-  func async(syncTaskThreadID: ThreadID) async {
+  func callAndSuspend(syncTaskThreadID: ThreadID) async {
     self.preconditionIsolated()
     dispatchPrecondition(condition: .onQueue(self.executor.queue))
 
-    // Dispatch may end up reusing the thread used to service the queue so we
-    // cannot truly assert exact thread identity in such tests.
-    // Usually this will be on a different thread by now though.
     print("\(Recipient.self)/\(#function) Current actor thread id = \(getCurrentThreadID()) @ :\(#line)")
     if compareThreadIDs(syncTaskThreadID, .equal, getCurrentThreadID()) {
       print("NOTICE: Actor must not run on the synchronous task's thread :\(#line)")
     }
 
-    await Task {
-      self.preconditionIsolated()
-    }.value
+    try? await Task.sleep(for: .milliseconds(100))
   }
 }

test/Concurrency/attr_execution/conversions_silgen.swift

@@ -425,3 +425,34 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
   let _: @concurrent (SendableKlass) async -> Void = y
   let _: @concurrent (NonSendableKlass) async -> Void = z
 }
+
+func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
+  // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
+  // CHECK: hop_to_executor [[EXECUTOR]]
+  let _: nonisolated(nonsending) () async -> Void = {
+    42
+  }
+
+  func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
+
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> @error any Error
+  // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
+  // CHECK: hop_to_executor [[EXECUTOR]]
+  testParam { 42 }
+
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU1_ : $@convention(thin) @async () -> ()
+  // CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
+  // CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
+  testParam { @concurrent in 42 }
+
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()
+  // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
+  // CHECK: hop_to_executor [[EXECUTOR]]
+  fn = { _ in }
+
+  // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU3_ : $@convention(thin) @async (Int) -> ()
+  // CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
+  // CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
+  fn = { @concurrent _ in }
+}