Skip to content

Commit 1336dd4

Browse files
xedinxedin
committed
[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 a058ffc)
1 parent 5c06176 commit 1336dd4

File tree

4 files changed

+59
-38
lines changed

4 files changed

+59
-38
lines changed

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,12 +2290,6 @@ static bool safeToDropGlobalActor(
22902290
if (otherIsolation.isErased())
22912291
return true;
22922292

2293-
// We currently allow unconditional dropping of global actors from
2294-
// async function types, despite this confusing Sendable checking
2295-
// in light of SE-338.
2296-
if (funcTy->isAsync())
2297-
return true;
2298-
22992293
// If the argument is passed over an isolation boundary, it's not
23002294
// safe to erase actor isolation, because the callee can call the
23012295
// function synchronously from outside the isolation domain.
@@ -2645,7 +2639,8 @@ namespace {
26452639
/// be correct AND the solver doesn't know, so we must emit a diagnostic.
26462640
void checkFunctionConversion(Expr *funcConv, Type fromType, Type toType) {
26472641
auto diagnoseNonSendableParametersAndResult =
2648-
[&](FunctionType *fnType, bool downgradeToWarning = false) {
2642+
[&](FunctionType *fnType,
2643+
std::optional<unsigned> warnUntilSwiftMode = std::nullopt) {
26492644
auto *dc = getDeclContext();
26502645
llvm::SmallPtrSet<Type, 2> nonSendableTypes;
26512646

@@ -2675,8 +2670,8 @@ namespace {
26752670
diag::invalid_function_conversion_with_non_sendable,
26762671
fromType, toType);
26772672

2678-
if (downgradeToWarning)
2679-
diag.warnUntilFutureSwiftVersion();
2673+
if (warnUntilSwiftMode)
2674+
diag.warnUntilSwiftVersion(*warnUntilSwiftMode);
26802675
}
26812676

26822677
for (auto type : nonSendableTypes) {
@@ -2693,20 +2688,27 @@ namespace {
26932688
auto toIsolation = toFnType->getIsolation();
26942689

26952690
if (auto fromActor = fromFnType->getGlobalActor()) {
2696-
if (toFnType->hasGlobalActor() ||
2697-
(toFnType->isAsync() && !toIsolation.isNonIsolatedCaller()))
2698-
return;
2691+
if (!toFnType->hasGlobalActor()) {
2692+
auto dc = const_cast<DeclContext *>(getDeclContext());
2693+
// If it's unsafe to drop global actor attribute:
2694+
// - for Sendable types we are going to perform Sendability
2695+
// checking of parameters/result.
2696+
// - for non-Sendable types we either leave it to region-based
2697+
// isolation to determine whether it's okay or not or
2698+
// diagnose if types are not-async.
2699+
if (safeToDropGlobalActor(dc, fromActor, toType,
2700+
getImmediateApply())) {
2701+
return;
2702+
}
26992703

2700-
auto dc = const_cast<DeclContext*>(getDeclContext());
2701-
if (!safeToDropGlobalActor(dc, fromActor, toType,
2702-
getImmediateApply())) {
2703-
// otherwise, it's not a safe cast.
2704-
ctx.Diags
2705-
.diagnose(funcConv->getLoc(),
2706-
diag::converting_func_loses_global_actor, fromType,
2707-
toType, fromActor)
2708-
.warnUntilSwiftVersion(6);
2709-
return;
2704+
if (!toFnType->isAsync()) {
2705+
ctx.Diags
2706+
.diagnose(funcConv->getLoc(),
2707+
diag::converting_func_loses_global_actor,
2708+
fromType, toType, fromActor)
2709+
.warnUntilSwiftVersion(6);
2710+
return;
2711+
}
27102712
}
27112713
}
27122714

@@ -2772,19 +2774,21 @@ namespace {
27722774
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
27732775
case FunctionTypeIsolation::Kind::Erased:
27742776
diagnoseNonSendableParametersAndResult(
2775-
toFnType, /*downgradeToWarning=*/true);
2777+
toFnType, version::Version::getFutureMajorLanguageVersion());
27762778
break;
27772779

27782780
case FunctionTypeIsolation::Kind::GlobalActor: {
2779-
// Handled above by `safeToDropGlobalActor` check.
2781+
diagnoseNonSendableParametersAndResult(toFnType,
2782+
/*warnUntilSwiftMode*/ 6);
27802783
break;
27812784
}
27822785

27832786
case FunctionTypeIsolation::Kind::NonIsolated: {
27842787
// nonisolated synchronous <-> @concurrent
27852788
if (fromFnType->isAsync() != toFnType->isAsync()) {
27862789
diagnoseNonSendableParametersAndResult(
2787-
toFnType, /*downgradeToWarning=*/true);
2790+
toFnType,
2791+
version::Version::getFutureMajorLanguageVersion());
27882792
}
27892793
break;
27902794
}
@@ -2799,7 +2803,7 @@ namespace {
27992803
case FunctionTypeIsolation::Kind::Parameter:
28002804
case FunctionTypeIsolation::Kind::Erased:
28012805
diagnoseNonSendableParametersAndResult(
2802-
toFnType, /*downgradeToWarning=*/true);
2806+
toFnType, version::Version::getFutureMajorLanguageVersion());
28032807
break;
28042808

28052809
case FunctionTypeIsolation::Kind::NonIsolated: {
@@ -2809,7 +2813,8 @@ namespace {
28092813
// actor isolation.
28102814
if (fromFnType->isAsync()) {
28112815
diagnoseNonSendableParametersAndResult(
2812-
toFnType, /*downgradeToWarning=*/true);
2816+
toFnType,
2817+
version::Version::getFutureMajorLanguageVersion());
28132818
break;
28142819
}
28152820
// Runs on the actor.
@@ -2821,7 +2826,9 @@ namespace {
28212826
break;
28222827

28232828
case FunctionTypeIsolation::Kind::GlobalActor:
2824-
llvm_unreachable("invalid conversion");
2829+
diagnoseNonSendableParametersAndResult(
2830+
toFnType, version::Version::getFutureMajorLanguageVersion());
2831+
break;
28252832
}
28262833
break;
28272834
}

‎test/Concurrency/attr_execution/conversions.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ func testNonSendableDiagnostics(
117117
let _: nonisolated(nonsending) () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
118118
// 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}}
119119

120+
let _: @concurrent (NonSendable) async -> Void = globalActor1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
121+
// 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}}
122+
let _: @concurrent () async -> NonSendable = globalActor2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
123+
// 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}}
124+
120125
let _: nonisolated(nonsending) (NonSendable) async -> Void = erased1 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}
121126
// 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}}
122127
let _: nonisolated(nonsending) () async -> NonSendable = erased2 // expected-note {{type 'NonSendable' does not conform to 'Sendable' protocol}}

‎test/Concurrency/attr_execution/conversions_silgen.swift

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,8 @@ func globalActorConversions3(_ x: @escaping @concurrent (SendableKlass) async ->
396396
_ = await v5(SendableKlass())
397397
}
398398

399-
// 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) -> ()) -> () {
400-
// CHECK: bb0([[X:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), [[Y:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed SendableKlass) -> (), [[Z:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()):
399+
// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYcctYaF : $@convention(thin) @async (@guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), @guaranteed @Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()) -> ()
400+
// CHECK: bb0([[X:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> (), [[Y:%.*]] : @guaranteed $@Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()):
401401

402402
// CHECK: [[X_C:%.*]] = copy_value [[X]]
403403
// CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen16NonSendableKlassCIeghg_ScA_pSgACIegHgg_TR : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed NonSendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()) -> ()
@@ -411,19 +411,13 @@ func globalActorConversions3(_ x: @escaping @concurrent (SendableKlass) async ->
411411
// CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen13SendableKlassCIeghg_ACIegHg_TRScMTU : $@convention(thin) @async (@guaranteed SendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed SendableKlass) -> ()) -> ()
412412
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[Y_C]])
413413

414-
// CHECK: [[Z_C:%.*]] = copy_value [[Z]]
415-
// CHECK: [[THUNK:%.*]] = function_ref @$s21attr_execution_silgen16NonSendableKlassCIeghg_ACIegHg_TRScMTU : $@convention(thin) @async (@guaranteed NonSendableKlass, @guaranteed @Sendable @callee_guaranteed (@guaranteed NonSendableKlass) -> ()) -> ()
416-
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed] [[THUNK]]([[Z_C]])
417-
418-
// CHECK: } // end sil function '$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYccyADYbScMYcctYaF'
414+
// CHECK: } // end sil function '$s21attr_execution_silgen26conversionsFromSyncToAsyncyyyAA16NonSendableKlassCYbc_yAA0jK0CYbScMYcctYaF'
419415

420416
func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> Void,
421-
_ y: @escaping @MainActor @Sendable (SendableKlass) -> Void,
422-
_ z: @escaping @MainActor @Sendable (NonSendableKlass) -> Void) async {
417+
_ y: @escaping @MainActor @Sendable (SendableKlass) -> Void) async {
423418
let _: nonisolated(nonsending) (NonSendableKlass) async -> Void = x
424419
let _: nonisolated(nonsending) (SendableKlass) async -> Void = y
425420
let _: @concurrent (SendableKlass) async -> Void = y
426-
let _: @concurrent (NonSendableKlass) async -> Void = z
427421
}
428422

429423
func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {

‎test/Concurrency/global_actor_function_types_swift6.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple -language-mode 6
2+
3+
final class NonSendable {
4+
}
5+
6+
@available(*, unavailable)
7+
extension NonSendable: Sendable {}
8+
9+
actor Test {
10+
func testNonSendableCrossingIsolationinAsync(v: NonSendable) {
11+
let _: () async -> NonSendable = { @MainActor in v }
12+
// 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}}
13+
// expected-note@-2 {{type 'NonSendable' does not conform to 'Sendable' protocol}}
14+
}
15+
}

0 commit comments

Comments
 (0)