Skip to content

Commit 51a2029

Browse files
committed
[sending] closure literals that are passed as sending parameters are now inferred to be nonisolated.
Consider the following piece of code and what the isolation is of the closure literal passed to doSomething(): ```swift func doSomething(_ f: sending () -> ()) { ... } @MyCustomActor func foo() async { doSomething { // What is the isolation here? } } ``` In this case, the isolation of the closure is @MyCustomActor. This is because non-Sendable closures are by default isolated to their current context (in this case @MyCustomActor since foo is @MyCustomActor isolated). This is a problem since 1. Our closure is a synchronous function that does not have the ability to hop to MyCustomActor to run said code. This could result in a concurrency hole caused by running the closure in doSomething() without hopping to MyCustomActor's executor. 2. In Region Based Isolation, a closure that is actor isolated cannot be sent, so we would immediately hit a region isolation error. To fix this issue, by default, if a closure literal is passed as a sending parameter, we make its isolation nonisolated. This ensures that it is disconnected and can be transferred safely. In the case of an async closure literal, we follow the same semantics, but we add an additional wrinkle: we keep support of inheritActorIsolation. If one marks an async closure literal with inheritActorIsolation, we allow for it to be passed as a sendable parameter since it is actually Sendable under the hood. (cherry picked from commit 3f39bdc)
1 parent 52ae003 commit 51a2029

File tree

7 files changed

+481
-16
lines changed

7 files changed

+481
-16
lines changed

docs/ReferenceGuides/UnderscoredAttributes.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -591,11 +591,14 @@ class C {
591591

592592
(Note that it is "inherit", not "inherits", unlike below.)
593593

594-
Marks that a `@Sendable async` closure argument should inherit the actor
595-
context (i.e. what actor it should be run on) based on the declaration site
596-
of the closure. This is different from the typical behavior, where the closure
597-
may be runnable anywhere unless its type specifically declares that it will
598-
run on a specific actor.
594+
Marks that a `@Sendable async` or `sendable async` closure argument should
595+
inherit the actor context (i.e. what actor it should be run on) based on the
596+
declaration site of the closure rather than be non-Sendable. This does not do
597+
anything if the closure is synchronous.
598+
599+
DISCUSSION: The reason why this does nothing when the closure is synchronous is
600+
since it does not have the ability to hop to the appropriate executor before it
601+
is run, so we may create concurrency errors.
599602

600603
## `@_inheritsConvenienceInitializers`
601604

include/swift/AST/Expr.h

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
266266
Kind : 2
267267
);
268268

269-
SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1,
269+
SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1,
270270
/// True if closure parameters were synthesized from anonymous closure
271271
/// variables.
272272
HasAnonymousClosureVars : 1,
@@ -281,7 +281,10 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
281281

282282
/// True if this closure's actor isolation behavior was determined by an
283283
/// \c \@preconcurrency declaration.
284-
IsolatedByPreconcurrency : 1
284+
IsolatedByPreconcurrency : 1,
285+
286+
/// True if this is a closure literal that is passed as a sending parameter.
287+
IsSendingParameter : 1
285288
);
286289

287290
SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
@@ -4126,6 +4129,7 @@ class ClosureExpr : public AbstractClosureExpr {
41264129
Bits.ClosureExpr.HasAnonymousClosureVars = false;
41274130
Bits.ClosureExpr.ImplicitSelfCapture = false;
41284131
Bits.ClosureExpr.InheritActorContext = false;
4132+
Bits.ClosureExpr.IsSendingParameter = false;
41294133
}
41304134

41314135
SourceRange getSourceRange() const;
@@ -4181,6 +4185,16 @@ class ClosureExpr : public AbstractClosureExpr {
41814185
Bits.ClosureExpr.IsolatedByPreconcurrency = value;
41824186
}
41834187

4188+
/// Whether the closure is a closure literal that is passed as a sending
4189+
/// parameter.
4190+
bool isSendingParameter() const {
4191+
return Bits.ClosureExpr.IsSendingParameter;
4192+
}
4193+
4194+
void setSendingParameter(bool value = true) {
4195+
Bits.ClosureExpr.IsSendingParameter = value;
4196+
}
4197+
41844198
/// Determine whether this closure expression has an
41854199
/// explicitly-specified result type.
41864200
bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

