Skip to content

Commit d780b62

Browse files
committed
Make sure that we capture the isolation of a defer function that
implicitly uses the isolation.
1 parent f244c8f commit d780b62

File tree

2 files changed

+89
-8
lines changed

2 files changed

+89
-8
lines changed

lib/Sema/TypeCheckCaptures.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class FindCapturedVars : public ASTWalker {
6666
DeclContext *CurDC;
6767
bool NoEscape, ObjC;
6868
bool HasGenericParamCaptures;
69+
bool HasUsesOfCurrentIsolation = false;
6970

7071
public:
7172
FindCapturedVars(SourceLoc CaptureLoc,
@@ -91,6 +92,10 @@ class FindCapturedVars : public ASTWalker {
9192
CapturedTypes);
9293
}
9394

95+
bool hasUsesOfCurrentIsolation() const {
96+
return HasUsesOfCurrentIsolation;
97+
}
98+
9499
bool hasGenericParamCaptures() const {
95100
return HasGenericParamCaptures;
96101
}
@@ -693,6 +698,31 @@ class FindCapturedVars : public ASTWalker {
693698
checkType(typeValue->getParamType(), E->getLoc());
694699
}
695700

701+
// Record that we saw an #isolation expression that hasn't been filled in.
702+
if (auto currentIsolation = dyn_cast<CurrentContextIsolationExpr>(E)) {
703+
if (!currentIsolation->getActor())
704+
HasUsesOfCurrentIsolation = true;
705+
}
706+
707+
// Record that we saw an apply of a function with caller isolation.
708+
if (auto apply = dyn_cast<ApplyExpr>(E)) {
709+
if (auto type = apply->getFn()->getType()) {
710+
if (auto fnType = type->getAs<AnyFunctionType>();
711+
fnType && fnType->getIsolation().isNonIsolatedCaller()) {
712+
HasUsesOfCurrentIsolation = true;
713+
}
714+
}
715+
}
716+
717+
// Look into caller-side default arguments.
718+
if (auto defArg = dyn_cast<DefaultArgumentExpr>(E)) {
719+
if (defArg->isCallerSide()) {
720+
if (auto callerSideExpr = defArg->getCallerSideDefaultExpr()) {
721+
callerSideExpr->walk(*this);
722+
}
723+
}
724+
}
725+
696726
return Action::Continue(E);
697727
}
698728

