Skip to content

Commit d54e6ba

Browse files
committed
[sending] Just like @sendable, sending stops isolation inference along DeclContexts.
Without this, we were hitting weird behaviors in flow_isolation.swift only when compiling in Swift 5 mode. Consider the following example: ``` func fakeTask(@_inheritActorContext _ x: sending @escaping () async -> ()) {} actor MyActor { var x: Int = 5 var y: Int = 6 var hax: MyActor? = nil init() { fakeTask { // Warning! You can remove await // Error! Cannot access actor state in nonisolated closure. _ = await self.hax } } } ``` The reason why this was happening is that we were not understanding that the closure passed to fakeTask is not part of the init and has different isolation from the init (it is nonisolated but does not have access to the init's state). This then caused us to exit early and not properly process the isolation crossing point there. Now, we properly understand that a sending inheritActorContext closure (just like an @sendable closure) has this property in actor initializers.
1 parent ca6cde6 commit d54e6ba

File tree

1 file changed

+56
-15
lines changed

1 file changed

+56
-15
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,49 @@ static bool isSendableClosure(
702702
return false;
703703
}
704704

705+
/// Returns true if this closure acts as an inference boundary in the AST. An
706+
/// inference boundary is an expression in the AST where we newly infer
707+
/// isolation different from our parent decl context.
708+
///
709+
/// Examples:
710+
///
711+
/// 1. a @Sendable closure.
712+
/// 2. a closure literal passed to a sending parameter.
713+
///
714+
/// NOTE: This does not mean that it has nonisolated isolation since for
715+
/// instance one could define an @MainActor closure in a nonisolated
716+
/// function. That @MainActor closure would act as an Isolation Inference
717+
/// Boundary.
718+
///
719+
/// \arg forActorIsolation we currently have two slightly varying semantics
720+
/// here. If this is set, then we assuming that we are being called recursively
721+
/// while walking up a decl context path to determine the actor isolation of a
722+
/// closure. In such a case, we do not want to be a boundary if we should
723+
/// inheritActorContext. In other contexts though, we want to determine if the
724+
/// closure is part of an init or deinit. In such a case, we are walking up the
725+
/// decl context chain and we want to stop if we see a sending parameter since
726+
/// in such a case, the sending closure parameter is known to not be part of the
727+
/// init or deinit.
728+
static bool
729+
isIsolationInferenceBoundaryClosure(const AbstractClosureExpr *closure,
730+
bool forActorIsolation) {
731+
if (auto *ce = dyn_cast<ClosureExpr>(closure)) {
732+
if (!forActorIsolation) {
733+
// For example, one would along this path see if for flow sensitive
734+
// isolation the closure is part of an init or deinit.
735+
if (ce->isPassedToSendingParameter())
736+
return true;
737+
} else {
738+
// This is for actor isolation. If we have inheritActorContext though, we
739+
// do not want to do anything since we are part of our parent's isolation.
740+
if (!ce->inheritsActorContext() && ce->isPassedToSendingParameter())
741+
return true;
742+
}
743+
}
744+
745+
return isSendableClosure(closure, forActorIsolation);
746+
}
747+
705748
/// Add Fix-It text for the given nominal type to adopt Sendable.
706749
static void addSendableFixIt(
707750
const NominalTypeDecl *nominal, InFlightDiagnostic &diag, bool unchecked) {
@@ -1628,11 +1671,14 @@ bool ReferencedActor::isKnownToBeLocal() const {
16281671
const AbstractFunctionDecl *
16291672
swift::isActorInitOrDeInitContext(const DeclContext *dc) {
16301673
while (true) {
1631-
// Non-Sendable closures are considered part of the enclosing context.
1674+
// Stop looking if we hit an isolation inference boundary.
16321675
if (auto *closure = dyn_cast<AbstractClosureExpr>(dc)) {
1633-
if (isSendableClosure(closure, /*for actor isolation*/ false))
1676+
if (isIsolationInferenceBoundaryClosure(closure,
1677+
false /*is for actor isolation*/))
16341678
return nullptr;
16351679

1680+
// Otherwise, look through our closure at the closure's parent decl
1681+
// context.
16361682
dc = dc->getParent();
16371683
continue;
16381684
}
@@ -4202,22 +4248,17 @@ namespace {
42024248
.withPreconcurrency(preconcurrency);
42034249
}
42044250

4205-
// Sendable closures are nonisolated unless the closure has
4206-
// specifically opted into inheriting actor isolation.
4207-
if (isSendableClosure(closure, /*forActorIsolation=*/true))
4251+
// If we have a closure that acts as an isolation inference boundary, then
4252+
// we return that it is non-isolated.
4253+
//
4254+
// NOTE: Since we already checked for global actor isolated things, we
4255+
// know that all Sendable closures must be nonisolated. That is why it is
4256+
// safe to rely on this path to handle Sendable closures.
4257+
if (isIsolationInferenceBoundaryClosure(
4258+
closure, true /*is for closure isolation*/))
42084259
return ActorIsolation::forNonisolated(/*unsafe=*/false)
42094260
.withPreconcurrency(preconcurrency);
42104261

4211-
// If the explicit closure is used as a non-Sendable sending parameter,
4212-
// make it nonisolated by default instead of inferring its isolation from
4213-
// the context.
4214-
if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure)) {
4215-
if (!explicitClosure->inheritsActorContext() &&
4216-
explicitClosure->isPassedToSendingParameter())
4217-
return ActorIsolation::forNonisolated(/*unsafe=*/false)
4218-
.withPreconcurrency(preconcurrency);
4219-
}
4220-
42214262
// A non-Sendable closure gets its isolation from its context.
42224263
auto parentIsolation = getActorIsolationOfContext(
42234264
closure->getParent(), getClosureActorIsolation);

0 commit comments

Comments
 (0)