Skip to content

Commit f2f07f3

Browse files
committed
[region-isolation] When determining isolation from a class_method, check for global actor isolation first.
Before this change in the following code, we would say that message is isolated to the actor instead of the global actor isolation of the actor's method: ```swift class Message { ... } actor MessageHolder { @mainactor func hold(_ message: Message) { ... } } @mainactor func sendMessage() async { let messageHolder = MessageHolder() let message = Message() // We identified messageHolder.hold as being MessageHolder isolated // instead of main actor isolated. messageHolder.hold(message) Task { @mainactor in print(message) } } ``` I also used this as an opportunity to simplify the logic in this part of the code. Specifically, I had made it so that multiple interesting cases were handled in the same conditional statement in a manner that it made it hard to know which cases were actually being handled and why it was correct. Now I split that into two separate if statements with comments that make it clear what we are actually trying to pattern match against. rdar://130980933 (cherry picked from commit 0a06c00)
1 parent e14d3a3 commit f2f07f3

File tree

3 files changed

+57
-22
lines changed

3 files changed

+57
-22
lines changed

include/swift/AST/ActorIsolation.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ class ActorIsolation {
180180
return parameterIndex;
181181
}
182182

183+
/// Returns true if this actor-instance isolation appllies to the self
184+
/// parameter of a method.
185+
bool isActorInstanceForSelfParameter() const {
186+
return getActorInstanceParameter() == 0;
187+
}
188+
183189
bool isSILParsed() const { return silParsed; }
184190

185191
bool isActorIsolated() const {

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -603,40 +603,55 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
603603
bool isNonIsolatedUnsafe = exprAnalysis.hasNonisolatedUnsafe();
604604
{
605605
auto isolation = swift::getActorIsolation(dre->getDecl());
606-
if (isolation.isActorIsolated() &&
607-
(isolation.getKind() != ActorIsolation::ActorInstance ||
608-
isolation.getActorInstanceParameter() == 0)) {
609-
if (cmi->getOperand()->getType().isAnyActor()) {
606+
607+
if (isolation.isActorIsolated()) {
608+
// Check if we have a global actor and handle it appropriately.
609+
if (isolation.getKind() == ActorIsolation::GlobalActor) {
610+
bool localNonIsolatedUnsafe =
611+
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
612+
return SILIsolationInfo::getGlobalActorIsolated(
613+
cmi, isolation.getGlobalActor())
614+
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
615+
}
616+
617+
// In this case, we have an actor instance that is self.
618+
if (isolation.getKind() != ActorIsolation::ActorInstance &&
619+
isolation.isActorInstanceForSelfParameter()) {
620+
bool localNonIsolatedUnsafe =
621+
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
610622
return SILIsolationInfo::getActorInstanceIsolated(
611-
cmi, cmi->getOperand(),
612-
cmi->getOperand()
613-
->getType()
614-
.getNominalOrBoundGenericNominal());
623+
cmi, cmi->getOperand(),
624+
cmi->getOperand()
625+
->getType()
626+
.getNominalOrBoundGenericNominal())
627+
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
615628
}
616-
return SILIsolationInfo::getGlobalActorIsolated(
617-
cmi, isolation.getGlobalActor());
618629
}
619-
620-
isNonIsolatedUnsafe |= isolation.isNonisolatedUnsafe();
621630
}
622631

623632
if (auto type = dre->getType()->getNominalOrBoundGenericNominal()) {
624633
if (auto isolation = swift::getActorIsolation(type)) {
625-
if (isolation.isActorIsolated() &&
626-
(isolation.getKind() != ActorIsolation::ActorInstance ||
627-
isolation.getActorInstanceParameter() == 0)) {
628-
if (cmi->getOperand()->getType().isAnyActor()) {
634+
if (isolation.isActorIsolated()) {
635+
// Check if we have a global actor and handle it appropriately.
636+
if (isolation.getKind() == ActorIsolation::GlobalActor) {
637+
bool localNonIsolatedUnsafe =
638+
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
639+
return SILIsolationInfo::getGlobalActorIsolated(
640+
cmi, isolation.getGlobalActor())
641+
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
642+
}
643+
644+
// In this case, we have an actor instance that is self.
645+
if (isolation.getKind() != ActorIsolation::ActorInstance &&
646+
isolation.isActorInstanceForSelfParameter()) {
647+
bool localNonIsolatedUnsafe =
648+
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
629649
return SILIsolationInfo::getActorInstanceIsolated(
630650
cmi, cmi->getOperand(),
631651
cmi->getOperand()
632652
->getType()
633653
.getNominalOrBoundGenericNominal())
634-
.withUnsafeNonIsolated(isNonIsolatedUnsafe);
635-
}
636-
637-
if (auto globalIso = SILIsolationInfo::getGlobalActorIsolated(
638-
cmi, isolation.getGlobalActor())) {
639-
return globalIso.withUnsafeNonIsolated(isNonIsolatedUnsafe);
654+
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
640655
}
641656
}
642657
}

test/Concurrency/transfernonsendable.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ actor MyActor {
4040
func useSendableFunction(_: @Sendable () -> Void) {}
4141
func useNonSendableFunction(_: () -> Void) {}
4242
func doSomething() {}
43+
@MainActor func useKlassMainActor(_ x: NonSendableKlass) {}
4344
}
4445

4546
final actor FinalActor {
@@ -1799,3 +1800,16 @@ actor FunctionWithSendableResultAndIsolationActor {
17991800
return ""
18001801
}
18011802
}
1803+
1804+
@MainActor
1805+
func testThatGlobalActorTakesPrecedenceOverActorIsolationOnMethods() async {
1806+
let a = MyActor()
1807+
let ns = NonSendableKlass()
1808+
1809+
// 'ns' should be main actor isolated since useKlassMainActor is @MainActor
1810+
// isolated. Previously we would let MyActor take precedence here...
1811+
a.useKlassMainActor(ns)
1812+
1813+
// Meaning we would get an error here.
1814+
Task { @MainActor in print(ns) }
1815+
}

0 commit comments

Comments
 (0)