From 1336dd4cb52f0c016242396d2fad2b70b5436584 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 9 May 2025 13:05:57 -0700 Subject: [PATCH] [Concurrency] Diagnose loss of global actor isolation in async function conversions Perform `Sendable` checking on parameter/result of the function type when conversion between asynchroneous functions results in a loss of global actor isolation attribute because access would result in data crossing an isolation boundary. This is a warning until Swift language mode 6. Resolves: rdar://130168104 (cherry picked from commit a058ffc9983e4bdbf7b411cf3008998365c1f77f) --- lib/Sema/TypeCheckConcurrency.cpp | 63 ++++++++++--------- .../attr_execution/conversions.swift | 5 ++ .../attr_execution/conversions_silgen.swift | 14 ++--- .../global_actor_function_types_swift6.swift | 15 +++++ 4 files changed, 59 insertions(+), 38 deletions(-) create mode 100644 test/Concurrency/global_actor_function_types_swift6.swift diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index c083a609287c8..3700efe422b53 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2290,12 +2290,6 @@ static bool safeToDropGlobalActor( if (otherIsolation.isErased()) return true; - // We currently allow unconditional dropping of global actors from - // async function types, despite this confusing Sendable checking - // in light of SE-338. - if (funcTy->isAsync()) - return true; - // If the argument is passed over an isolation boundary, it's not // safe to erase actor isolation, because the callee can call the // function synchronously from outside the isolation domain. @@ -2645,7 +2639,8 @@ namespace { /// 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) { + [&](FunctionType *fnType, + std::optional warnUntilSwiftMode = std::nullopt) { auto *dc = getDeclContext(); llvm::SmallPtrSet nonSendableTypes; @@ -2675,8 +2670,8 @@ namespace { diag::invalid_function_conversion_with_non_sendable, fromType, toType); - if (downgradeToWarning) - diag.warnUntilFutureSwiftVersion(); + if (warnUntilSwiftMode) + diag.warnUntilSwiftVersion(*warnUntilSwiftMode); } for (auto type : nonSendableTypes) { @@ -2693,20 +2688,27 @@ namespace { auto toIsolation = toFnType->getIsolation(); if (auto fromActor = fromFnType->getGlobalActor()) { - if (toFnType->hasGlobalActor() || - (toFnType->isAsync() && !toIsolation.isNonIsolatedCaller())) - return; + if (!toFnType->hasGlobalActor()) { + auto dc = const_cast(getDeclContext()); + // If it's unsafe to drop global actor attribute: + // - for Sendable types we are going to perform Sendability + // checking of parameters/result. + // - for non-Sendable types we either leave it to region-based + // isolation to determine whether it's okay or not or + // diagnose if types are not-async. + if (safeToDropGlobalActor(dc, fromActor, toType, + getImmediateApply())) { + return; + } - auto dc = const_cast(getDeclContext()); - if (!safeToDropGlobalActor(dc, fromActor, toType, - getImmediateApply())) { - // otherwise, it's not a safe cast. - ctx.Diags - .diagnose(funcConv->getLoc(), - diag::converting_func_loses_global_actor, fromType, - toType, fromActor) - .warnUntilSwiftVersion(6); - return; + if (!toFnType->isAsync()) { + ctx.Diags + .diagnose(funcConv->getLoc(), + diag::converting_func_loses_global_actor, + fromType, toType, fromActor) + .warnUntilSwiftVersion(6); + return; + } } } @@ -2772,11 +2774,12 @@ namespace { case FunctionTypeIsolation::Kind::NonIsolatedCaller: case FunctionTypeIsolation::Kind::Erased: diagnoseNonSendableParametersAndResult( - toFnType, /*downgradeToWarning=*/true); + toFnType, version::Version::getFutureMajorLanguageVersion()); break; case FunctionTypeIsolation::Kind::GlobalActor: { - // Handled above by `safeToDropGlobalActor` check. + diagnoseNonSendableParametersAndResult(toFnType, + /*warnUntilSwiftMode*/ 6); break; } @@ -2784,7 +2787,8 @@ namespace { // nonisolated synchronous <-> @concurrent if (fromFnType->isAsync() != toFnType->isAsync()) { diagnoseNonSendableParametersAndResult( - toFnType, /*downgradeToWarning=*/true); + toFnType, + version::Version::getFutureMajorLanguageVersion()); } break; } @@ -2799,7 +2803,7 @@ namespace { case FunctionTypeIsolation::Kind::Parameter: case FunctionTypeIsolation::Kind::Erased: diagnoseNonSendableParametersAndResult( - toFnType, /*downgradeToWarning=*/true); + toFnType, version::Version::getFutureMajorLanguageVersion()); break; case FunctionTypeIsolation::Kind::NonIsolated: { @@ -2809,7 +2813,8 @@ namespace { // actor isolation. if (fromFnType->isAsync()) { diagnoseNonSendableParametersAndResult( - toFnType, /*downgradeToWarning=*/true); + toFnType, + version::Version::getFutureMajorLanguageVersion()); break; } // Runs on the actor. @@ -2821,7 +2826,9 @@ namespace { break; case FunctionTypeIsolation::Kind::GlobalActor: - llvm_unreachable("invalid conversion"); + diagnoseNonSendableParametersAndResult( + toFnType, version::Version::getFutureMajorLanguageVersion()); + break; } break; } diff --git a/test/Concurrency/attr_execution/conversions.swift b/test/Concurrency/attr_execution/conversions.swift index 75a996f093af9..6462cd83a2490 100644 --- a/test/Concurrency/attr_execution/conversions.swift +++ b/test/Concurrency/attr_execution/conversions.swift @@ -117,6 +117,11 @@ func testNonSendableDiagnostics( let _: nonisolated(nonsending) () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} // expected-error@-1 {{cannot convert '@MainActor @Sendable () async -> NonSendable' to 'nonisolated(nonsending) () async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: @concurrent (NonSendable) async -> Void = globalActor1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@MainActor @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 _: @concurrent () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} + // expected-warning@-1 {{cannot convert '@MainActor @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + let _: nonisolated(nonsending) (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 'nonisolated(nonsending) (NonSendable) async -> Void' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} let _: nonisolated(nonsending) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}} diff --git a/test/Concurrency/attr_execution/conversions_silgen.swift b/test/Concurrency/attr_execution/conversions_silgen.swift index 287955239d0e4..f6389f5ea2451 100644 --- a/test/Concurrency/attr_execution/conversions_silgen.swift +++ b/test/Concurrency/attr_execution/conversions_silgen.swift @@ -396,8 +396,8 @@ func globalActorConversions3(_ x: @escaping @concurrent (SendableKlass) async -> _ = await v5(SendableKlass()) } -// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYccyADYbScMYcctYaF : $@convention(thin) @async (@guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), @guaranteed @Sendable @callee_guaranteed (@guaranteed SendableKlass) -> (), @guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()) -> () { -// CHECK: bb0([[X:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), [[Y:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed SendableKlass) -> (), [[Z:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()): +// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYcctYaF : $@convention(thin) @async (@guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), @guaranteed @Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()) -> () +// CHECK: bb0([[X:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), [[Y:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()): // CHECK: [[X_C:%.*]] = copy_value [[X]] // CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen16NonSendableKlassCIeghg_ScA_pSgACIegHgg_TR : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional, @guaranteed NonSendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()) -> () @@ -411,19 +411,13 @@ func globalActorConversions3(_ x: @escaping @concurrent (SendableKlass) async -> // CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen13SendableKlassCIeghg_ACIegHg_TRScMTU : $@convention(thin) @async (@guaranteed SendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()) -> () // CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[Y_C]]) -// CHECK: [[Z_C:%.*]] = copy_value [[Z]] -// CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen16NonSendableKlassCIeghg_ACIegHg_TRScMTU : $@convention(thin) @async (@guaranteed NonSendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()) -> () -// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[Z_C]]) - -// CHECK: } // end sil function '$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYccyADYbScMYcctYaF' +// CHECK: } // end sil function '$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYcctYaF' func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> Void, - _ y: @escaping @MainActor @Sendable (SendableKlass) -> Void, - _ z: @escaping @MainActor @Sendable (NonSendableKlass) -> Void) async { + _ y: @escaping @MainActor @Sendable (SendableKlass) -> Void) async { let _: nonisolated(nonsending) (NonSendableKlass) async -> Void = x let _: nonisolated(nonsending) (SendableKlass) async -> Void = y let _: @concurrent (SendableKlass) async -> Void = y - let _: @concurrent (NonSendableKlass) async -> Void = z } func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) { diff --git a/test/Concurrency/global_actor_function_types_swift6.swift b/test/Concurrency/global_actor_function_types_swift6.swift new file mode 100644 index 0000000000000..2d422c06c6eb4 --- /dev/null +++ b/test/Concurrency/global_actor_function_types_swift6.swift @@ -0,0 +1,15 @@ +// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple -language-mode 6 + +final class NonSendable { +} + +@available(*, unavailable) +extension NonSendable: Sendable {} + +actor Test { + func testNonSendableCrossingIsolationinAsync(v: NonSendable) { + let _: () async -> NonSendable = { @MainActor in v } + // expected-error@-1 {{cannot convert '@MainActor @Sendable () async -> NonSendable' to '() async -> NonSendable' because crossing of an isolation boundary requires parameter and result types to conform to 'Sendable' protocol}} + // expected-note@-2 {{type 'NonSendable' does not conform to 'Sendable' protocol}} + } +}