Skip to content

Commit 834b606

Browse files
authored
Correctly forward the implicit nonisolated(nonsending) parameter in thunks (#82705)
Also, I discovered that we don't apply nonisolated(nonsending) to function types in the new mode. That's one for a different patch. Fixes rdar://154401813 and rdar://154137740
1 parent 6e9c010 commit 834b606

File tree

6 files changed

+139
-14
lines changed

6 files changed

+139
-14
lines changed

lib/SILGen/SILGenAvailability.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,9 @@ void SILGenFunction::emitBackDeploymentThunk(SILDeclRef thunk) {
508508
SmallVector<ManagedValue, 4> params;
509509
SmallVector<ManagedValue, 4> indirectParams;
510510
SmallVector<ManagedValue, 4> indirectErrorResults;
511-
collectThunkParams(loc, params, &indirectParams, &indirectErrorResults);
511+
ManagedValue implicitIsolationParam;
512+
collectThunkParams(loc, params, &indirectParams, &indirectErrorResults,
513+
&implicitIsolationParam);
512514

513515
// Build up the list of arguments that we're going to invoke the real
514516
// function with.
@@ -519,6 +521,9 @@ void SILGenFunction::emitBackDeploymentThunk(SILDeclRef thunk) {
519521
for (auto indirectErrorResult : indirectErrorResults) {
520522
paramsForForwarding.emplace_back(indirectErrorResult.getLValueAddress());
521523
}
524+
if (implicitIsolationParam) {
525+
paramsForForwarding.emplace_back(implicitIsolationParam.forward(*this));
526+
}
522527

523528
for (auto param : params) {
524529
// We're going to directly call either the original function or the fallback

lib/SILGen/SILGenFunction.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2623,7 +2623,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
26232623
void collectThunkParams(
26242624
SILLocation loc, SmallVectorImpl<ManagedValue> &params,
26252625
SmallVectorImpl<ManagedValue> *indirectResultParams = nullptr,
2626-
SmallVectorImpl<ManagedValue> *indirectErrorParams = nullptr);
2626+
SmallVectorImpl<ManagedValue> *indirectErrorParams = nullptr,
2627+
ManagedValue *implicitIsolationParam = nullptr);
26272628

26282629
/// Build the type of a function transformation thunk.
26292630
CanSILFunctionType buildThunkType(CanSILFunctionType &sourceType,

lib/SILGen/SILGenPoly.cpp

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,8 @@ ManagedValue Transform::transformTuple(ManagedValue inputTuple,
987987
void SILGenFunction::collectThunkParams(
988988
SILLocation loc, SmallVectorImpl<ManagedValue> &params,
989989
SmallVectorImpl<ManagedValue> *indirectResults,
990-
SmallVectorImpl<ManagedValue> *indirectErrors) {
990+
SmallVectorImpl<ManagedValue> *indirectErrors,
991+
ManagedValue *implicitIsolation) {
991992
// Add the indirect results.
992993
for (auto resultTy : F.getConventions().getIndirectSILResultTypes(
993994
getTypeExpansionContext())) {
@@ -1029,8 +1030,12 @@ void SILGenFunction::collectThunkParams(
10291030
// If our thunk has an implicit param and we are being asked to forward it,
10301031
// to the callee, skip it. We are going to handle it especially later.
10311032
if (param.hasOption(SILParameterInfo::ImplicitLeading) &&
1032-
param.hasOption(SILParameterInfo::Isolated))
1033+
param.hasOption(SILParameterInfo::Isolated)) {
1034+
if (implicitIsolation)
1035+
*implicitIsolation = functionArgument;
10331036
continue;
1037+
}
1038+
10341039
params.push_back(functionArgument);
10351040
}
10361041
}
@@ -5463,9 +5468,18 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
54635468
options |= ThunkGenFlag::CalleeHasImplicitIsolatedParam;
54645469
}
54655470

5471+
// Collect the thunk parameters. We don't need to collect the indirect
5472+
// error parameter because it'll be stored as IndirectErrorResult, which
5473+
// gets used implicitly by emitApplyWithRethrow.
54665474
SmallVector<ManagedValue, 8> params;
54675475
SmallVector<ManagedValue, 4> indirectResultParams;
5468-
SGF.collectThunkParams(loc, params, &indirectResultParams);
5476+
ManagedValue implicitIsolationParam;
5477+
SGF.collectThunkParams(loc, params, &indirectResultParams,
5478+
/*indirect error params*/ nullptr,
5479+
&implicitIsolationParam);
5480+
5481+
assert(bool(options & ThunkGenFlag::ThunkHasImplicitIsolatedParam)
5482+
== implicitIsolationParam.isValid());
54695483

54705484
// Ignore the self parameter at the SIL level. IRGen will use it to
54715485
// recover type metadata.
@@ -5511,9 +5525,11 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
55115525
case FunctionTypeIsolation::Kind::NonIsolated:
55125526
break;
55135527

5528+
// For a function for caller isolation, we'll have to figure out what the
5529+
// output function's formal isolation is. This is quite doable, but we don't
5530+
// have to do it yet.
55145531
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
5515-
hopToIsolatedParameter = true;
5516-
break;
5532+
llvm_unreachable("synchronous function has caller isolation?");
55175533

55185534
// For a function with parameter isolation, we'll have to dig the
55195535
// argument out after translation but before making the call.
@@ -5595,12 +5611,15 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
55955611
/*mandatory*/false);
55965612
}
55975613

5598-
// If we are thunking a nonisolated caller to nonisolated or global actor, we
5599-
// need to load the actor.
5614+
// If the input function has caller isolation, we need to fill in that
5615+
// argument with the formal isolation of the output function.
56005616
if (options.contains(ThunkGenFlag::CalleeHasImplicitIsolatedParam)) {
56015617
auto outputIsolation = outputSubstType->getIsolation();
56025618
switch (outputIsolation.getKind()) {
56035619
case FunctionTypeIsolation::Kind::NonIsolated:
5620+
case FunctionTypeIsolation::Kind::Erased:
5621+
// Converting a caller-isolated function to @isolated(any) makes
5622+
// it @concurrent. In either case, emit a nil actor reference.
56045623
argValues.push_back(SGF.emitNonIsolatedIsolation(loc).getValue());
56055624
break;
56065625
case FunctionTypeIsolation::Kind::GlobalActor: {
@@ -5610,11 +5629,17 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
56105629
SGF.emitGlobalActorIsolation(loc, globalActor).getValue());
56115630
break;
56125631
}
5632+
case FunctionTypeIsolation::Kind::NonIsolatedCaller: {
5633+
argValues.push_back(implicitIsolationParam.getValue());
5634+
break;
5635+
}
56135636
case FunctionTypeIsolation::Kind::Parameter:
5614-
case FunctionTypeIsolation::Kind::Erased:
5615-
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
5637+
// This would require a conversion from a type with caller
5638+
// isolation to a type with parameter isolation, which is not
5639+
// currently allowed and probably won't ever be. Anyway, to
5640+
// implement it, we'd need to borrow the isolated parameter
5641+
// and wrap it up as an `Optional<any Actor>`.
56165642
llvm_unreachable("Should never see this");
5617-
break;
56185643
}
56195644
}
56205645

@@ -5965,7 +5990,9 @@ static void buildWithoutActuallyEscapingThunkBody(SILGenFunction &SGF,
59655990
SmallVector<ManagedValue, 8> params;
59665991
SmallVector<ManagedValue, 8> indirectResults;
59675992
SmallVector<ManagedValue, 1> indirectErrorResults;
5968-
SGF.collectThunkParams(loc, params, &indirectResults, &indirectErrorResults);
5993+
ManagedValue implicitIsolationParam;
5994+
SGF.collectThunkParams(loc, params, &indirectResults, &indirectErrorResults,
5995+
&implicitIsolationParam);
59695996

59705997
// Ignore the self parameter at the SIL level. IRGen will use it to
59715998
// recover type metadata.
@@ -5986,6 +6013,11 @@ static void buildWithoutActuallyEscapingThunkBody(SILGenFunction &SGF,
59866013
for (auto indirectError : indirectErrorResults)
59876014
argValues.push_back(indirectError.getLValueAddress());
59886015

6016+
// Forward the implicit isolation parameter.
6017+
if (implicitIsolationParam.isValid()) {
6018+
argValues.push_back(implicitIsolationParam.getValue());
6019+
}
6020+
59896021
// Add the rest of the arguments.
59906022
forwardFunctionArguments(SGF, loc, fnType, params, argValues);
59916023

@@ -6175,6 +6207,7 @@ ManagedValue SILGenFunction::getThunkedAutoDiffLinearMap(
61756207
return getThunkedResult();
61766208
thunk->setGenericEnvironment(genericEnv);
61776209

6210+
// FIXME: handle implicit isolation parameter here
61786211
SILGenFunction thunkSGF(SGM, *thunk, FunctionDC);
61796212
SmallVector<ManagedValue, 4> params;
61806213
SmallVector<ManagedValue, 4> thunkIndirectResults;
@@ -6523,6 +6556,7 @@ SILFunction *SILGenModule::getOrCreateCustomDerivativeThunk(
65236556
thunk->setGenericEnvironment(thunkGenericEnv);
65246557

65256558
SILGenFunction thunkSGF(*this, *thunk, customDerivativeFn->getDeclContext());
6559+
// FIXME: handle implicit isolation parameter here
65266560
SmallVector<ManagedValue, 4> params;
65276561
SmallVector<ManagedValue, 4> indirectResults;
65286562
SmallVector<ManagedValue, 1> indirectErrorResults;

lib/SILGen/SILGenType.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,8 +1298,9 @@ SILFunction *SILGenModule::emitDefaultOverride(SILDeclRef replacement,
12981298
SmallVector<ManagedValue, 4> params;
12991299
SmallVector<ManagedValue, 4> indirectResults;
13001300
SmallVector<ManagedValue, 4> indirectErrors;
1301+
ManagedValue implicitIsolationParam;
13011302
SGF.collectThunkParams(replacement.getDecl(), params, &indirectResults,
1302-
&indirectErrors);
1303+
&indirectErrors, &implicitIsolationParam);
13031304

13041305
auto self = params.back();
13051306

@@ -1315,6 +1316,11 @@ SILFunction *SILGenModule::emitDefaultOverride(SILDeclRef replacement,
13151316
for (auto result : indirectResults) {
13161317
args.push_back(result.forward(SGF));
13171318
}
1319+
// Indirect errors would go here, but we don't currently support
1320+
// throwing coroutines.
1321+
if (implicitIsolationParam.isValid()) {
1322+
args.push_back(implicitIsolationParam.forward(SGF));
1323+
}
13181324
for (auto param : params) {
13191325
args.push_back(param.forward(SGF));
13201326
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// RUN: %target-swift-frontend -emit-sil -parse-as-library -swift-version 6 -enable-upcoming-feature NonisolatedNonsendingByDefault %s | %FileCheck %s
2+
3+
// REQUIRES: swift_feature_NonisolatedNonsendingByDefault
4+
5+
protocol P: Sendable {
6+
func f() async
7+
}
8+
9+
// Our map closure (abstracted as returning T) returns a function
10+
// value, which requires it to be reabstracted to the most general
11+
// abstraction pattern, i.e. from
12+
// nonisolated(nonsending) () async -> ()
13+
// to
14+
// nonisolated(nonsending) () async -> U, where U == ()
15+
// Note that we preserve nonisolated(nonsending).
16+
//
17+
// The thunk code did not expect the output function type to be
18+
// nonisolated(nonsending), so it didn't handle and propagate the
19+
// implicit isolation argument correctly.
20+
func testPartialApplication(p: [any P]) async {
21+
_ = p.map { $0.f }
22+
}
23+
// CHECK-LABEL: sil private @$s22nonisolated_nonsending22testPartialApplication1pySayAA1P_pG_tYaFyyYaYbYCcAaD_pXEfU_ :
24+
// CHECK: function_ref @$sScA_pSgIeghHgIL_AAytIeghHgILr_TR :
25+
26+
// Reabstraction thunk from caller-isolated () -> () to caller-isolated () -> T
27+
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @$sScA_pSgIeghHgIL_AAytIeghHgILr_TR :
28+
// CHECK: bb0(%0 : $*(), %1 : $Optional<any Actor>, %2 : $@Sendable @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()):
29+
// CHECK-NEXT: apply %2(%1) :
30+
31+
func takesGenericAsyncFunction<T>(_ fn: nonisolated(nonsending) (T) async -> Void) {}
32+
33+
// This is a more direct test of the partial-application code above.
34+
// Note that we have to make the functions explicitly nonisolated(nonsending)
35+
// because NonisolatedNonsendingByDefault only applies to declarations,
36+
// not function types in the abstract.
37+
func testReabstractionPreservingCallerIsolation(fn: nonisolated(nonsending) (Int) async -> Void) {
38+
takesGenericAsyncFunction(fn)
39+
}
40+
// CHECK-LABEL: sil hidden @$s22nonisolated_nonsending42testReabstractionPreservingCallerIsolation2fnyySiYaYCXE_tF :
41+
// CHECK: bb0(%0 : $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()):
42+
// CHECK: [[THUNK:%.*]] = function_ref @$sScA_pSgSiIgHgILy_AASiIegHgILn_TR :
43+
44+
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @$sScA_pSgSiIgHgILy_AASiIegHgILn_TR :
45+
// CHECK: bb0(%0 : $Optional<any Actor>, %1 : $*Int, %2 : $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()):
46+
// CHECK-NEXT: %3 = load %1
47+
// CHECK-NEXT: apply %2(%0, %3)
48+
49+
func takesAsyncIsolatedAnyFunction(_ fn: @isolated(any) () async -> Void) {}
50+
func takesGenericAsyncIsolatedAnyFunction<T>(_ fn: @isolated(any) (T) async -> Void) {}
51+
52+
// These would be good to test, but we apparently reject this conversion.
53+
#if false
54+
55+
// The same bug, but converting to an @isolated(any) function type.
56+
func testConversionToIsolatedAny(fn: nonisolated(nonsending) () async -> Void) {
57+
takesAsyncIsolatedAnyFunction(fn)
58+
}
59+
60+
func testReabstractionToIsolatedAny(fn: nonisolated(nonsending) (Int) async -> Void) {
61+
takesGenericAsyncIsolatedAnyFunction(fn)
62+
}
63+
64+
#endif

test/SILGen/back_deployed_attr.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,18 @@ public func backDeployedCaller(_ s: inout S<Z>) {
8585
// CHECK: function_ref @$s11back_deploy1SV1xxvsTwb : $@convention(method) <τ_0_0> (@in τ_0_0, @inout S<τ_0_0>) -> ()
8686
s.x = Z()
8787
}
88+
89+
// The same bug from test/Concurrency/nonisolated_nonsending.swift also applied to
90+
// back-deployment thunks.
91+
92+
// CHECK-LABEL: sil non_abi [serialized] [back_deployed_thunk] [ossa] @$s11back_deploy0A29DeployedNonisolatedNonsendingSiyYaFTwb :
93+
@backDeployed(before: macOS 10.52)
94+
nonisolated(nonsending)
95+
public func backDeployedNonisolatedNonsending() async -> Int {
96+
// CHECK: bb0(%0 : @guaranteed $Optional<any Actor>):
97+
// CHECK: [[FALLBACK_FN:%.*]] = function_ref @$s11back_deploy0A29DeployedNonisolatedNonsendingSiyYaFTwB :
98+
// CHECK: apply [[FALLBACK_FN]](%0)
99+
// CHECK: [[SHIPPING_FN:%.*]] = function_ref @$s11back_deploy0A29DeployedNonisolatedNonsendingSiyYaF :
100+
// CHECK: apply [[SHIPPING_FN]](%0)
101+
return 0
102+
}

0 commit comments

Comments
 (0)