From 45ca72ed738e96a663e708392fb822590bf9c908 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 26 Feb 2025 16:11:12 -0800 Subject: [PATCH 1/4] [Concurrency] Implement sendability checking for `@Sendable` function type conversions Function conversions that cross an isolation boundary require `Sendable` argument and result types, and the destination function type must be `async`. --- include/swift/AST/DiagnosticsSema.def | 7 + lib/Sema/TypeCheckConcurrency.cpp | 146 +++++++++++++++++- .../attr_execution_conversions.swift | 56 +++++++ 3 files changed, 205 insertions(+), 4 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index da70dec265efe..0d1f9414ca0f0 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -8323,6 +8323,13 @@ ERROR(attr_execution_type_attr_incompatible_with_isolated_any,none, "cannot use '@execution' together with @isolated(any)", ()) +ERROR(invalid_function_conversion_with_non_sendable,none, + "cannot convert %0 to %1 because crossing of an isolation boundary " + "requires parameter and result types to conform to 'Sendable' protocol", + (Type, Type)) +NOTE(type_does_not_conform_to_Sendable,none, + "type %0 does not conform to 'Sendable' protocol", (Type)) + #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 084196654c246..2bbe2dbfdeea3 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2654,12 +2654,58 @@ 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) { + auto diagnoseNonSendableParametersAndResult = + [&](FunctionType *fnType, bool downgradeToWarning = false) { + auto *dc = getDeclContext(); + llvm::SmallPtrSet nonSendableTypes; + + SendableCheckContext context(dc); + for (auto ¶m : fnType->getParams()) { + diagnoseNonSendableTypes( + param.getPlainType(), context, + /*inDerivedConformance=*/Type(), funcConv->getLoc(), + [&](Type type, DiagnosticBehavior behavior) { + nonSendableTypes.insert(type); + return true; + }); + } + + diagnoseNonSendableTypes( + fnType->getResult(), context, + /*inDerivedConformance=*/Type(), funcConv->getLoc(), + [&](Type type, DiagnosticBehavior behavior) { + nonSendableTypes.insert(type); + return true; + }); + + if (!nonSendableTypes.empty()) { + auto &ctx = dc->getASTContext(); + { + auto diag = ctx.Diags.diagnose( + funcConv->getLoc(), + diag::invalid_function_conversion_with_non_sendable, + fromType, toType); + + if (downgradeToWarning) + diag.warnUntilSwiftVersion(7); + } + + for (auto type : nonSendableTypes) { + ctx.Diags.diagnose(funcConv->getLoc(), + diag::type_does_not_conform_to_Sendable, + type); + } + } + }; + if (auto fromFnType = fromType->getAs()) { - if (auto fromActor = fromFnType->getGlobalActor()) { - if (auto toFnType = toType->getAs()) { + if (auto toFnType = toType->getAs()) { + auto fromIsolation = fromFnType->getIsolation(); + auto toIsolation = toFnType->getIsolation(); - // ignore some kinds of casts, as they're diagnosed elsewhere. - if (toFnType->hasGlobalActor() || toFnType->isAsync()) + if (auto fromActor = fromFnType->getGlobalActor()) { + if (toFnType->hasGlobalActor() || + (toFnType->isAsync() && !toIsolation.isNonIsolatedCaller())) return; auto dc = const_cast(getDeclContext()); @@ -2672,7 +2718,99 @@ namespace { diag::converting_func_loses_global_actor, fromType, toType, fromActor) .warnUntilSwiftVersion(6); + return; + } + } + + // Conversions from non-Sendable types are handled by + // region-based isolation. + // Function conversions are used to inject concurrency attributes + // into interface types until that changes we won't be able to + // diagnose all of the cases here. + if (!fromFnType->isSendable()) + return; + + switch (toIsolation.getKind()) { + // Converting to `@execution(caller)` function type + case FunctionTypeIsolation::Kind::NonIsolatedCaller: { + switch (fromIsolation.getKind()) { + case FunctionTypeIsolation::Kind::NonIsolated: + case FunctionTypeIsolation::Kind::GlobalActor: + case FunctionTypeIsolation::Kind::Erased: + diagnoseNonSendableParametersAndResult(toFnType); + break; + + case FunctionTypeIsolation::Kind::NonIsolatedCaller: + case FunctionTypeIsolation::Kind::Parameter: + llvm_unreachable("invalid conversion"); + } + break; + } + + // Converting to nonisolated synchronous or @execution(concurrent) + // asynchronous function type could require crossing an isolation + // boundary. + case FunctionTypeIsolation::Kind::NonIsolated: { + switch (fromIsolation.getKind()) { + case FunctionTypeIsolation::Kind::Parameter: + case FunctionTypeIsolation::Kind::NonIsolatedCaller: + case FunctionTypeIsolation::Kind::Erased: + diagnoseNonSendableParametersAndResult( + toFnType, /*downgradeToWarning=*/true); + break; + + case FunctionTypeIsolation::Kind::GlobalActor: { + // Handled above by `safeToDropGlobalActor` check. + break; + } + + case FunctionTypeIsolation::Kind::NonIsolated: { + // nonisolated synchronous <-> @execution(concurrent) + if (fromFnType->isAsync() != toFnType->isAsync()) { + diagnoseNonSendableParametersAndResult( + toFnType, /*downgradeToWarning=*/true); + } + break; + } + } + break; + } + + // Converting to an actor-isolated function always + // requires crossing an isolation boundary. + case FunctionTypeIsolation::Kind::GlobalActor: { + switch (fromIsolation.getKind()) { + case FunctionTypeIsolation::Kind::Parameter: + case FunctionTypeIsolation::Kind::Erased: + diagnoseNonSendableParametersAndResult( + toFnType, /*downgradeToWarning=*/true); + break; + + case FunctionTypeIsolation::Kind::GlobalActor: + // Actor isolation change. + if (fromIsolation.getGlobalActorType()->isEqual( + toIsolation.getGlobalActorType())) { + diagnoseNonSendableParametersAndResult( + toFnType, /*downgradeToWarning=*/true); + } + break; + + // Runs on the actor. + case FunctionTypeIsolation::Kind::NonIsolated: + case FunctionTypeIsolation::Kind::NonIsolatedCaller: + break; } + break; + } + + // Converting to @isolated(any) doesn't cross an isolation + // boundary. + case FunctionTypeIsolation::Kind::Erased: + break; + + // TODO: Figure out what exactly needs to happen here. + case FunctionTypeIsolation::Kind::Parameter: + break; } } } diff --git a/test/Concurrency/attr_execution_conversions.swift b/test/Concurrency/attr_execution_conversions.swift index e44edabb2e542..823567295df00 100644 --- a/test/Concurrency/attr_execution_conversions.swift +++ b/test/Concurrency/attr_execution_conversions.swift @@ -77,3 +77,59 @@ do { // expected-error@-1 {{cannot convert value of type '(@execution(caller) () async -> ()).Type' to expected argument type '(@isolated(any) () async -> Void).Type'}} } + +// Converting to `@execution(caller)` function +class NonSendable {} + +func testNonSendableDiagnostics( + globalActor1: @escaping @Sendable @MainActor (NonSendable) async -> Void, + globalActor2: @escaping @Sendable @MainActor () async -> NonSendable, + erased1: @escaping @Sendable @isolated(any) (NonSendable) async -> Void, + erased2: @escaping @Sendable @isolated(any) () async -> NonSendable, + nonIsolated1: @escaping @Sendable (NonSendable) -> Void, + nonIsolated2: @escaping @Sendable @execution(concurrent) (NonSendable) async -> Void, + nonIsolated3: @escaping @Sendable () -> NonSendable, + nonIsolated4: @escaping @Sendable @execution(concurrent) () async -> NonSendable, + caller1: @escaping @Sendable @execution(caller) (NonSendable) async -> Void, + caller2: @escaping @Sendable @execution(caller) () async -> NonSendable +) { + let _: @execution(caller) (NonSendable) async -> Void = globalActor1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@MainActor @Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@MainActor @Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + let _: @execution(caller) (NonSendable) async -> Void = erased1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@isolated(any) @Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@isolated(any) @Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + let _: @execution(caller) (NonSendable) async -> Void = nonIsolated1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{annot convert '@Sendable (NonSendable) -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) (NonSendable) async -> Void = nonIsolated2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + let _: @execution(caller) () async -> NonSendable = nonIsolated3 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@Sendable () -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) () async -> NonSendable = nonIsolated4 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-error@-1 {{cannot convert '@Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + let _: @execution(concurrent) (NonSendable) async -> Void = erased1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@isolated(any) @Sendable (NonSendable) async -> Void' to '(NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(concurrent) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@isolated(any) @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + + let _: @execution(concurrent) (NonSendable) async -> Void = caller1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@execution(caller) @Sendable (NonSendable) async -> Void' to '(NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(concurrent) () async -> NonSendable = caller2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@execution(caller) @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + + let _: @MainActor (NonSendable) async -> Void = nonIsolated1 // Ok + let _: @MainActor (NonSendable) async -> Void = nonIsolated2 // Ok + + let _: @MainActor () async -> NonSendable = nonIsolated3 // Ok + let _: @MainActor () async -> NonSendable = nonIsolated4 // Ok + + let _: @MainActor (NonSendable) async -> Void = caller1 // Ok + let _: @MainActor () async -> NonSendable = caller2 // Ok +} From 1a42c153b8b79e8acd67f7dd40549142178aa04f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 14:25:05 -0800 Subject: [PATCH 2/4] [Concurrency] Converting from `nonisolated` to `@execution(caller)` does not cross an isolation boundary --- lib/Sema/TypeCheckConcurrency.cpp | 11 ++++++++++- test/Concurrency/attr_execution_conversions.swift | 6 ++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 2bbe2dbfdeea3..6a787bec9ad91 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2734,7 +2734,16 @@ namespace { // Converting to `@execution(caller)` function type case FunctionTypeIsolation::Kind::NonIsolatedCaller: { switch (fromIsolation.getKind()) { - case FunctionTypeIsolation::Kind::NonIsolated: + case FunctionTypeIsolation::Kind::NonIsolated: { + // nonisolated -> @execution(caller) doesn't cross + // an isolation boundary. + if (!fromFnType->isAsync()) + break; + + // @execution(concurrent) -> @execution(caller) + // crosses an isolation boundary. + LLVM_FALLTHROUGH; + } case FunctionTypeIsolation::Kind::GlobalActor: case FunctionTypeIsolation::Kind::Erased: diagnoseNonSendableParametersAndResult(toFnType); diff --git a/test/Concurrency/attr_execution_conversions.swift b/test/Concurrency/attr_execution_conversions.swift index 823567295df00..4261cfff79543 100644 --- a/test/Concurrency/attr_execution_conversions.swift +++ b/test/Concurrency/attr_execution_conversions.swift @@ -103,13 +103,11 @@ func testNonSendableDiagnostics( let _: @execution(caller) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} // expected-error@-1 {{cannot convert '@isolated(any) @Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} - let _: @execution(caller) (NonSendable) async -> Void = nonIsolated1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} - // expected-error@-1 {{annot convert '@Sendable (NonSendable) -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) (NonSendable) async -> Void = nonIsolated1 // Ok let _: @execution(caller) (NonSendable) async -> Void = nonIsolated2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} // expected-error@-1 {{cannot convert '@Sendable (NonSendable) async -> Void' to '@execution(caller) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} - let _: @execution(caller) () async -> NonSendable = nonIsolated3 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} - // expected-error@-1 {{cannot convert '@Sendable () -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @execution(caller) () async -> NonSendable = nonIsolated3 // Ok let _: @execution(caller) () async -> NonSendable = nonIsolated4 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} // expected-error@-1 {{cannot convert '@Sendable () async -> NonSendable' to '@execution(caller) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} From 23739ef9af070d22cb0bbc30c8ba2343e64729c9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 14:34:53 -0800 Subject: [PATCH 3/4] [Concurrency] Converting from @execution(concurrent) to actor-isolated crosses an isolation boundary --- lib/Sema/TypeCheckConcurrency.cpp | 15 ++++++++++++++- test/Concurrency/attr_execution_conversions.swift | 6 ++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 6a787bec9ad91..5d78094b7adb1 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2804,8 +2804,21 @@ namespace { } break; + case FunctionTypeIsolation::Kind::NonIsolated: { + // Since @execution(concurrent) as an asynchronous + // function it would mean that without Sendable + // check it would be possible for non-Sendable state + // to escape from actor isolation. + if (fromFnType->isAsync()) { + diagnoseNonSendableParametersAndResult( + toFnType, /*downgradeToWarning=*/true); + break; + } + // Runs on the actor. + break; + } + // Runs on the actor. - case FunctionTypeIsolation::Kind::NonIsolated: case FunctionTypeIsolation::Kind::NonIsolatedCaller: break; } diff --git a/test/Concurrency/attr_execution_conversions.swift b/test/Concurrency/attr_execution_conversions.swift index 4261cfff79543..566d73bf9c165 100644 --- a/test/Concurrency/attr_execution_conversions.swift +++ b/test/Concurrency/attr_execution_conversions.swift @@ -123,10 +123,12 @@ func testNonSendableDiagnostics( // expected-warning@-1 {{cannot convert '@execution(caller) @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} let _: @MainActor (NonSendable) async -> Void = nonIsolated1 // Ok - let _: @MainActor (NonSendable) async -> Void = nonIsolated2 // Ok + let _: @MainActor (NonSendable) async -> Void = nonIsolated2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@Sendable (NonSendable) async -> Void' to '@MainActor (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} let _: @MainActor () async -> NonSendable = nonIsolated3 // Ok - let _: @MainActor () async -> NonSendable = nonIsolated4 // Ok + let _: @MainActor () async -> NonSendable = nonIsolated4 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@Sendable () async -> NonSendable' to '@MainActor () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} let _: @MainActor (NonSendable) async -> Void = caller1 // Ok let _: @MainActor () async -> NonSendable = caller2 // Ok From 156a43d16287133dc09ce43b43b4bb755a273ded Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 15:23:34 -0800 Subject: [PATCH 4/4] [Concurrency] Make sure that conversion that switches from one actor to another is not accepted --- lib/Sema/TypeCheckConcurrency.cpp | 12 +++--------- test/Concurrency/attr_execution_conversions.swift | 10 ++++++++++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 5d78094b7adb1..4df57a00dde85 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2795,15 +2795,6 @@ namespace { toFnType, /*downgradeToWarning=*/true); break; - case FunctionTypeIsolation::Kind::GlobalActor: - // Actor isolation change. - if (fromIsolation.getGlobalActorType()->isEqual( - toIsolation.getGlobalActorType())) { - diagnoseNonSendableParametersAndResult( - toFnType, /*downgradeToWarning=*/true); - } - break; - case FunctionTypeIsolation::Kind::NonIsolated: { // Since @execution(concurrent) as an asynchronous // function it would mean that without Sendable @@ -2821,6 +2812,9 @@ namespace { // Runs on the actor. case FunctionTypeIsolation::Kind::NonIsolatedCaller: break; + + case FunctionTypeIsolation::Kind::GlobalActor: + llvm_unreachable("invalid conversion"); } break; } diff --git a/test/Concurrency/attr_execution_conversions.swift b/test/Concurrency/attr_execution_conversions.swift index 566d73bf9c165..cca8705dbd50b 100644 --- a/test/Concurrency/attr_execution_conversions.swift +++ b/test/Concurrency/attr_execution_conversions.swift @@ -4,6 +4,11 @@ // REQUIRES: concurrency // REQUIRES: swift_feature_ExecutionAttribute +@globalActor +actor MyActor { + static let shared = MyActor() +} + @execution(concurrent) func concurrentTest() async { } @@ -132,4 +137,9 @@ func testNonSendableDiagnostics( let _: @MainActor (NonSendable) async -> Void = caller1 // Ok let _: @MainActor () async -> NonSendable = caller2 // Ok + + let _: @MyActor (NonSendable) async -> Void = globalActor1 + // expected-error@-1 {{cannot convert value actor-isolated to 'MainActor' to specified type actor-isolated to 'MyActor'}} + let _: @MyActor () async -> NonSendable = globalActor2 + // expected-error@-1 {{cannot convert value actor-isolated to 'MainActor' to specified type actor-isolated to 'MyActor'}} }