Skip to content

Commit 3de72c5

Browse files
committed
[TypeChecker] Allow closures to assume nonisolated(nonsending)
Always infer `nonisolated(nonsending)` from context directly on a closure unless the closure is marked as `@concurrent`, otherwise the closure is not going to get correct isolation and going to hop to the wrong executor in its preamble. Resolves: rdar://149107104
1 parent aa2bb0c commit 3de72c5

File tree

3 files changed

+64
-6
lines changed

3 files changed

+64
-6
lines changed

lib/Sema/CSApply.cpp

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6514,18 +6514,31 @@ ArgumentList *ExprRewriter::coerceCallArguments(
65146514
return ArgumentList::createTypeChecked(ctx, args, newArgs);
65156515
}
65166516

6517-
/// Whether the given expression is a closure that should inherit
6518-
/// the actor context from where it was formed.
6519-
static bool closureInheritsActorContext(Expr *expr) {
6517+
/// Looks through any non-semantic expressions and a capture list
6518+
/// to find out whether the given expression is an explicit closure.
6519+
static ClosureExpr *isExplicitClosureExpr(Expr *expr) {
65206520
if (auto IE = dyn_cast<IdentityExpr>(expr))
6521-
return closureInheritsActorContext(IE->getSubExpr());
6521+
return isExplicitClosureExpr(IE->getSubExpr());
65226522

65236523
if (auto CLE = dyn_cast<CaptureListExpr>(expr))
6524-
return closureInheritsActorContext(CLE->getClosureBody());
6524+
return isExplicitClosureExpr(CLE->getClosureBody());
6525+
6526+
return dyn_cast<ClosureExpr>(expr);
6527+
}
65256528

6526-
if (auto CE = dyn_cast<ClosureExpr>(expr))
6529+
/// Whether the given expression is a closure that should inherit
6530+
/// the actor context from where it was formed.
6531+
static bool closureInheritsActorContext(Expr *expr) {
6532+
if (auto *CE = isExplicitClosureExpr(expr))
65276533
return CE->inheritsActorContext();
6534+
return false;
6535+
}
65286536

6537+
/// Determine whether the given expression is a closure that
6538+
/// is explicitly marked as `@concurrent`.
6539+
static bool isClosureMarkedAsConcurrent(Expr *expr) {
6540+
if (auto *CE = isExplicitClosureExpr(expr))
6541+
return CE->getAttrs().hasAttribute<ConcurrentAttr>();
65296542
return false;
65306543
}
65316544

@@ -7767,6 +7780,23 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
77677780
}
77687781
}
77697782

7783+
// If we have a ClosureExpr, then we can safely propagate the
7784+
// 'nonisolated(nonsending)' isolation if it's not explicitly
7785+
// marked as `@concurrent`.
7786+
if (toEI.getIsolation().isNonIsolatedCaller() &&
7787+
(fromEI.getIsolation().isNonIsolated() &&
7788+
!isClosureMarkedAsConcurrent(expr))) {
7789+
auto newFromFuncType = fromFunc->withIsolation(
7790+
FunctionTypeIsolation::forNonIsolatedCaller());
7791+
if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
7792+
fromFunc = newFromFuncType->castTo<FunctionType>();
7793+
// Propagating 'nonisolated(nonsending)' might have satisfied the entire
7794+
// conversion. If so, we're done, otherwise keep converting.
7795+
if (fromFunc->isEqual(toType))
7796+
return expr;
7797+
}
7798+
}
7799+
77707800
if (ctx.LangOpts.isDynamicActorIsolationCheckingEnabled()) {
77717801
// Passing a synchronous global actor-isolated function value and
77727802
// parameter that expects a synchronous non-isolated function type could

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4696,6 +4696,13 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
46964696
}
46974697
}
46984698

4699+
// `nonisolated(nonsending)` inferred from the context makes
4700+
// the closure caller isolated.
4701+
if (auto *closureTy = getType(closure)->getAs<FunctionType>()) {
4702+
if (closureTy->getIsolation().isNonIsolatedCaller())
4703+
return ActorIsolation::forCallerIsolationInheriting();
4704+
}
4705+
46994706
// If a closure has an isolated parameter, it is isolated to that
47004707
// parameter.
47014708
for (auto param : *closure->getParameters()) {

test/Concurrency/attr_execution/conversions_silgen.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,24 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
425425
let _: @concurrent (SendableKlass) async -> Void = y
426426
let _: @concurrent (NonSendableKlass) async -> Void = z
427427
}
428+
429+
func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
430+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
431+
let _: nonisolated(nonsending) () async -> Void = {
432+
42
433+
}
434+
435+
func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
436+
437+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> @error any Error
438+
testParam { 42 }
439+
440+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU1_ : $@convention(thin) @async () -> ()
441+
testParam { @concurrent in 42 }
442+
443+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()
444+
fn = { _ in }
445+
446+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU3_ : $@convention(thin) @async (Int) -> ()
447+
fn = { @concurrent _ in }
448+
}

0 commit comments

Comments
 (0)