Skip to content

Commit 13de9d6

Browse files
committed
Properly compute actor isolation of local functions with captures.
Non-Sendable local functions can be actor-isolated if they capture an isolated parameter. Treat them as such. Fixes rdar://83090397.
1 parent 61e9074 commit 13de9d6

File tree

8 files changed

+99
-45
lines changed

8 files changed

+99
-45
lines changed

include/swift/AST/CaptureInfo.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ namespace swift {
3636
class ValueDecl;
3737
class FuncDecl;
3838
class OpaqueValueExpr;
39+
class ParamDecl;
3940

4041
/// CapturedValue includes both the declaration being captured, along with flags
4142
/// that indicate how it is captured.
@@ -219,6 +220,9 @@ class CaptureInfo {
219220
return StorageAndFlags.getPointer()->getOpaqueValue();
220221
}
221222

223+
/// Retrieve the isolated parameter that has been captured, if there is one.
224+
ParamDecl *getIsolatedParamCapture() const;
225+
222226
SWIFT_DEBUG_DUMP;
223227
void print(raw_ostream &OS) const;
224228
};

lib/AST/CaptureInfo.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,29 @@ getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
7878
}
7979
}
8080

81+
ParamDecl *CaptureInfo::getIsolatedParamCapture() const {
82+
if (!hasLocalCaptures())
83+
return nullptr;
84+
85+
for (const auto &capture : getCaptures()) {
86+
if (!capture.getDecl()->isLocalCapture())
87+
continue;
88+
89+
if (capture.isDynamicSelfMetadata())
90+
continue;
91+
92+
auto param = dyn_cast_or_null<ParamDecl>(capture.getDecl());
93+
if (!param)
94+
continue;
95+
96+
// If we have captured an isolated parameter, return it.
97+
if (param->isIsolated())
98+
return param;
99+
}
100+
101+
return nullptr;
102+
}
103+
81104
LLVM_ATTRIBUTE_USED void CaptureInfo::dump() const {
82105
print(llvm::errs());
83106
llvm::errs() << '\n';

lib/SILGen/SILGenProlog.cpp

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,17 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
558558
!F.isAsync() &&
559559
!isInActorDestructor(FunctionDC);
560560

561+
// Local function to load the expected executor from a local actor
562+
auto loadExpectedExecutorForLocalVar = [&](VarDecl *var) {
563+
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
564+
Type actorType = var->getType();
565+
RValue actorInstanceRV = emitRValueForDecl(
566+
loc, var, actorType, AccessSemantics::Ordinary);
567+
ManagedValue actorInstance =
568+
std::move(actorInstanceRV).getScalarValue();
569+
ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
570+
};
571+
561572
if (auto *funcDecl =
562573
dyn_cast_or_null<AbstractFunctionDecl>(FunctionDC->getAsDecl())) {
563574
auto actorIsolation = getActorIsolation(funcDecl);
@@ -569,13 +580,7 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
569580
if (F.isAsync()) {
570581
for (auto param : *funcDecl->getParameters()) {
571582
if (param->isIsolated()) {
572-
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
573-
Type actorType = param->getType();
574-
RValue actorInstanceRV = emitRValueForDecl(
575-
loc, param, actorType, AccessSemantics::Ordinary);
576-
ManagedValue actorInstance =
577-
std::move(actorInstanceRV).getScalarValue();
578-
ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
583+
loadExpectedExecutorForLocalVar(param);
579584
break;
580585
}
581586
}
@@ -588,16 +593,21 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
588593
}
589594

590595
case ActorIsolation::ActorInstance: {
591-
assert(selfParam && "no self parameter for ActorInstance isolation");
592596
// Only produce an executor for actor-isolated functions that are async
593597
// or are local functions. The former require a hop, while the latter
594598
// are prone to dynamic data races in code that does not enforce Sendable
595599
// completely.
596600
if (F.isAsync() ||
597601
(wantDataRaceChecks && funcDecl->isLocalCapture())) {
598-
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
599-
ManagedValue selfArg = ManagedValue::forUnmanaged(F.getSelfArgument());
600-
ExpectedExecutor = emitLoadActorExecutor(loc, selfArg);
602+
if (auto isolatedParam = funcDecl->getCaptureInfo()
603+
.getIsolatedParamCapture()) {
604+
loadExpectedExecutorForLocalVar(isolatedParam);
605+
} else {
606+
assert(selfParam && "no self parameter for ActorInstance isolation");
607+
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
608+
ManagedValue selfArg = ManagedValue::forUnmanaged(F.getSelfArgument());
609+
ExpectedExecutor = emitLoadActorExecutor(loc, selfArg);
610+
}
601611
}
602612
break;
603613
}
@@ -619,14 +629,7 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
619629

620630
case ClosureActorIsolation::ActorInstance: {
621631
if (wantExecutor) {
622-
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
623-
auto actorDecl = actorIsolation.getActorInstance();
624-
Type actorType = actorDecl->getType();
625-
RValue actorInstanceRV = emitRValueForDecl(loc,
626-
actorDecl, actorType, AccessSemantics::Ordinary);
627-
ManagedValue actorInstance =
628-
std::move(actorInstanceRV).getScalarValue();
629-
ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
632+
loadExpectedExecutorForLocalVar(actorIsolation.getActorInstance());
630633
}
631634
break;
632635
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2640,7 +2640,7 @@ namespace {
26402640
if (isSendableClosure(closure, /*forActorIsolation=*/true))
26412641
return ClosureActorIsolation::forIndependent();
26422642

2643-
// A non-escaping closure gets its isolation from its context.
2643+
// A non-Sendable closure gets its isolation from its context.
26442644
auto parentIsolation = getActorIsolationOfContext(closure->getParent());
26452645

26462646
// We must have parent isolation determined to get here.
@@ -2658,25 +2658,9 @@ namespace {
26582658

26592659
case ActorIsolation::ActorInstance:
26602660
case ActorIsolation::DistributedActorInstance: {
2661-
SmallVector<CapturedValue, 2> localCaptures;
2662-
closure->getCaptureInfo().getLocalCaptures(localCaptures);
2663-
for (const auto &localCapture : localCaptures) {
2664-
if (localCapture.isDynamicSelfMetadata())
2665-
continue;
2666-
2667-
auto param = dyn_cast_or_null<ParamDecl>(localCapture.getDecl());
2668-
if (!param)
2669-
continue;
2670-
2671-
// If we have captured an isolated parameter, the closure is isolated
2672-
// to that actor instance.
2673-
if (param->isIsolated()) {
2674-
return ClosureActorIsolation::forActorInstance(param);
2675-
}
2676-
}
2661+
if (auto param = closure->getCaptureInfo().getIsolatedParamCapture())
2662+
return ClosureActorIsolation::forActorInstance(param);
26772663

2678-
// When no actor instance is not captured, this closure is
2679-
// actor-independent.
26802664
return ClosureActorIsolation::forIndependent();
26812665
}
26822666
}
@@ -3328,19 +3312,23 @@ ActorIsolation ActorIsolationRequest::evaluate(
33283312
return inferred;
33293313
};
33303314

3331-
// If this is a "defer" function body, inherit the global actor isolation
3332-
// from its context.
3315+
// If this is a local function, inherit the actor isolation from its
3316+
// context if it global or was captured.
33333317
if (auto func = dyn_cast<FuncDecl>(value)) {
3334-
if (func->isDeferBody()) {
3318+
if (func->isLocalCapture() && !func->isSendable()) {
33353319
switch (auto enclosingIsolation =
33363320
getActorIsolationOfContext(func->getDeclContext())) {
3337-
case ActorIsolation::ActorInstance:
3338-
case ActorIsolation::DistributedActorInstance:
33393321
case ActorIsolation::Independent:
33403322
case ActorIsolation::Unspecified:
33413323
// Do nothing.
33423324
break;
33433325

3326+
case ActorIsolation::ActorInstance:
3327+
case ActorIsolation::DistributedActorInstance:
3328+
if (auto param = func->getCaptureInfo().getIsolatedParamCapture())
3329+
return inferredIsolation(enclosingIsolation);
3330+
break;
3331+
33443332
case ActorIsolation::GlobalActor:
33453333
case ActorIsolation::GlobalActorUnsafe:
33463334
return inferredIsolation(enclosingIsolation);

test/Concurrency/actor_definite_init.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,6 @@ actor Rain {
447447

448448
defer { _ = self.x }
449449

450-
defer { Task { await self.f() } }
450+
defer { Task { self.f() } }
451451
}
452452
}

test/Concurrency/actor_definite_init_swift6.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,6 @@ actor Rain {
445445

446446
defer { _ = self.x }
447447

448-
defer { Task { await self.f() } }
448+
defer { Task { self.f() } }
449449
}
450450
}

test/Concurrency/actor_isolation.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,27 @@ func checkLocalFunctions() async {
639639
print(k)
640640
}
641641

642+
@available(SwiftStdlib 5.1, *)
643+
actor LocalFunctionIsolatedActor {
644+
func a() -> Bool { // expected-note{{calls to instance method 'a()' from outside of its actor context are implicitly asynchronous}}
645+
return true
646+
}
647+
648+
func b() -> Bool {
649+
func c() -> Bool {
650+
return true && a() // okay, c is isolated
651+
}
652+
return c()
653+
}
654+
655+
func b2() -> Bool {
656+
@Sendable func c() -> Bool {
657+
return true && a() // expected-error{{actor-isolated instance method 'a()' can not be referenced from a non-isolated context}}
658+
}
659+
return c()
660+
}
661+
}
662+
642663
// ----------------------------------------------------------------------
643664
// Lazy properties with initializers referencing 'self'
644665
// ----------------------------------------------------------------------

test/SILGen/check_executor.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,19 @@ public actor MyActor {
4646
return 5
4747
}
4848
}
49+
50+
// CHECK-CANONICAL-LABEL: sil private [ossa] @$s4test7MyActorC0A13LocalFunctionyyF5localL_SiyF : $@convention(thin) (@guaranteed MyActor) -> Int
51+
// CHECK-CANONICAL: [[CAPTURE:%.*]] = copy_value %0 : $MyActor
52+
// CHECK-CANONICAL-NEXT: [[BORROWED_CAPTURE:%.*]] = begin_borrow [[CAPTURE]] : $MyActor
53+
// CHECK-CANONICAL-NEXT: [[EXECUTOR:%.*]] = builtin "buildDefaultActorExecutorRef"<MyActor>([[BORROWED_CAPTURE]] : $MyActor) : $Builtin.Executor
54+
// CHECK-CANONICAL-NEXT: [[EXECUTOR_DEP:%.*]] = mark_dependence [[EXECUTOR]] : $Builtin.Executor on [[BORROWED_CAPTURE]] : $MyActor
55+
// CHECK-CANONICAL: [[CHECK_FN:%.*]] = function_ref @$ss22_checkExpectedExecutor14_filenameStart01_D6Length01_D7IsASCII5_line9_executoryBp_BwBi1_BwBetF : $@convention(thin) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, Builtin.Word, Builtin.Executor) -> ()
56+
// CHECK-CANONICAL-NEXT: apply [[CHECK_FN]]({{.*}}, [[EXECUTOR_DEP]])
57+
public func testLocalFunction() {
58+
func local() -> Int {
59+
return counter
60+
}
61+
62+
print(local())
63+
}
4964
}

0 commit comments

Comments
 (0)