Skip to content

Commit a8c5389

Browse files
xedinxedin
authored
Merge pull request #81418 from xedin/rdar-130168104
[Concurrency] Diagnose loss of global actor isolation in async functi…
2 parents 0d3edd6 + a058ffc commit a8c5389

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
@@ -2274,12 +2274,6 @@ static bool safeToDropGlobalActor(
22742274
if (otherIsolation.isErased())
22752275
return true;
22762276

2277-
// We currently allow unconditional dropping of global actors from
2278-
// async function types, despite this confusing Sendable checking
2279-
// in light of SE-338.
2280-
if (funcTy->isAsync())
2281-
return true;
2282-
22832277
// If the argument is passed over an isolation boundary, it's not
22842278
// safe to erase actor isolation, because the callee can call the
22852279
// function synchronously from outside the isolation domain.
@@ -2629,7 +2623,8 @@ namespace {
26292623
/// be correct AND the solver doesn't know, so we must emit a diagnostic.
26302624
void checkFunctionConversion(Expr *funcConv, Type fromType, Type toType) {
26312625
auto diagnoseNonSendableParametersAndResult =
2632-
[&](FunctionType *fnType, bool downgradeToWarning = false) {
2626+
[&](FunctionType *fnType,
2627+
std::optional<unsigned> warnUntilSwiftMode = std::nullopt) {
26332628
auto *dc = getDeclContext();
26342629
llvm::SmallPtrSet<Type, 2> nonSendableTypes;
26352630

@@ -2659,8 +2654,8 @@ namespace {
26592654
diag::invalid_function_conversion_with_non_sendable,
26602655
fromType, toType);
26612656

2662-
if (downgradeToWarning)
2663-
diag.warnUntilFutureSwiftVersion();
2657+
if (warnUntilSwiftMode)
2658+
diag.warnUntilSwiftVersion(*warnUntilSwiftMode);
26642659
}
26652660

26662661
for (auto type : nonSendableTypes) {
@@ -2677,20 +2672,27 @@ namespace {
26772672
auto toIsolation = toFnType->getIsolation();
26782673

26792674
if (auto fromActor = fromFnType->getGlobalActor()) {
2680-
if (toFnType->hasGlobalActor() ||
2681-
(toFnType->isAsync() && !toIsolation.isNonIsolatedCaller()))
2682-
return;
2675+
if (!toFnType->hasGlobalActor()) {
2676+
auto dc = const_cast<DeclContext *>(getDeclContext());
2677+
// If it's unsafe to drop global actor attribute:
2678+
// - for Sendable types we are going to perform Sendability
2679+
// checking of parameters/result.
2680+
// - for non-Sendable types we either leave it to region-based
2681+
// isolation to determine whether it's okay or not or
2682+
// diagnose if types are not-async.
2683+
if (safeToDropGlobalActor(dc, fromActor, toType,
2684+
getImmediateApply())) {
2685+
return;
2686+
}
26832687

2684-
auto dc = const_cast<DeclContext*>(getDeclContext());
2685-
if (!safeToDropGlobalActor(dc, fromActor, toType,
2686-
getImmediateApply())) {
2687-
// otherwise, it's not a safe cast.
2688-
ctx.Diags
2689-
.diagnose(funcConv->getLoc(),
2690-
diag::converting_func_loses_global_actor, fromType,
2691-
toType, fromActor)
2692-
.warnUntilSwiftVersion(6);
2693-
return;
2688+
if (!toFnType->isAsync()) {
2689+
ctx.Diags
2690+
.diagnose(funcConv->getLoc(),
2691+
diag::converting_func_loses_global_actor,
2692+
fromType, toType, fromActor)
2693+
.warnUntilSwiftVersion(6);
2694+
return;
2695+
}
26942696
}
26952697
}
26962698

@@ -2756,19 +2758,21 @@ namespace {
27562758
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
27572759
case FunctionTypeIsolation::Kind::Erased:
27582760
diagnoseNonSendableParametersAndResult(
2759-
toFnType, /*downgradeToWarning=*/true);
2761+
toFnType, version::Version::getFutureMajorLanguageVersion());
27602762
break;
27612763

27622764
case FunctionTypeIsolation::Kind::GlobalActor: {
2763-
// Handled above by `safeToDropGlobalActor` check.
2765+
diagnoseNonSendableParametersAndResult(toFnType,
2766+
/*warnUntilSwiftMode*/ 6);
27642767
break;
27652768
}
27662769

27672770
case FunctionTypeIsolation::Kind::NonIsolated: {
27682771
// nonisolated synchronous <-> @concurrent
27692772
if (fromFnType->isAsync() != toFnType->isAsync()) {
27702773
diagnoseNonSendableParametersAndResult(
2771-
toFnType, /*downgradeToWarning=*/true);
2774+
toFnType,
2775+
version::Version::getFutureMajorLanguageVersion());
27722776
}
27732777
break;
27742778
}
@@ -2783,7 +2787,7 @@ namespace {
27832787
case FunctionTypeIsolation::Kind::Parameter:
27842788
case FunctionTypeIsolation::Kind::Erased:
27852789
diagnoseNonSendableParametersAndResult(
2786-
toFnType, /*downgradeToWarning=*/true);
2790+
toFnType, version::Version::getFutureMajorLanguageVersion());
27872791
break;
27882792

27892793
case FunctionTypeIsolation::Kind::NonIsolated: {
@@ -2793,7 +2797,8 @@ namespace {
27932797
// actor isolation.
27942798
if (fromFnType->isAsync()) {
27952799
diagnoseNonSendableParametersAndResult(
2796-
toFnType, /*downgradeToWarning=*/true);
2800+
toFnType,
2801+
version::Version::getFutureMajorLanguageVersion());
27972802
break;
27982803
}
27992804
// Runs on the actor.
@@ -2805,7 +2810,9 @@ namespace {
28052810
break;
28062811

28072812
case FunctionTypeIsolation::Kind::GlobalActor:
2808-
llvm_unreachable("invalid conversion");
2813+
diagnoseNonSendableParametersAndResult(
2814+
toFnType, version::Version::getFutureMajorLanguageVersion());
2815+
break;
28092816
}
28102817
break;
28112818
}

‎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)