Skip to content

Commit 7c79a24

Browse files
committed
[region-isolation] Values that are captured by an actor isolated closures are transferred to that closure.
This commit makes it so that we treat values captured by an actor isolated closure as being transferred to that closure. I also introduced a new diagnostic for these warnings that puts the main warning on the capture point of the value so the user is able to see the actual capture that causes the transfer to occur: ```swift nonisolated func testLocal2() async { let l = NonSendableKlass() // This is not safe since we use l later. self.assumeIsolated { isolatedSelf in isolatedSelf.ns = l } useValue(l) // expected-note {{access here could race}} } ``` ``` test.swift:74:14: warning: main actor-isolated closure captures value of non-Sendable type 'NonSendableKlass' from nonisolated context; later accesses to value could race useValue(x) // expected-warning {{main actor-isolated closure captures value of non-Sendable type 'NonSendableKlass' from nonisolated context; later accesses to value could race}} ^ test.swift:76:12: note: access here could race useValue(x) // expected-note {{access here could race}} ^ ``` One thing to keep in mind is that if we have a function argument being captured in this way, we still emit the "call site passes `self`" error. I am going to begin cleaning that up in the next commit in this PR so that we emit a better error here. But it makes sense to split these into two separate commits since they are doing different things. rdar://121345525
1 parent 2ae159b commit 7c79a24

File tree

6 files changed

+300
-34
lines changed

6 files changed

+300
-34
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,9 @@ WARNING(regionbasedisolation_transfer_yields_race_no_isolation, none,
898898
WARNING(regionbasedisolation_transfer_yields_race_with_isolation, none,
899899
"transferring value of non-Sendable type %0 from %1 context to %2 context; later accesses could race",
900900
(Type, ActorIsolation, ActorIsolation))
901+
WARNING(regionbasedisolation_isolated_capture_yields_race, none,
902+
"%1 closure captures value of non-Sendable type %0 from %2 context; later accesses to value could race",
903+
(Type, ActorIsolation, ActorIsolation))
901904
WARNING(regionbasedisolation_transfer_yields_race_transferring_parameter, none,
902905
"transferring value of non-Sendable type %0 into transferring parameter; later accesses could race",
903906
(Type))

include/swift/AST/Expr.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3846,6 +3846,20 @@ class AbstractClosureExpr : public DeclContext, public Expr {
38463846
Bits.AbstractClosureExpr.Discriminator = InvalidDiscriminator;
38473847
}
38483848

3849+
/// If we find that a capture x of this AbstractClosureExpr belongs to a
3850+
/// different isolation domain than the closure, add an ApplyIsolationCrossing
3851+
/// to foundIsolationCrossing.
3852+
///
3853+
/// \p foundIsolationCrossings an out parameter that contains the
3854+
/// ApplyIsolationCrossing if any of the captures are isolation crossing and
3855+
/// the index of the capture in the capture array. We return the index since
3856+
/// all captures may not cross isolation boundaries and we may need to be able
3857+
/// to look up the corresponding capture at the SIL level by index.
3858+
void getIsolationCrossing(
3859+
SmallVectorImpl<
3860+
std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>>
3861+
&foundIsolationCrossings);
3862+
38493863
CaptureInfo getCaptureInfo() const { return Captures; }
38503864
void setCaptureInfo(CaptureInfo captures) { Captures = captures; }
38513865

lib/AST/Expr.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2927,3 +2927,55 @@ Type ThrownErrorDestination::getContextErrorType() const {
29272927
auto conversion = storage.get<Conversion *>();
29282928
return conversion->conversion->getType();
29292929
}
2930+
2931+
void AbstractClosureExpr::getIsolationCrossing(
2932+
SmallVectorImpl<std::tuple<CapturedValue, unsigned, ApplyIsolationCrossing>>
2933+
&foundIsolationCrossings) {
2934+
/// For each capture...
2935+
for (auto pair : llvm::enumerate(getCaptureInfo().getCaptures())) {
2936+
auto capture = pair.value();
2937+
2938+
// First check quickly if we have dynamic self metadata. This is Sendable
2939+
// data so we don't care about it.
2940+
if (capture.isDynamicSelfMetadata())
2941+
continue;
2942+
2943+
auto declIsolation = swift::getActorIsolation(capture.getDecl());
2944+
2945+
// Then see if we have an opaque value.
2946+
if (auto *opaqueValue = capture.getOpaqueValue()) {
2947+
continue;
2948+
}
2949+
2950+
// If our decl is actor isolated...
2951+
if (declIsolation.isActorIsolated()) {
2952+
// And our closure is also actor isolated, then add the apply isolation
2953+
// crossing if the actors are different.
2954+
if (actorIsolation.isActorIsolated()) {
2955+
if (declIsolation.getActor() == actorIsolation.getActor()) {
2956+
continue;
2957+
}
2958+
2959+
foundIsolationCrossings.emplace_back(
2960+
capture, pair.index(),
2961+
ApplyIsolationCrossing(declIsolation, actorIsolation));
2962+
continue;
2963+
}
2964+
2965+
// Otherwise, our closure is not actor isolated but our decl is actor
2966+
// isolated. Add it.
2967+
foundIsolationCrossings.emplace_back(
2968+
capture, pair.index(),
2969+
ApplyIsolationCrossing(declIsolation, actorIsolation));
2970+
continue;
2971+
}
2972+
2973+
if (!actorIsolation.isActorIsolated()) {
2974+
continue;
2975+
}
2976+
2977+
foundIsolationCrossings.emplace_back(
2978+
capture, pair.index(),
2979+
ApplyIsolationCrossing(declIsolation, actorIsolation));
2980+
}
2981+
}

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,31 @@ class PartitionOpTranslator {
15741574
}
15751575
}
15761576