include/swift/AST/Types.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3837,6 +3837,7 @@ struct ParameterListInfo {
38373837
SmallBitVector implicitSelfCapture;
38383838
SmallBitVector inheritActorContext;
38393839
SmallBitVector variadicGenerics;
3840+
SmallBitVector isSending;
38403841

38413842
public:
38423843
ParameterListInfo() { }
@@ -3868,6 +3869,9 @@ struct ParameterListInfo {
38683869

38693870
bool isVariadicGenericParameter(unsigned paramIdx) const;
38703871

3872+
/// Returns true if this is a sending parameter.
3873+
bool isSendingParameter(unsigned paramIdx) const;
3874+
38713875
/// Retrieve the number of non-defaulted parameters.
38723876
unsigned numNonDefaultedParameters() const {
38733877
return defaultArguments.count();

lib/AST/Type.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,6 +1301,7 @@ ParameterListInfo::ParameterListInfo(
13011301
implicitSelfCapture.resize(params.size());
13021302
inheritActorContext.resize(params.size());
13031303
variadicGenerics.resize(params.size());
1304+
isSending.resize(params.size());
13041305

13051306
// No parameter owner means no parameter list means no default arguments
13061307
// - hand back the zeroed bitvector.
@@ -1364,6 +1365,10 @@ ParameterListInfo::ParameterListInfo(
13641365
if (param->getInterfaceType()->is<PackExpansionType>()) {
13651366
variadicGenerics.set(i);
13661367
}
1368+
1369+
if (param->isSending()) {
1370+
isSending.set(i);
1371+
}
13671372
}
13681373
}
13691374

@@ -1404,6 +1409,9 @@ bool ParameterListInfo::isVariadicGenericParameter(unsigned paramIdx) const {
14041409
: false;
14051410
}
14061411

1412+
bool ParameterListInfo::isSendingParameter(unsigned paramIdx) const {
1413+
return paramIdx < isSending.size() ? isSending[paramIdx] : false;
1414+
}
14071415

14081416
/// Turn a param list into a symbolic and printable representation that does not
14091417
/// include the types, something like (_:, b:, c:)

lib/Sema/CSApply.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5922,23 +5922,25 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
59225922
}
59235923

59245924
/// Apply the contextually Sendable flag to the given expression,
5925-
static void applyContextualClosureFlags(
5926-
Expr *expr, bool implicitSelfCapture, bool inheritActorContext) {
5925+
static void applyContextualClosureFlags(Expr *expr, bool implicitSelfCapture,
5926+
bool inheritActorContext,
5927+
bool isSendingParameter) {
59275928
if (auto closure = dyn_cast<ClosureExpr>(expr)) {
59285929
closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
59295930
closure->setInheritsActorContext(inheritActorContext);
5931+
closure->setSendingParameter(isSendingParameter);
59305932
return;
59315933
}
59325934

59335935
if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
5934-
applyContextualClosureFlags(
5935-
captureList->getClosureBody(), implicitSelfCapture,
5936-
inheritActorContext);
5936+
applyContextualClosureFlags(captureList->getClosureBody(),
5937+
implicitSelfCapture, inheritActorContext,
5938+
isSendingParameter);
59375939
}
59385940

59395941
if (auto identity = dyn_cast<IdentityExpr>(expr)) {
5940-
applyContextualClosureFlags(
5941-
identity->getSubExpr(), implicitSelfCapture, inheritActorContext);
5942+
applyContextualClosureFlags(identity->getSubExpr(), implicitSelfCapture,
5943+
inheritActorContext, isSendingParameter);
59425944
}
59435945
}
59445946

@@ -6166,8 +6168,10 @@ ArgumentList *ExprRewriter::coerceCallArguments(
61666168
// implicit self capture or inheriting actor context.
61676169
bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
61686170
bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
6169-
applyContextualClosureFlags(
6170-
argExpr, isImplicitSelfCapture, inheritsActorContext);
6171+
bool isSendingParameter = paramInfo.isSendingParameter(paramIdx);
6172+
6173+
applyContextualClosureFlags(argExpr, isImplicitSelfCapture,
6174+
inheritsActorContext, isSendingParameter);
61716175

61726176
// If the types exactly match, this is easy.
61736177
auto paramType = param.getOldType();

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4176,6 +4176,16 @@ namespace {
41764176
return ActorIsolation::forNonisolated(/*unsafe=*/false)
41774177
.withPreconcurrency(preconcurrency);
41784178

4179+
// If the explicit closure is used as a non-Sendable sending parameter,
4180+
// make it nonisolated by default instead of inferring its isolation from
4181+
// the context.
4182+
if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure)) {
4183+
if (!explicitClosure->inheritsActorContext() &&
4184+
explicitClosure->isSendingParameter())
4185+
return ActorIsolation::forNonisolated(/*unsafe=*/false)
4186+
.withPreconcurrency(preconcurrency);
4187+
}
4188+
41794189
// A non-Sendable closure gets its isolation from its context.
41804190
auto parentIsolation = getActorIsolationOfContext(
41814191
closure->getParent(), getClosureActorIsolation);

0 commit comments

Comments
 (0)