@@ -747,13 +777,18 @@ class FindCapturedVars : public ASTWalker {
747777
/// Given that a local function is isolated to the given var, should we
748778
/// force a capture of the var?
749779
static bool shouldCaptureIsolationInLocalFunc(AbstractFunctionDecl *AFD,
750-
VarDecl *var) {
780+
VarDecl *var,
781+
bool hasUsesOfCurrentIsolation) {
751782
assert(isa<ParamDecl>(var));
752783

753784
// Don't try to capture an isolated parameter of the function itself.
754785
if (var->getDeclContext() == AFD)
755786
return false;
756787

788+
// Force capture if we have uses of the isolation in the function body.
789+
if (hasUsesOfCurrentIsolation)
790+
return true;
791+
757792
// We only *need* to force a capture of the isolation in an async function
758793
// (in which case it's needed for executor switching) or if we're in the
759794
// mode that forces an executor check in all synchronous functions. But
@@ -792,7 +827,8 @@ CaptureInfo CaptureInfoRequest::evaluate(Evaluator &evaluator,
792827
auto actorIsolation = getActorIsolation(AFD);
793828
if (actorIsolation.getKind() == ActorIsolation::ActorInstance) {
794829
if (auto *var = actorIsolation.getActorInstance()) {
795-
if (shouldCaptureIsolationInLocalFunc(AFD, var))
830+
if (shouldCaptureIsolationInLocalFunc(AFD, var,
831+
finder.hasUsesOfCurrentIsolation()))
796832
finder.addCapture(CapturedValue(var, 0, AFD->getLoc()));
797833
}
798834
}

test/Concurrency/isolated_nonsending_isolation_macro_sil.swift renamed to test/Concurrency/isolation_macro_sil.swift

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -parse-as-library -emit-sil %s | %FileCheck %s
1+
// RUN: %target-swift-frontend -parse-as-library -emit-sil %s -module-name test | %FileCheck %s
22

33
// REQUIRES: concurrency
44

@@ -9,16 +9,18 @@ nonisolated(nonsending) func nonisolatedNonsending() async {
99

1010
func take(iso: (any Actor)?) {}
1111

12+
func takeDefaulted(iso: isolated (any Actor)? = #isolation) {}
13+
1214
// CHECK-LABEL: // nonisolatedNonsending()
1315
// CHECK-NEXT: // Isolation: caller_isolation_inheriting
14-
// CHECK-NEXT: sil hidden @$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
16+
// CHECK-NEXT: sil hidden @$s4test21nonisolatedNonsendingyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
1517
// CHECK: bb0([[ACTOR:%.*]] : $Optional<any Actor>):
1618
// CHECK: retain_value [[ACTOR]]
1719
// CHECK: debug_value [[ACTOR]], let, name "iso"
18-
// CHECK: [[FUNC:%.*]] = function_ref @$s39isolated_nonsending_isolation_macro_sil4take3isoyScA_pSg_tF : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
20+
// CHECK: [[FUNC:%.*]] = function_ref @$s4test4take3isoyScA_pSg_tF : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
1921
// CHECK: apply [[FUNC]]([[ACTOR]]) : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
2022
// CHECK: release_value [[ACTOR]]
21-
// CHECK: } // end sil function '$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF'
23+
// CHECK: } // end sil function '$s4test21nonisolatedNonsendingyyYaF'
2224

2325
// Check that we emit #isolation correctly in closures.
2426
// CHECK-LABEL: // closure #1 in containsClosure()
@@ -33,6 +35,49 @@ func containsClosure() {
3335
}
3436
}
3537

38+
// Check that we capture variables as necessary to emit #isolation
39+
// correctly in defer bodies.
40+
func deferWithIsolatedParam(_ iso: isolated (any Actor)) {
41+
defer {
42+
take(iso: #isolation)
43+
}
44+
do {}
45+
}
46+
// CHECK-LABEL: sil hidden @$s4test22deferWithIsolatedParamyyScA_pYiF :
47+
// CHECK: bb0(%0 : $any Actor)
48+
// CHECK: [[DEFER:%.*]] = function_ref @$s4test22deferWithIsolatedParamyyScA_pYiF6$deferL_yyF :
49+
// CHECK-NEXT: apply [[DEFER]](%0)
50+
51+
// CHECK-LABEL: sil private @$s4test22deferWithIsolatedParamyyScA_pYiF6$deferL_yyF :
52+
// CHECK: bb0(%0 : @closureCapture $any Actor):
53+
// CHECK: [[T0:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, %0
54+
// CHECK: [[FN:%.*]] = function_ref @$s4test4take3isoyScA_pSg_tF :
55+
// CHECK-NEXT: apply [[FN]]([[T0]])
56+
57+
// Check that that happens even with uses in caller-side default
58+
// arguments, which capture analysis was not previously walking into.
59+
func deferWithIsolatedParam_defaultedUse(_ iso: isolated (any Actor)) {
60+
defer {
61+
takeDefaulted()
62+
}
63+
do {}
64+
}
65+
66+
// CHECK-LABEL: sil hidden @$s4test35deferWithIsolatedParam_defaultedUseyyScA_pYiF :
67+
// CHECK: bb0(%0 : $any Actor):
68+
// CHECK: [[DEFER:%.*]] = function_ref @$s4test35deferWithIsolatedParam_defaultedUseyyScA_pYiF6$deferL_yyF :
69+
// CHECK-NEXT: apply [[DEFER]](%0)
70+
71+
// CHECK-LABEL: sil private @$s4test35deferWithIsolatedParam_defaultedUseyyScA_pYiF6$deferL_yyF :
72+
// CHECK: bb0(%0 : @closureCapture $any Actor):
73+
// CHECK: [[T0:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, %0
74+
// CHECK: [[FN:%.*]] = function_ref @$s4test13takeDefaulted3isoyScA_pSgYi_tF :
75+
// CHECK-NEXT: apply [[FN]]([[T0]])
76+
77+
// TODO: we can't currently call nonisolated(nonsending) functions in
78+
// defer bodies because they have to be async, but that should be
79+
// tested here as well.
80+
3681
// Check that we emit #isolation correctly in defer bodies.
3782
nonisolated(nonsending)
3883
func hasDefer() async {
@@ -41,7 +86,7 @@ func hasDefer() async {
4186
}
4287
do {}
4388
}
44-
// CHECK-LABEL: sil hidden @$s39isolated_nonsending_isolation_macro_sil8hasDeferyyYaF :
89+
// CHECK-LABEL: sil hidden @$s4test8hasDeferyyYaF :
4590
// CHECK: bb0(%0 : $Optional<any Actor>):
4691
// CHECK: // function_ref $defer
4792
// CHECK-NEXT: [[DEFER:%.*]] = function_ref
@@ -66,7 +111,7 @@ func hasNestedDefer() async {
66111
do {}
67112
}
68113

69-
// CHECK-LABEL: sil hidden @$s39isolated_nonsending_isolation_macro_sil14hasNestedDeferyyYaF :
114+
// CHECK-LABEL: sil hidden @$s4test14hasNestedDeferyyYaF :
70115
// CHECK: bb0(%0 : $Optional<any Actor>):
71116
// CHECK: // function_ref $defer #1 () in hasNestedDefer()
72117
// CHECK-NEXT: [[DEFER:%.*]] = function_ref

0 commit comments

Comments
 (0)