1577+
/// Handles the semantics for SIL applies that cross isolation.
1578+
///
1579+
/// Semantically this causes all arguments of the applysite to be transferred.
1580+
void translateIsolatedPartialApply(PartialApplyInst *pai) {
1581+
ApplySite applySite(pai);
1582+
1583+
// For each argument operand.
1584+
for (auto &op : applySite.getArgumentOperands()) {
1585+
// See if we tracked it.
1586+
if (auto value = tryToTrackValue(op.get())) {
1587+
// If we are tracking it, transfer it and if it is actor derived, mark
1588+
// our partial apply as actor derived.
1589+
builder.addTransfer(value->getRepresentative().getValue(), &op);
1590+
}
1591+
}
1592+
1593+
// Now that we have transferred everything into the partial_apply, perform
1594+
// an assign fresh for the partial_apply. If we use any of the transferred
1595+
// values later, we will error, so it is safe to just create a new value.
1596+
auto paiValue = tryToTrackValue(pai).value();
1597+
SILValue rep = paiValue.getRepresentative().getValue();
1598+
markValueAsActorDerived(rep);
1599+
translateSILAssignFresh(rep);
1600+
}
1601+
15771602
void translateSILPartialApply(PartialApplyInst *pai) {
15781603
assert(!isIsolationBoundaryCrossingApply(pai));
15791604

@@ -1583,6 +1608,14 @@ class PartitionOpTranslator {
15831608
return translateSILPartialApplyAsyncLetBegin(pai);
15841609
}
15851610

1611+
// Check if our closure is isolated to a specific actor. If it is, we need
1612+
// to treat all of the captures as being transferred.
1613+
if (auto *ace = pai->getLoc().getAsASTNode<AbstractClosureExpr>()) {
1614+
if (ace->getActorIsolation().isActorIsolated()) {
1615+
return translateIsolatedPartialApply(pai);
1616+
}
1617+
}
1618+
15861619
SILMultiAssignOptions options;
15871620
for (auto arg : pai->getOperandValues()) {
15881621
if (auto value = tryToTrackValue(arg)) {

0 commit comments

Comments
 (0)