Skip to content

Commit cd2cc7f

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. (cherry picked from commit d54e6ba)
1 parent 215a0ad commit cd2cc7f

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
@@ -701,6 +701,49 @@ static bool isSendableClosure(
701701
return false;
702702
}
703703

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

1679+
// Otherwise, look through our closure at the closure's parent decl
1680+
// context.
16351681
dc = dc->getParent();
16361682
continue;
16371683
}
@@ -4195,22 +4241,17 @@ namespace {
41954241
.withPreconcurrency(preconcurrency);
41964242
}
41974243

4198-
// Sendable closures are nonisolated unless the closure has
4199-
// specifically opted into inheriting actor isolation.
4200-
if (isSendableClosure(closure, /*forActorIsolation=*/true))
4244+
// If we have a closure that acts as an isolation inference boundary, then
4245+
// we return that it is non-isolated.
4246+
//
4247+
// NOTE: Since we already checked for global actor isolated things, we
4248+
// know that all Sendable closures must be nonisolated. That is why it is
4249+
// safe to rely on this path to handle Sendable closures.
4250+
if (isIsolationInferenceBoundaryClosure(
4251+
closure, true /*is for closure isolation*/))
42014252
return ActorIsolation::forNonisolated(/*unsafe=*/false)
42024253
.withPreconcurrency(preconcurrency);
42034254

4204-
// If the explicit closure is used as a non-Sendable sending parameter,
4205-
// make it nonisolated by default instead of inferring its isolation from
4206-
// the context.
4207-
if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure)) {
4208-
if (!explicitClosure->inheritsActorContext() &&
4209-
explicitClosure->isPassedToSendingParameter())
4210-
return ActorIsolation::forNonisolated(/*unsafe=*/false)
4211-
.withPreconcurrency(preconcurrency);
4212-
}
4213-
42144255
// A non-Sendable closure gets its isolation from its context.
42154256
auto parentIsolation = getActorIsolationOfContext(
42164257
closure->getParent(), getClosureActorIsolation);

0 commit comments

Comments
 (0)