Skip to content

Commit c3309d6

Browse files
committed
[region-isolation] Allow for Sendable global actor isolated closures to use transferred non-Sendable parameters.
This is safe since: 1. We transfer in the non-Sendable parameter into the global actor isolation region so we know that we will not use the non-Sendable paramter again except on that actor. 2. Since the closure is global actor isolated, we know that despite the fact that it is Sendable, it will only ever be executed serially on said global actor implying that we do not need to worry about different executions of the Sendable closure running concurrently with each other. rdar://125200006
1 parent 4272552 commit c3309d6

File tree

2 files changed

+30
-18
lines changed

2 files changed

+30
-18
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,30 +1718,32 @@ class PartitionOpTranslator {
17181718
}
17191719

17201720
// Now that we have transferred everything into the partial_apply, perform
1721-
// an assign fresh for the partial_apply. If we use any of the transferred
1722-
// values later, we will error, so it is safe to just create a new value.
1721+
// an assign fresh for the partial_apply if it is non-Sendable. If we use
1722+
// any of the transferred values later, we will error, so it is safe to just
1723+
// create a new value.
1724+
if (pai->getFunctionType()->isSendable())
1725+
return;
1726+
17231727
auto paiValue = tryToTrackValue(pai).value();
17241728
SILValue rep = paiValue.getRepresentative().getValue();
17251729
mergeIsolationRegionInfo(rep, actorIsolation);
17261730
translateSILAssignFresh(rep);
17271731
}
17281732

17291733
void translateSILPartialApply(PartialApplyInst *pai) {
1730-
// First check if our partial apply is Sendable. In such a case, we will
1731-
// have emitted an earlier warning in Sema.
1732-
//
1733-
// DISCUSSION: The reason why we can treat values passed into an async let
1734-
// as transferring safely but it is unsafe to do this for arbitrary Sendable
1735-
// closures is that we do not know how many times the Sendable closure will
1736-
// be executed. It is possible to have different invocations of the Sendable
1737-
// closure to cause races with the captured non-Sendable value. In contrast
1738-
// since we know an async let runs exactly once, we do not need to worry
1739-
// about such a possibility. If we had the ability in the language to
1740-
// specify that a closure is run at most once or that it is always run
1741-
// serially, we could lift this restriction... so for now we leave in the
1742-
// Sema warning and just bail here.
1743-
if (pai->getFunctionType()->isSendableType())
1744-
return;
1734+
// First check if our partial apply is Sendable and not global actor
1735+
// isolated. In such a case, we will have emitted an earlier warning in Sema
1736+
// and can return early. If we have a global actor isolated partial_apply,
1737+
// we can be looser and can use region isolation since we know that the
1738+
// Sendable closure will be executed serially due to the closure having to
1739+
// run on the global actor queue meaning that we do not have to worry about
1740+
// the Sendable closure being run concurrency.
1741+
if (pai->getFunctionType()->isSendableType()) {
1742+
auto isolationInfo = SILIsolationInfo::get(pai);
1743+
if (!isolationInfo || !isolationInfo.hasActorIsolation() ||
1744+
!isolationInfo.getActorIsolation().isGlobalActor())
1745+
return;
1746+
}
17451747

17461748
// Then check if our partial_apply is fed into an async let begin. If so,
17471749
// handle it especially.

test/Concurrency/transfernonsendable.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ func testSendableClosureCapturesNonSendable(a: MyActor) {
596596
func testSendableClosureCapturesNonSendable2(a: FinalMainActorIsolatedKlass) {
597597
let klass = NonSendableKlass()
598598
let _ = { @Sendable @MainActor in
599-
a.klass = klass // expected-warning {{capture of 'klass' with non-sendable type 'NonSendableKlass' in a `@Sendable` closure}}
599+
a.klass = klass // expected-complete-warning {{capture of 'klass' with non-sendable type 'NonSendableKlass' in a `@Sendable` closure}}
600600
}
601601
}
602602

@@ -1711,3 +1711,13 @@ func associatedTypeTestBasic2<T: AssociatedTypeTestProtocol>(_: T, iso: isolated
17111711
// expected-tns-note @-1 {{sending 'iso'-isolated 'x' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and 'iso'-isolated uses}}
17121712
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
17131713
}
1714+
1715+
func sendableGlobalActorIsolated() {
1716+
let x = NonSendableKlass()
1717+
let _ = { @Sendable @MainActor in
1718+
print(x) // expected-tns-warning {{sending 'x' risks causing data races}}
1719+
// expected-tns-note @-1 {{'x' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses}}
1720+
// expected-complete-warning @-2 {{capture of 'x' with non-sendable type 'NonSendableKlass' in a `@Sendable` closure}}
1721+
}
1722+
print(x) // expected-tns-note {{access can happen concurrently}}
1723+
}

0 commit comments

Comments
 (0)