From 641cfe06be653d5ae2ffdbe315d86e92416cefb3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 4 Sep 2025 16:51:45 -0700 Subject: [PATCH 1/4] [Concurrency] Fix `@Sendable` closures not inferring `nonisolated(nonsending)` If there are no explicit concurrency attributes, isolated parameters, or captures associated with the closure it should infer `nonisolated(nonsending)` for the parent conversion injected by the solver (this conversion is injected because the solver cannot check captures to elide it). The change pushes `isIsolationInferenceBoundaryClosure` check down with added benefit of getting preconcurrency context from the parent. (cherry picked from commit 3ae34e8e68ce3f105a067451b7df092324e24d34) --- lib/Sema/TypeCheckConcurrency.cpp | 20 +++++++++---------- test/Concurrency/actor_inout_isolation.swift | 2 +- .../attr_execution/conversions_silgen.swift | 8 ++++++++ .../sendable_checking_captures_swift5.swift | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 90691b5030215..fda5f5b9415d2 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -5007,16 +5007,6 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation( return ActorIsolation::forActorInstanceCapture(param); } - // If we have a closure that acts as an isolation inference boundary, then - // we return that it is non-isolated. - // - // NOTE: Since we already checked for global actor isolated things, we - // know that all Sendable closures must be nonisolated. That is why it is - // safe to rely on this path to handle Sendable closures. - if (isIsolationInferenceBoundaryClosure(closure, - /*canInheritActorContext=*/true)) - return ActorIsolation::forNonisolated(/*unsafe=*/false); - // A non-Sendable closure gets its isolation from its context. auto parentIsolation = getActorIsolationOfContext( closure->getParent(), getClosureActorIsolation); @@ -5048,6 +5038,16 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation( } } + // If we have a closure that acts as an isolation inference boundary, then + // we return that it is non-isolated. + // + // NOTE: Since we already checked for global actor isolated things, we + // know that all Sendable closures must be nonisolated. That is why it is + // safe to rely on this path to handle Sendable closures. + if (isIsolationInferenceBoundaryClosure(closure, + /*canInheritActorContext=*/true)) + return ActorIsolation::forNonisolated(/*unsafe=*/false); + return normalIsolation; }(); diff --git a/test/Concurrency/actor_inout_isolation.swift b/test/Concurrency/actor_inout_isolation.swift index f318f4d702b50..7b244f2043143 100644 --- a/test/Concurrency/actor_inout_isolation.swift +++ b/test/Concurrency/actor_inout_isolation.swift @@ -220,7 +220,7 @@ if #available(SwiftStdlib 5.1, *) { let _ = Task.detached { await { (_ foo: inout Int) async in foo += 1 }(&number) } // expected-error @-1 {{actor-isolated var 'number' cannot be passed 'inout' to 'async' function call}} // expected-minimal-error @-2 {{global actor 'MyGlobalActor'-isolated var 'number' can not be used 'inout' from a nonisolated context}} - // expected-complete-tns-error @-3 {{main actor-isolated var 'number' can not be used 'inout' from a nonisolated context}} + // expected-complete-tns-warning @-3 {{main actor-isolated var 'number' can not be used 'inout' from a nonisolated context}} } // attempt to pass global state owned by the global actor to another async function diff --git a/test/Concurrency/attr_execution/conversions_silgen.swift b/test/Concurrency/attr_execution/conversions_silgen.swift index 0ecacaa916f91..65b012fbb2edd 100644 --- a/test/Concurrency/attr_execution/conversions_silgen.swift +++ b/test/Concurrency/attr_execution/conversions_silgen.swift @@ -613,3 +613,11 @@ func testConvertToThrowing(isolation: isolated (any Actor)? = #isolation) async observe() } } + +func testSendableClosureNonisolatedNonSendingInference() { + // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFySiYaYbYCcfU_ : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, Int) -> () + // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional, %1 : $Int): + // CHECK: hop_to_executor [[EXECUTOR]] + // CHECK: // end sil function '$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFySiYaYbYCcfU_' + let _: nonisolated(nonsending) @Sendable (Int) async -> Void = { _ in } +} diff --git a/test/Concurrency/sendable_checking_captures_swift5.swift b/test/Concurrency/sendable_checking_captures_swift5.swift index 2346d64079ec3..470b40607e2f9 100644 --- a/test/Concurrency/sendable_checking_captures_swift5.swift +++ b/test/Concurrency/sendable_checking_captures_swift5.swift @@ -92,5 +92,5 @@ do { let c: Class test(c) - // expected-complete-warning@-1:8 {{implicit capture of 'c' requires that 'Class' conforms to 'Sendable'; this is an error in the Swift 6 language mode}} + // expected-complete-warning@-1:8 {{implicit capture of 'c' requires that 'Class' conforms to 'Sendable'}} } From 4699419dabd332e6c6eee14f4a4208fbc6b2a8bb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 19 Sep 2025 11:49:12 -0700 Subject: [PATCH 2/4] [Concurrency] Check function conversions after their sub-expressions have been checked Explicit closure expressions gets their isolation inferred during actor isolation checking, if they appear as a sub-expression to a function conversion we need to delay function checking until the closure was processed. It shouldn't matter for other cases. (cherry picked from commit 61fbaa7af7f7d46b75758bf1d4cb9148acad0933) --- lib/Sema/TypeCheckConcurrency.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index fda5f5b9415d2..3805d81c620e0 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -3477,19 +3477,6 @@ namespace { } } - // The constraint solver may not have chosen legal casts. - if (auto funcConv = dyn_cast(expr)) { - checkFunctionConversion(funcConv, - funcConv->getSubExpr()->getType(), - funcConv->getType()); - } - - if (auto *isolationErasure = dyn_cast(expr)) { - checkFunctionConversion(isolationErasure, - isolationErasure->getSubExpr()->getType(), - isolationErasure->getType()); - } - if (auto *defaultArg = dyn_cast(expr)) { checkDefaultArgument(defaultArg); } @@ -3581,6 +3568,19 @@ namespace { } } + // The constraint solver may not have chosen legal casts. + if (auto funcConv = dyn_cast(expr)) { + checkFunctionConversion(funcConv, + funcConv->getSubExpr()->getType(), + funcConv->getType()); + } + + if (auto *isolationErasure = dyn_cast(expr)) { + checkFunctionConversion(isolationErasure, + isolationErasure->getSubExpr()->getType(), + isolationErasure->getType()); + } + return Action::Continue(expr); } From 18809358e19c82ac18b8172e96c4cf4585594dbd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 19 Sep 2025 11:52:38 -0700 Subject: [PATCH 3/4] [Concurrency] Avoid Sendable checking for conversions that strip `@Sendable` from `nonisolated(nonsending)` function type The isolation doesn't change in this cause and making function type non-Sendable means that it woun't be able to leave the current concurrency domain and the compiler won't generate a thunk. (cherry picked from commit 40c9674dfde44e7d048a14455ff3b3f6521a2dc4) --- lib/Sema/TypeCheckConcurrency.cpp | 13 +++++-------- .../attr_execution/conversions_silgen.swift | 8 ++++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 3805d81c620e0..b1062463dae9e 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2797,16 +2797,13 @@ namespace { diagnoseNonSendableParametersAndResult(toFnType); break; - case FunctionTypeIsolation::Kind::Parameter: - llvm_unreachable("invalid conversion"); - + // @Sendable nonisolated(nonsending) -> nonisolated(nonsending) + // doesn't require Sendable checking. case FunctionTypeIsolation::Kind::NonIsolatedCaller: - // Non isolated caller is always async. This can only occur if we - // are converting from an `@Sendable` representation to something - // else. So we need to just check that we diagnose non sendable - // parameters and results. - diagnoseNonSendableParametersAndResult(toFnType); break; + + case FunctionTypeIsolation::Kind::Parameter: + llvm_unreachable("invalid conversion"); } break; } diff --git a/test/Concurrency/attr_execution/conversions_silgen.swift b/test/Concurrency/attr_execution/conversions_silgen.swift index 65b012fbb2edd..959aeecb4c3d4 100644 --- a/test/Concurrency/attr_execution/conversions_silgen.swift +++ b/test/Concurrency/attr_execution/conversions_silgen.swift @@ -620,4 +620,12 @@ func testSendableClosureNonisolatedNonSendingInference() { // CHECK: hop_to_executor [[EXECUTOR]] // CHECK: // end sil function '$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFySiYaYbYCcfU_' let _: nonisolated(nonsending) @Sendable (Int) async -> Void = { _ in } + + // CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFyS2SYaKYCcYaYbYCcfU0_ : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed String) -> (@owned String, @error any Error)) -> @error any Error + // CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional, %1 : @guaranteed $@async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed String) -> (@owned String, @error any Error)): + // CHECK: hop_to_executor [[EXECUTOR]] + // CHECK: // end sil function '$s21attr_execution_silgen49testSendableClosureNonisolatedNonSendingInferenceyyFyS2SYaKYCcYaYbYCcfU0_' + let _: nonisolated(nonsending) @Sendable ( + nonisolated(nonsending) @escaping (String) async throws -> String + ) async throws -> Void = { _ in } } From a886c5f38acfcbfc359643ba91b29211a4c979f4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 19 Sep 2025 14:13:28 -0700 Subject: [PATCH 4/4] [Concurrency] Prevent extraneous sendability checking when applying `nonisolated(nonsending)` `nonisolated(nonsending)` is not presented in interface types which means that references to declarations that have this attribute require a function conversion to apply it. Function conversion checking should detect that and avoid sendability checking for situations like that but there is really no conversion there. (cherry picked from commit ce3310050b3fbd5354d3dbe54385f960f62e7609) --- lib/Sema/TypeCheckConcurrency.cpp | 15 ++++++++++++++- .../attr_execution/conversions_silgen.swift | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index b1062463dae9e..c290e4969c49e 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2683,7 +2683,8 @@ namespace { /// Some function conversions synthesized by the constraint solver may not /// be correct AND the solver doesn't know, so we must emit a diagnostic. - void checkFunctionConversion(Expr *funcConv, Type fromType, Type toType) { + void checkFunctionConversion(ImplicitConversionExpr *funcConv, + Type fromType, Type toType) { auto diagnoseNonSendableParametersAndResult = [&](FunctionType *fnType, std::optional warnUntilSwiftMode = std::nullopt) { @@ -2788,6 +2789,18 @@ namespace { if (!fromFnType->isAsync()) break; + // Applying `nonisolated(nonsending)` to an interface type + // of a declaration. + if (auto *declRef = + dyn_cast(funcConv->getSubExpr())) { + auto *decl = declRef->getDecl(); + if (auto *nonisolatedAttr = + decl->getAttrs().getAttribute()) { + if (nonisolatedAttr->isNonSending()) + return; + } + } + // @concurrent -> nonisolated(nonsending) // crosses an isolation boundary. LLVM_FALLTHROUGH; diff --git a/test/Concurrency/attr_execution/conversions_silgen.swift b/test/Concurrency/attr_execution/conversions_silgen.swift index 959aeecb4c3d4..048b7efefe350 100644 --- a/test/Concurrency/attr_execution/conversions_silgen.swift +++ b/test/Concurrency/attr_execution/conversions_silgen.swift @@ -629,3 +629,17 @@ func testSendableClosureNonisolatedNonSendingInference() { nonisolated(nonsending) @escaping (String) async throws -> String ) async throws -> Void = { _ in } } + +// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF : $@convention(thin) () -> () +// CHECK: [[TEST_REF:%.*]] = function_ref @$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF0D0L_7closureyS2SYaKYCc_tYaYbKF : $@convention(thin) @Sendable @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed String) -> (@owned String, @error any Error)) -> @error any Error +// CHECK: // end sil function '$s21attr_execution_silgen014testSendableToE35ConversionWithNonisilatedNonsendingyyF' +func testSendableToSendableConversionWithNonisilatedNonsending() { + @Sendable nonisolated(nonsending) func test( + closure: nonisolated(nonsending) @escaping (String) async throws -> String + ) async throws { + } + + let _: nonisolated(nonsending) @Sendable ( + nonisolated(nonsending) @escaping (String) async throws -> String + ) async throws -> Void = test +}