Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/SILGen/SILGenConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
42 changes: 36 additions & 6 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6494,18 +6494,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;
}

Expand Down Expand Up @@ -7747,6 +7760,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
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4699,6 +4699,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()) {
Expand Down
134 changes: 49 additions & 85 deletions test/Concurrency/Runtime/startSynchronously.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand All @@ -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

Expand All @@ -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))
}
}

Expand All @@ -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)")

Expand All @@ -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()
}
Expand All @@ -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:.*]]]
Expand All @@ -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

Expand All @@ -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))
}
}
31 changes: 31 additions & 0 deletions test/Concurrency/attr_execution/conversions_silgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}