averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

Rewrite the type of nonisolated(nonsending) closures.

Browse Source 
The constraint solver does not reliably give closures a function type
that includes `nonisolated(noncaller)`, even when the immediate context
requires a conversion to such a type. We were trying to work around this
in SILGen, but the peephole only kicked in if the types matched exactly,
so a contextual conversion that e.g. added `throws` was still emitting
the closure as `@concurrent`, which is of course the wrong semantics.
It's relatively easy to avoid all this by just rewriting the closure's
type to include `nonisolated(nonsending)` at a point where we can reliably
decide that, and then SILGen doesn't have to peephole anything for
correctness.

Fixes rdar://155313349
This commit is contained in:
John McCall
2025-07-28 22:08:43 -04:00
parent 73209e1ea0
commit 787cca9e37
4 changed files with 71 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
lib/SILGen/SILGenExpr.cpp
View File

@@ -498,9 +498,6 @@ namespace {
emitFunctionCvtFromExecutionCallerToGlobalActor(FunctionConversionExpr *E,
SGFContext C);
RValue emitFunctionCvtForNonisolatedNonsendingClosureExpr(
FunctionConversionExpr *E, SGFContext C);
RValue visitActorIsolationErasureExpr(ActorIsolationErasureExpr *E,
SGFContext C);
RValue visitExtractFunctionIsolationExpr(ExtractFunctionIsolationExpr *E,
@@ -2037,44 +2034,6 @@ RValueEmitter::emitFunctionCvtToExecutionCaller(FunctionConversionExpr *e,
return RValue(SGF, e, destType, result);
}
RValue RValueEmitter::emitFunctionCvtForNonisolatedNonsendingClosureExpr(
FunctionConversionExpr *E, SGFContext C) {
// The specific AST pattern for this looks as follows:
//
// (function_conversion_expr type="nonisolated(nonsending) () async -> Void"
// (closure_expr type="() async -> ()" isolated_to_caller_isolation))
CanAnyFunctionType destType =
cast<FunctionType>(E->getType()->getCanonicalType());
auto subExpr = E->getSubExpr()->getSemanticsProvidingExpr();
// If we do not have a closure or if that closure is not caller isolation
// inheriting, bail.
auto *closureExpr = dyn_cast<ClosureExpr>(subExpr);
if (!closureExpr ||
!closureExpr->getActorIsolation().isCallerIsolationInheriting())
return RValue();
// Then grab our closure type... make sure it is non isolated and then make
// sure it is the same as our destType but with nonisolated.
CanAnyFunctionType closureType =
cast<FunctionType>(closureExpr->getType()->getCanonicalType());
if (!closureType->getIsolation().isNonIsolated() ||
closureType !=
destType->withIsolation(FunctionTypeIsolation::forNonIsolated())
->getCanonicalType())
return RValue();
// NOTE: This is a partial inline of getClosureTypeInfo. We do this so we have
// more control and make this change less viral in the compiler for 6.2.
auto newExtInfo = closureType->getExtInfo().withIsolation(
FunctionTypeIsolation::forNonIsolatedCaller());
closureType = closureType.withExtInfo(newExtInfo);
auto info = SGF.getFunctionTypeInfo(closureType);
auto closure = emitClosureReference(closureExpr, info);
return RValue(SGF, closureExpr, destType, closure);
}
RValue RValueEmitter::emitFunctionCvtFromExecutionCallerToGlobalActor(
FunctionConversionExpr *e, SGFContext C) {
// We are pattern matching a conversion sequence like the following:
@@ -2187,26 +2146,7 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
// convention.
auto subExpr = e->getSubExpr()->getSemanticsProvidingExpr();
// Before we go any further into emitting the convert function expr, see if
// our SubExpr is a ClosureExpr with the exact same type as our
// FunctionConversionExpr except with the FunctionConversionExpr adding
// nonisolated(nonsending). Then see if the ClosureExpr itself (even though it
// is not nonisolated(nonsending) typed is considered to have
// nonisolated(nonsending) isolation. In such a case, emit the closure
// directly. We are going to handle it especially in closure emission to work
// around the missing information in the type.
//
// DISCUSSION: We need to do this here since in the Expression TypeChecker we
// do not have access to capture information when we would normally want to
// mark the closure type as being nonisolated(nonsending). As a result, we
// cannot know if the nonisolated(nonsending) should be overridden by for
// example an actor that is captured by the closure. So to work around this in
// Sema, we still mark the ClosureExpr as having the appropriate isolation
// even though its type does not have it... and then we work around this here
// and also in getClosureTypeInfo.
if (destType->getIsolation().isNonIsolatedCaller())
if (auto rv = emitFunctionCvtForNonisolatedNonsendingClosureExpr(e, C))
return rv;
// Look through `as` type ascriptions that don't induce bridging too.
while (auto subCoerce = dyn_cast<CoerceExpr>(subExpr)) {

17
lib/Sema/TypeCheckConcurrency.cpp
View File

@@ -3346,7 +3346,22 @@ namespace {
}
if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
closure->setActorIsolation(determineClosureIsolation(closure));
auto isolation = determineClosureIsolation(closure);
closure->setActorIsolation(isolation);
// There is a case in which the constraint solver cannot decide
// that a closure is `nonisolated(nonsending)` because it cannot
// analyze the captures, but the closure isolation logic can.
// Rewrite the closure type at this point.
if (isolation.isCallerIsolationInheriting()) {
auto fnType = closure->getType()->castTo<AnyFunctionType>();
if (!fnType->getIsolation().isNonIsolatedCaller()) {
fnType = fnType->withIsolation(
FunctionTypeIsolation::forNonIsolatedCaller());
closure->setType(fnType);
}
}
checkLocalCaptures(closure);
contextStack.push_back(closure);
return Action::Continue(expr);

6
test/Concurrency/attr_execution/attr_execution.swift
View File

@@ -18,7 +18,7 @@ func callerTest() async {}
struct Test {
// CHECK-LABEL: // closure #1 in variable initialization expression of Test.x
// CHECK: // Isolation: caller_isolation_inheriting
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
var x: () async -> Void = {}
// CHECK-LABEL: // Test.test()
@@ -67,7 +67,7 @@ func takesClosure(fn: () async -> Void) {
}
// CHECK-LABEL: sil hidden [ossa] @$s14attr_execution11testClosureyyF : $@convention(thin) () -> ()
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: [[THUNKED_CLOSURE:%.*]] = thin_to_thick_function %0 to $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// 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>) -> ()) -> ()
// CHECK: apply [[TAKES_CLOSURE]]([[THUNKED_CLOSURE]])
@@ -75,7 +75,7 @@ func takesClosure(fn: () async -> Void) {
// CHECK-LABEL: // closure #1 in testClosure()
// CHECK: // Isolation: caller_isolation_inheriting
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
func testClosure() {
takesClosure {
}

77
test/Concurrency/attr_execution/conversions_silgen.swift
View File

@@ -263,15 +263,6 @@ public func testConcurrentCallerLocalVariables(_ x: @escaping @concurrent () asy
// CHECK: } // end sil function '$s21attr_execution_silgen22globalActorConversionsyyyyYac_yyYaYCctYaF'
// 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>) -> ()) -> () {
// FIVE: bb0([[FUNC:%.*]] : @guaranteed
// FIVE: [[ACTOR:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@thick MainActor.Type) -> @owned MainActor
// FIVE: [[E:%.*]] = init_existential_ref [[ACTOR]] : $MainActor : $MainActor, $any Actor
// FIVE: [[E_OPT:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, [[E]]
// FIVE: hop_to_executor [[E_OPT]]
// FIVE: apply [[FUNC]]([[E_OPT]]) : $@async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// FIVE: } // end sil function '$sScA_pSgIegHgIL_IegH_TRScMTU'
// 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>) -> ()) -> () {
// SIX: bb0([[FUNC:%.*]] : $@convention(thin) @async (@sil_isolated
// SIX: [[ACTOR:%.*]] = apply {{%.*}}({{%.*}}) : $@convention(method) (@thick MainActor.Type) -> @owned MainActor
@@ -515,7 +506,7 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
}
func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
// CHECK: hop_to_executor [[EXECUTOR]]
let _: nonisolated(nonsending) () async -> Void = {
@@ -524,23 +515,10 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
// 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 {
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
// CHECK: hop_to_executor [[EXECUTOR]]
// CHECK: } // end sil function '$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_'
// 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>) -> ()) -> () {
// FIVE: bb0([[FUNC:%.*]] : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()):
// FIVE: [[ACTOR:%.*]] = enum $Optional<any Actor>, #Optional.none!enumelt
// FIVE: hop_to_executor [[ACTOR]]
// FIVE: apply [[FUNC]]([[ACTOR]])
// FIVE: } // end sil function '$sScA_pSgIetHgIL_IegH_TR'
// 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 {
// CHECK: bb0([[ACTOR:%.*]] : @guaranteed $Optional<any Actor>, [[FUNC:%.*]] : @guaranteed $@noescape @async @callee_guaranteed () -> ()):
// CHECK: apply [[FUNC]]()
// CHECK: hop_to_executor [[ACTOR]]
// CHECK: } // end sil function '$sIgH_ScA_pSgs5Error_pIegHgILzo_TR'
// CHECK: } // end sil function '$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_'
testParam { 42 }
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU1_ : $@convention(thin) @async () -> ()
@@ -548,7 +526,13 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
testParam { @concurrent in 42 }
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
// 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 {
// CHECK: bb0([[ACTOR:%.*]] : @guaranteed $Optional<any Actor>, [[FUNC:%.*]] : @guaranteed $@noescape @async @callee_guaranteed () -> ()):
// CHECK: apply [[FUNC]]()
// CHECK: hop_to_executor [[ACTOR]]
// CHECK: } // end sil function '$sIgH_ScA_pSgs5Error_pIegHgILzo_TR'
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
// CHECK: hop_to_executor [[EXECUTOR]]
fn = { _ in }
@@ -588,3 +572,44 @@ func testClosuresDontAssumeGlobalActorWithMarkedAsConcurrent() {
test { @Sendable @concurrent in
}
}
nonisolated(nonsending)
public func takesCallerIsolatedThrowingFunction<T>(
_ operation: nonisolated(nonsending) () async throws -> T
) async rethrows -> T {
try await operation()
}
func observe() {}
// Test that we emit closures with nonisolated(nonsending) isolation without
// introducing an intermediate @concurrent closure function.
func testConvertToThrowing(isolation: isolated (any Actor)? = #isolation) async {
// CHECK-LABEL: sil hidden [ossa] @$s21attr_execution_silgen21testConvertToThrowing9isolationyScA_pSgYi_tYaF :
// CHECK: [[ACTOR_COPY:%.*]] = copy_value %0
// CHECK-NEXT: [[ACTOR_BORROW:%.*]] = begin_borrow [[ACTOR_COPY]]
// CHECK-NEXT: hop_to_executor [[ACTOR_BORROW]]
// CHECK: [[CLOSURE:%.*]] = function_ref @$s21attr_execution_silgen21testConvertToThrowing9isolationyScA_pSgYi_tYaFyyYaYCXEfU_ :
// CHECK-NEXT: [[CLOSURE_VALUE:%.*]] = thin_to_thick_function [[CLOSURE]] to
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[FN:%.*]] = function_ref
// CHECK-NEXT: try_apply [[FN]]<()>({{%.*}}, {{%.*}}, [[CLOSURE_VALUE]]) {{.*}}, normal bb1, error bb2
// CHECK: bb1(
// This hop is unnecessary because nonisolated(nonsending) should
// preserve isolation on return.
// CHECK-NEXT: hop_to_executor [[ACTOR_BORROW]]
// 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 <()>
// CHECK: bb0(
// CHECK-NEXT: debug_value
// This hop is unnecessary because nonisolated(nonsending) should
// ensure isolation before call.
// CHECK-NEXT: hop_to_executor %1
// CHECK-NEXT: // function_ref observe()
// CHECK-NEXT: [[FN:%.*]] = function_ref @$s21attr_execution_silgen7observeyyF :
// CHECK-NEXT: apply [[FN]]()
await takesCallerIsolatedThrowingFunction {
observe()
}
}