Skip to content

Commit 7542f2f

Browse files
rjmccallrjmccall
authored
Merge pull request #83385 from rjmccall/rewrite-nonisolated-nonsending-closure-types
Rewrite the type of `nonisolated(nonsending)` closures.
2 parents 4148357 + 92b9857 commit 7542f2f

File tree

4 files changed

+70
-90
lines changed

4 files changed

+70
-90
lines changed

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 1 addition & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -498,9 +498,6 @@ namespace {
498498
emitFunctionCvtFromExecutionCallerToGlobalActor(FunctionConversionExpr *E,
499499
SGFContext C);
500500

501-
RValue emitFunctionCvtForNonisolatedNonsendingClosureExpr(
502-
FunctionConversionExpr *E, SGFContext C);
503-
504501
RValue visitActorIsolationErasureExpr(ActorIsolationErasureExpr *E,
505502
SGFContext C);
506503
RValue visitExtractFunctionIsolationExpr(ExtractFunctionIsolationExpr *E,
@@ -2035,44 +2032,6 @@ RValueEmitter::emitFunctionCvtToExecutionCaller(FunctionConversionExpr *e,
20352032
return RValue(SGF, e, destType, result);
20362033
}
20372034

2038-
RValue RValueEmitter::emitFunctionCvtForNonisolatedNonsendingClosureExpr(
2039-
FunctionConversionExpr *E, SGFContext C) {
2040-
// The specific AST pattern for this looks as follows:
2041-
//
2042-
// (function_conversion_expr type="nonisolated(nonsending) () async -> Void"
2043-
// (closure_expr type="() async -> ()" isolated_to_caller_isolation))
2044-
CanAnyFunctionType destType =
2045-
cast<FunctionType>(E->getType()->getCanonicalType());
2046-
auto subExpr = E->getSubExpr()->getSemanticsProvidingExpr();
2047-
2048-
// If we do not have a closure or if that closure is not caller isolation
2049-
// inheriting, bail.
2050-
auto *closureExpr = dyn_cast<ClosureExpr>(subExpr);
2051-
if (!closureExpr ||
2052-
!closureExpr->getActorIsolation().isCallerIsolationInheriting())
2053-
return RValue();
2054-
2055-
// Then grab our closure type... make sure it is non isolated and then make
2056-
// sure it is the same as our destType but with nonisolated.
2057-
CanAnyFunctionType closureType =
2058-
cast<FunctionType>(closureExpr->getType()->getCanonicalType());
2059-
if (!closureType->getIsolation().isNonIsolated() ||
2060-
closureType !=
2061-
destType->withIsolation(FunctionTypeIsolation::forNonIsolated())
2062-
->getCanonicalType())
2063-
return RValue();
2064-
2065-
// NOTE: This is a partial inline of getClosureTypeInfo. We do this so we have
2066-
// more control and make this change less viral in the compiler for 6.2.
2067-
auto newExtInfo = closureType->getExtInfo().withIsolation(
2068-
FunctionTypeIsolation::forNonIsolatedCaller());
2069-
closureType = closureType.withExtInfo(newExtInfo);
2070-
auto info = SGF.getFunctionTypeInfo(closureType);
2071-
2072-
auto closure = emitClosureReference(closureExpr, info);
2073-
return RValue(SGF, closureExpr, destType, closure);
2074-
}
2075-
20762035
RValue RValueEmitter::emitFunctionCvtFromExecutionCallerToGlobalActor(
20772036
FunctionConversionExpr *e, SGFContext C) {
20782037
// We are pattern matching a conversion sequence like the following:
@@ -2185,26 +2144,7 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21852144
// convention.
21862145
auto subExpr = e->getSubExpr()->getSemanticsProvidingExpr();
21872146

2188-
// Before we go any further into emitting the convert function expr, see if
2189-
// our SubExpr is a ClosureExpr with the exact same type as our
2190-
// FunctionConversionExpr except with the FunctionConversionExpr adding
2191-
// nonisolated(nonsending). Then see if the ClosureExpr itself (even though it
2192-
// is not nonisolated(nonsending) typed is considered to have
2193-
// nonisolated(nonsending) isolation. In such a case, emit the closure
2194-
// directly. We are going to handle it especially in closure emission to work
2195-
// around the missing information in the type.
2196-
//
2197-
// DISCUSSION: We need to do this here since in the Expression TypeChecker we
2198-
// do not have access to capture information when we would normally want to
2199-
// mark the closure type as being nonisolated(nonsending). As a result, we
2200-
// cannot know if the nonisolated(nonsending) should be overridden by for
2201-
// example an actor that is captured by the closure. So to work around this in
2202-
// Sema, we still mark the ClosureExpr as having the appropriate isolation
2203-
// even though its type does not have it... and then we work around this here
2204-
// and also in getClosureTypeInfo.
2205-
if (destType->getIsolation().isNonIsolatedCaller())
2206-
if (auto rv = emitFunctionCvtForNonisolatedNonsendingClosureExpr(e, C))
2207-
return rv;
2147+
22082148

22092149
// Look through `as` type ascriptions that don't induce bridging too.
22102150
while (auto subCoerce = dyn_cast<CoerceExpr>(subExpr)) {

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3329,7 +3329,22 @@ namespace {
33293329
}
33303330

33313331
if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
3332-
closure->setActorIsolation(determineClosureIsolation(closure));
3332+
auto isolation = determineClosureIsolation(closure);
3333+
closure->setActorIsolation(isolation);
3334+
3335+
// There is a case in which the constraint solver cannot decide
3336+
// that a closure is `nonisolated(nonsending)` because it cannot
3337+
// analyze the captures, but the closure isolation logic can.
3338+
// Rewrite the closure type at this point.
3339+
if (isolation.isCallerIsolationInheriting()) {
3340+
auto fnType = closure->getType()->castTo<AnyFunctionType>();
3341+
if (!fnType->getIsolation().isNonIsolatedCaller()) {
3342+
fnType = fnType->withIsolation(
3343+
FunctionTypeIsolation::forNonIsolatedCaller());
3344+
closure->setType(fnType);
3345+
}
3346+
}
3347+
33333348
checkLocalCaptures(closure);
33343349
contextStack.push_back(closure);
33353350
return Action::Continue(expr);

‎test/Concurrency/attr_execution/attr_execution.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func callerTest() async {}
1818
struct Test {
1919
// CHECK-LABEL: // closure #1 in variable initialization expression of Test.x
2020
// CHECK: // Isolation: caller_isolation_inheriting
21-
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
21+
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
2222
var x: () async -> Void = {}
2323

2424
// CHECK-LABEL: // Test.test()
@@ -67,15 +67,15 @@ func takesClosure(fn: () async -> Void) {
6767
}
6868

6969
// CHECK-LABEL: sil hidden [ossa] @$s14attr_execution11testClosureyyF : $@convention(thin) () -> ()
70-
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
70+
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7171
// CHECK: [[THUNKED_CLOSURE:%.*]] = thin_to_thick_function %0 to $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7272
// CHECK: [[TAKES_CLOSURE:%.*]] = function_ref @$s14attr_execution12takesClosure2fnyyyYaYCXE_tF : $@convention(thin) (@guaranteed @noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> ()
7373
// CHECK: apply [[TAKES_CLOSURE]]([[THUNKED_CLOSURE]])
7474
// CHECK: } // end sil function '$s14attr_execution11testClosureyyF'
7575

7676
// CHECK-LABEL: // closure #1 in testClosure()
7777
// CHECK: // Isolation: caller_isolation_inheriting
78-
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
78+
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7979
func testClosure() {
8080
takesClosure {
8181
}

‎test/Concurrency/attr_execution/conversions_silgen.swift

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -263,15 +263,6 @@ public func testConcurrentCallerLocalVariables(_ x: @escaping @concurrent () asy
263263

264264
// CHECK: } // end sil function '$s21attr_execution_silgen22globalActorConversionsyyyyYac_yyYaYCctYaF'
265265

266-
// FIVE-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sScA_pSgIegHgIL_IegH_TRScMTU : $@convention(thin) @async (@guaranteed @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> () {
267-
// FIVE: bb0([[FUNC:%.*]] : @guaranteed
268-
// FIVE: [[ACTOR:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@thick MainActor.Type) -> @owned MainActor
269-
// FIVE: [[E:%.*]] = init_existential_ref [[ACTOR]] : $MainActor : $MainActor, $any Actor
270-
// FIVE: [[E_OPT:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, [[E]]
271-
// FIVE: hop_to_executor [[E_OPT]]
272-
// FIVE: apply [[FUNC]]([[E_OPT]]) : $@async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
273-
// FIVE: } // end sil function '$sScA_pSgIegHgIL_IegH_TRScMTU'
274-
275266
// SIX-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sScA_pSgIetHgIL_IeghH_TRScMTU : $@convention(thin) @Sendable @async (@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> () {
276267
// SIX: bb0([[FUNC:%.*]] : $@convention(thin) @async (@sil_isolated
277268
// SIX: [[ACTOR:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@thick MainActor.Type) -> @owned MainActor
@@ -515,7 +506,7 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
515506
}
516507

517508
func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
518-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
509+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
519510
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
520511
// CHECK: hop_to_executor [[EXECUTOR]]
521512
let _: nonisolated(nonsending) () async -> Void = {
@@ -524,31 +515,24 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
524515

525516
func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
526517

527-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
518+
// 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 {
528519
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
529520
// CHECK: hop_to_executor [[EXECUTOR]]
530-
// CHECK: } // end sil function '$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_'
521+
// CHECK: } // end sil function '$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_'
522+
testParam { 42 }
531523

532-
// FIVE-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sScA_pSgIetHgIL_IegH_TR : $@convention(thin) @async (@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> () {
533-
// FIVE: bb0([[FUNC:%.*]] : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()):
534-
// FIVE: [[ACTOR:%.*]] = enum $Optional<any Actor>, #Optional.none!enumelt
535-
// FIVE: hop_to_executor [[ACTOR]]
536-
// FIVE: apply [[FUNC]]([[ACTOR]])
537-
// FIVE: } // end sil function '$sScA_pSgIetHgIL_IegH_TR'
524+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU1_ : $@convention(thin) @async () -> ()
525+
// CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
526+
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
527+
testParam { @concurrent in 42 }
538528

539529
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sIgH_ScA_pSgs5Error_pIegHgILzo_TR : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @guaranteed @noescape @async @callee_guaranteed () -> ()) -> @error any Error {
540530
// CHECK: bb0([[ACTOR:%.*]] : @guaranteed $Optional<any Actor>, [[FUNC:%.*]] : @guaranteed $@noescape @async @callee_guaranteed () -> ()):
541531
// CHECK: apply [[FUNC]]()
542532
// CHECK: hop_to_executor [[ACTOR]]
543533
// CHECK: } // end sil function '$sIgH_ScA_pSgs5Error_pIegHgILzo_TR'
544-
testParam { 42 }
545534

546-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU1_ : $@convention(thin) @async () -> ()
547-
// CHECK: [[GENERIC_EXECUTOR:%.*]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt
548-
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
549-
testParam { @concurrent in 42 }
550-
551-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
535+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
552536
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
553537
// CHECK: hop_to_executor [[EXECUTOR]]
554538
fn = { _ in }
@@ -588,3 +572,44 @@ func testClosuresDontAssumeGlobalActorWithMarkedAsConcurrent() {
588572
test { @Sendable @concurrent in
589573
}
590574
}
575+
576+
nonisolated(nonsending)
577+
public func takesCallerIsolatedThrowingFunction<T>(
578+
_ operation: nonisolated(nonsending) () async throws -> T
579+
) async rethrows -> T {
580+
try await operation()
581+
}
582+
583+
func observe() {}
584+
585+
// Test that we emit closures with nonisolated(nonsending) isolation without
586+
// introducing an intermediate @concurrent closure function.
587+
func testConvertToThrowing(isolation: isolated (any Actor)? = #isolation) async {
588+
// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen21testConvertToThrowing9isolationyScA_pSgYi_tYaF :
589+
// CHECK: [[ACTOR_COPY:%.*]] = copy_value %0
590+
// CHECK-NEXT: [[ACTOR_BORROW:%.*]] = begin_borrow [[ACTOR_COPY]]
591+
// CHECK-NEXT: hop_to_executor [[ACTOR_BORROW]]
592+
// CHECK: [[CLOSURE:%.*]] = function_ref @$s21attr_execution_silgen21testConvertToThrowing9isolationyScA_pSgYi_tYaFyyYaYCXEfU_ :
593+
// CHECK-NEXT: [[CLOSURE_VALUE:%.*]] = thin_to_thick_function [[CLOSURE]] to
594+
// CHECK-NEXT: // function_ref
595+
// CHECK-NEXT: [[FN:%.*]] = function_ref
596+
// CHECK-NEXT: try_apply [[FN]]<()>({{%.*}}, {{%.*}}, [[CLOSURE_VALUE]]) {{.*}}, normal bb1, error bb2
597+
// CHECK: bb1(
598+
// This hop is unnecessary because nonisolated(nonsending) should
599+
// preserve isolation on return.
600+
// CHECK-NEXT: hop_to_executor [[ACTOR_BORROW]]
601+
602+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen21testConvertToThrowing9isolationyScA_pSgYi_tYaFyyYaYCXEfU_ : $@convention(thin) @async @substituted <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> (@out τ_0_0, @error any Error) for <()>
603+
// CHECK: bb0(
604+
// CHECK-NEXT: debug_value
605+
// This hop is unnecessary because nonisolated(nonsending) should
606+
// ensure isolation before call.
607+
// CHECK-NEXT: hop_to_executor %1
608+
// CHECK-NEXT: // function_ref observe()
609+
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s21attr_execution_silgen7observeyyF :
610+
// CHECK-NEXT: apply [[FN]]()
611+
612+
await takesCallerIsolatedThrowingFunction {
613+
observe()
614+
}
615+
}

0 commit comments

Comments
 (0)