Skip to content

Commit 134f5de

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 (cherry picked from commit 3de72c5)
1 parent c18e231 commit 134f5de

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
@@ -6494,18 +6494,31 @@ ArgumentList *ExprRewriter::coerceCallArguments(
64946494
return ArgumentList::createTypeChecked(ctx, args, newArgs);
64956495
}
64966496

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

65036503
if (auto CLE = dyn_cast<CaptureListExpr>(expr))
6504-
return closureInheritsActorContext(CLE->getClosureBody());
6504+
return isExplicitClosureExpr(CLE->getClosureBody());
6505+
6506+
return dyn_cast<ClosureExpr>(expr);
6507+
}
65056508

6506-
if (auto CE = dyn_cast<ClosureExpr>(expr))
6509+
/// Whether the given expression is a closure that should inherit
6510+
/// the actor context from where it was formed.
6511+
static bool closureInheritsActorContext(Expr *expr) {
6512+
if (auto *CE = isExplicitClosureExpr(expr))
65076513
return CE->inheritsActorContext();
6514+
return false;
6515+
}
65086516

6517+
/// Determine whether the given expression is a closure that
6518+
/// is explicitly marked as `@concurrent`.
6519+
static bool isClosureMarkedAsConcurrent(Expr *expr) {
6520+
if (auto *CE = isExplicitClosureExpr(expr))
6521+
return CE->getAttrs().hasAttribute<ConcurrentAttr>();
65096522
return false;
65106523
}
65116524

@@ -7747,6 +7760,23 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
77477760
}
77487761
}
77497762

7763+
// If we have a ClosureExpr, then we can safely propagate the
7764+
// 'nonisolated(nonsending)' isolation if it's not explicitly
7765+
// marked as `@concurrent`.
7766+
if (toEI.getIsolation().isNonIsolatedCaller() &&
7767+
(fromEI.getIsolation().isNonIsolated() &&
7768+
!isClosureMarkedAsConcurrent(expr))) {
7769+
auto newFromFuncType = fromFunc->withIsolation(
7770+
FunctionTypeIsolation::forNonIsolatedCaller());
7771+
if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
7772+
fromFunc = newFromFuncType->castTo<FunctionType>();
7773+
// Propagating 'nonisolated(nonsending)' might have satisfied the entire
7774+
// conversion. If so, we're done, otherwise keep converting.
7775+
if (fromFunc->isEqual(toType))
7776+
return expr;
7777+
}
7778+
}
7779+
77507780
if (ctx.LangOpts.isDynamicActorIsolationCheckingEnabled()) {
77517781
// Passing a synchronous global actor-isolated function value and
77527782
// 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
@@ -4699,6 +4699,13 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
46994699
}
47004700
}
47014701

4702+
// `nonisolated(nonsending)` inferred from the context makes
4703+
// the closure caller isolated.
4704+
if (auto *closureTy = getType(closure)->getAs<FunctionType>()) {
4705+
if (closureTy->getIsolation().isNonIsolatedCaller())
4706+
return ActorIsolation::forCallerIsolationInheriting();
4707+
}
4708+
47024709
// If a closure has an isolated parameter, it is isolated to that
47034710
// parameter.
47044711
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)