Skip to content

Commit 14906ba

Browse files
DougGregorDougGregor
authored
Merge pull request #34051 from DougGregor/concurrency-partial-application
[Concurrency] Handle partial application of actor-isolated methods.
2 parents e6da0c2 + 63ed61d commit 14906ba

File tree

6 files changed

+156
-27
lines changed

6 files changed

+156
-27
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4149,6 +4149,9 @@ ERROR(actor_isolated_self_independent_context,none,
41494149
"actor-isolated %0 %1 can not be referenced from an "
41504150
"'@actorIndependent' context",
41514151
(DescriptiveDeclKind, DeclName))
4152+
ERROR(actor_isolated_partial_apply,none,
4153+
"actor-isolated %0 %1 can not be partially applied",
4154+
(DescriptiveDeclKind, DeclName))
41524155
WARNING(concurrent_access_local,none,
41534156
"local %0 %1 is unsafe to reference in code that may execute "
41544157
"concurrently",

‎lib/AST/Expr.cpp

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,17 +2007,37 @@ Expr *AutoClosureExpr::getSingleExpressionBody() const {
20072007
}
20082008

20092009
Expr *AutoClosureExpr::getUnwrappedCurryThunkExpr() const {
2010+
auto maybeUnwrapOpenExistential = [](Expr *expr) {
2011+
if (auto *openExistential = dyn_cast<OpenExistentialExpr>(expr)) {
2012+
expr = openExistential->getSubExpr()->getSemanticsProvidingExpr();
2013+
if (auto *ICE = dyn_cast<ImplicitConversionExpr>(expr))
2014+
expr = ICE->getSyntacticSubExpr();
2015+
}
2016+
2017+
return expr;
2018+
};
2019+
2020+
auto maybeUnwrapOptionalEval = [](Expr *expr) {
2021+
if (auto optEval = dyn_cast<OptionalEvaluationExpr>(expr))
2022+
expr = optEval->getSubExpr();
2023+
if (auto inject = dyn_cast<InjectIntoOptionalExpr>(expr))
2024+
expr = inject->getSubExpr();
2025+
if (auto erasure = dyn_cast<ErasureExpr>(expr))
2026+
expr = erasure->getSubExpr();
2027+
if (auto bind = dyn_cast<BindOptionalExpr>(expr))
2028+
expr = bind->getSubExpr();
2029+
return expr;
2030+
};
2031+
20102032
switch (getThunkKind()) {
20112033
case AutoClosureExpr::Kind::None:
20122034
break;
20132035

20142036
case AutoClosureExpr::Kind::SingleCurryThunk: {
20152037
auto *body = getSingleExpressionBody();
20162038
body = body->getSemanticsProvidingExpr();
2017-
2018-
if (auto *openExistential = dyn_cast<OpenExistentialExpr>(body)) {
2019-
body = openExistential->getSubExpr()->getSemanticsProvidingExpr();
2020-
}
2039+
body = maybeUnwrapOpenExistential(body);
2040+
body = maybeUnwrapOptionalEval(body);
20212041

20222042
if (auto *outerCall = dyn_cast<ApplyExpr>(body)) {
20232043
return outerCall->getFn();
@@ -2034,18 +2054,12 @@ Expr *AutoClosureExpr::getUnwrappedCurryThunkExpr() const {
20342054
AutoClosureExpr::Kind::SingleCurryThunk);
20352055
auto *innerBody = innerClosure->getSingleExpressionBody();
20362056
innerBody = innerBody->getSemanticsProvidingExpr();
2037-
2038-
if (auto *openExistential = dyn_cast<OpenExistentialExpr>(innerBody)) {
2039-
innerBody = openExistential->getSubExpr()->getSemanticsProvidingExpr();
2040-
if (auto *ICE = dyn_cast<ImplicitConversionExpr>(innerBody))
2041-
innerBody = ICE->getSyntacticSubExpr();
2042-
}
2057+
innerBody = maybeUnwrapOpenExistential(innerBody);
2058+
innerBody = maybeUnwrapOptionalEval(innerBody);
20432059

20442060
if (auto *outerCall = dyn_cast<ApplyExpr>(innerBody)) {
20452061
if (auto *innerCall = dyn_cast<ApplyExpr>(outerCall->getFn())) {
2046-
if (auto *declRef = dyn_cast<DeclRefExpr>(innerCall->getFn())) {
2047-
return declRef;
2048-
}
2062+
return innerCall->getFn();
20492063
}
20502064
}
20512065
}

‎lib/Sema/CSApply.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,6 @@ namespace {
925925

926926
case ValueOwnership::Owned:
927927
case ValueOwnership::Shared:
928-
// Ensure that the argument type matches up exactly.
929928
auto selfArgTy = ParenType::get(context,
930929
selfParam.getPlainType(),
931930
selfParam.getParameterFlags());

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,55 @@ class IsolationRestriction {
338338

339339
}
340340

341+
namespace {
342+
/// Describes the important parts of a partial apply thunk.
343+
struct PartialApplyThunkInfo {
344+
Expr *base;
345+
Expr *fn;
346+
bool isEscaping;
347+
};
348+
}
349+
350+
/// Try to decompose a call that might be an invocation of a partial apply
351+
/// thunk.
352+
static Optional<PartialApplyThunkInfo> decomposePartialApplyThunk(
353+
ApplyExpr *apply, Expr *parent) {
354+
// Check for a call to the outer closure in the thunk.
355+
auto outerAutoclosure = dyn_cast<AutoClosureExpr>(apply->getFn());
356+
if (!outerAutoclosure ||
357+
outerAutoclosure->getThunkKind()
358+
!= AutoClosureExpr::Kind::DoubleCurryThunk)
359+
return None;
360+
361+
auto memberFn = outerAutoclosure->getUnwrappedCurryThunkExpr();
362+
if (!memberFn)
363+
return None;
364+
365+
// Determine whether the partial apply thunk was immediately converted to
366+
// noescape.
367+
bool isEscaping = true;
368+
if (auto conversion = dyn_cast_or_null<FunctionConversionExpr>(parent)) {
369+
auto fnType = conversion->getType()->getAs<FunctionType>();
370+
isEscaping = fnType && !fnType->isNoEscape();
371+
}
372+
373+
return PartialApplyThunkInfo{apply->getArg(), memberFn, isEscaping};
374+
}
375+
376+
/// Find the immediate member reference in the given expression.
377+
static Optional<std::pair<ValueDecl *, SourceLoc>>
378+
findMemberReference(Expr *expr) {
379+
if (auto declRef = dyn_cast<DeclRefExpr>(expr))
380+
return std::make_pair(declRef->getDecl(), declRef->getLoc());
381+
382+
if (auto otherCtor = dyn_cast<OtherConstructorDeclRefExpr>(expr)) {
383+
return std::make_pair(
384+
static_cast<ValueDecl *>(otherCtor->getDecl()), otherCtor->getLoc());
385+
}
386+
387+
return None;
388+
}
389+
341390
void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
342391
class ActorIsolationWalker : public ASTWalker {
343392
ASTContext &ctx;
@@ -377,18 +426,27 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
377426
return { true, expr };
378427
}
379428

380-
if (auto call = dyn_cast<SelfApplyExpr>(expr)) {
381-
Expr *fn = call->getFn()->getValueProvidingExpr();
382-
if (auto declRef = dyn_cast<DeclRefExpr>(fn)) {
383-
checkMemberReference(
384-
call->getArg(), declRef->getDecl(), declRef->getLoc());
385-
call->getArg()->walk(*this);
386-
return { false, expr };
429+
if (auto apply = dyn_cast<ApplyExpr>(expr)) {
430+
// If this is a call to a partial apply thunk, decompose it to check it
431+
// like based on the original written syntax, e.g., "self.method".
432+
if (auto partialApply = decomposePartialApplyThunk(
433+
apply, Parent.getAsExpr())) {
434+
if (auto memberRef = findMemberReference(partialApply->fn)) {
435+
checkMemberReference(
436+
partialApply->base, memberRef->first, memberRef->second,
437+
partialApply->isEscaping);
438+
439+
partialApply->base->walk(*this);
440+
return { false, expr };
441+
}
387442
}
443+
}
388444

389-
if (auto otherCtor = dyn_cast<OtherConstructorDeclRefExpr>(fn)) {
445+
if (auto call = dyn_cast<SelfApplyExpr>(expr)) {
446+
Expr *fn = call->getFn()->getValueProvidingExpr();
447+
if (auto memberRef = findMemberReference(fn)) {
390448
checkMemberReference(
391-
call->getArg(), otherCtor->getDecl(), otherCtor->getLoc());
449+
call->getArg(), memberRef->first, memberRef->second);
392450
call->getArg()->walk(*this);
393451
return { false, expr };
394452
}
@@ -557,7 +615,8 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
557615

558616
/// Check a reference with the given base expression to the given member.
559617
bool checkMemberReference(
560-
Expr *base, ValueDecl *member, SourceLoc memberLoc) {
618+
Expr *base, ValueDecl *member, SourceLoc memberLoc,
619+
bool isEscapingPartialApply = false) {
561620
if (!base || !member)
562621
return false;
563622

@@ -566,6 +625,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
566625
return false;
567626

568627
case IsolationRestriction::ActorSelf: {
628+
// Must reference actor-isolated state on 'self'.
569629
auto selfVar = getSelfReference(base);
570630
if (!selfVar) {
571631
ctx.Diags.diagnose(
@@ -583,6 +643,18 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
583643
cast<ValueDecl>(selfVar->getDeclContext()->getAsDecl()))) {
584644
case ActorIsolation::ActorInstance:
585645
case ActorIsolation::ActorPrivileged:
646+
// An escaping partial application of something that is part of
647+
// the actor's isolated state is never permitted.
648+
if (isEscapingPartialApply) {
649+
ctx.Diags.diagnose(
650+
memberLoc, diag::actor_isolated_partial_apply,
651+
member->getDescriptiveKind(),
652+
member->getName());
653+
noteIsolatedActorMember(member);
654+
return true;
655+
}
656+
break;
657+
586658
case ActorIsolation::Unspecified:
587659
// Okay
588660
break;
@@ -595,7 +667,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
595667
member->getDescriptiveKind(),
596668
member->getName());
597669
noteIsolatedActorMember(member);
598-
break;
670+
return true;
599671
}
600672

601673
// Check whether we are in a context that will not execute concurrently

‎lib/Sema/TypeCheckEffects.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,7 +1514,21 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
15141514
ContextScope scope(*this, Context::forClosure(E));
15151515
scope.enterSubFunction();
15161516
scope.resetCoverageForAutoclosureBody();
1517+
1518+
// Curry thunks aren't actually a call to the asynchronous function.
1519+
// Assume that async is covered in such contexts.
1520+
switch (E->getThunkKind()) {
1521+
case AutoClosureExpr::Kind::DoubleCurryThunk:
1522+
case AutoClosureExpr::Kind::SingleCurryThunk:
1523+
Flags.set(ContextFlags::IsAsyncCovered);
1524+
break;
1525+
1526+
case AutoClosureExpr::Kind::None:
1527+
break;
1528+
}
1529+
15171530
E->getBody()->walk(*this);
1531+
15181532
scope.preserveCoverageFromAutoclosureBody();
15191533
return ShouldNotRecurse;
15201534
}

‎test/Concurrency/actor_isolation.swift

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@ func globalFunc() { }
77
func acceptClosure<T>(_: () -> T) { }
88
func acceptEscapingClosure<T>(_: @escaping () -> T) { }
99

10+
func acceptAsyncClosure<T>(_: () async -> T) { }
11+
func acceptEscapingAsyncClosure<T>(_: @escaping () async -> T) { }
12+
1013
// ----------------------------------------------------------------------
1114
// Actor state isolation restrictions
1215
// ----------------------------------------------------------------------
1316
actor class MySuperActor {
1417
var superState: Int = 25
1518

16-
func superMethod() { }
19+
func superMethod() { } // expected-note 2 {{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}}
1720
func superAsyncMethod() async { }
1821

1922
subscript (index: Int) -> String { // expected-note{{subscript declared here}}
@@ -28,7 +31,7 @@ actor class MyActor: MySuperActor {
2831
class func synchronousClass() { }
2932
static func synchronousStatic() { }
3033

31-
func synchronous() -> String { text.first ?? "nothing" } // expected-note 6{{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}}
34+
func synchronous() -> String { text.first ?? "nothing" } // expected-note 18{{only asynchronous methods can be used outside the actor instance; do you want to add 'async'?}}
3235
func asynchronous() async -> String { synchronous() }
3336
}
3437

@@ -46,6 +49,7 @@ extension MyActor {
4649
// @actorIndependent
4750
_ = actorIndependentFunc(otherActor: self)
4851
_ = actorIndependentVar
52+
4953
actorIndependentVar = 17
5054
_ = self.actorIndependentFunc(otherActor: self)
5155
_ = self.actorIndependentVar
@@ -60,6 +64,16 @@ extension MyActor {
6064
_ = immutableGlobal
6165
_ = mutableGlobal // expected-warning{{reference to var 'mutableGlobal' is not concurrency-safe because it involves shared mutable state}}
6266

67+
// Partial application
68+
_ = synchronous // expected-error{{actor-isolated instance method 'synchronous()' can not be referenced from an '@actorIndependent' context}}
69+
_ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' can not be referenced from an '@actorIndependent' context}}
70+
acceptClosure(synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be referenced from an '@actorIndependent' context}}
71+
acceptClosure(self.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be referenced from an '@actorIndependent' context}}
72+
acceptClosure(otherActor.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
73+
acceptEscapingClosure(synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be referenced from an '@actorIndependent' context}}}}
74+
acceptEscapingClosure(self.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be referenced from an '@actorIndependent'}}
75+
acceptEscapingClosure(otherActor.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
76+
6377
return 5
6478
}
6579

@@ -137,6 +151,19 @@ extension MyActor {
137151
}
138152

139153
localVar = 0
154+
155+
// Partial application
156+
_ = synchronous // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
157+
_ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' is unsafe to reference in code that may execute concurrently}}
158+
acceptClosure(synchronous)
159+
acceptClosure(self.synchronous)
160+
acceptClosure(otherActor.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
161+
acceptEscapingClosure(synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
162+
acceptEscapingClosure(self.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
163+
acceptEscapingClosure(otherActor.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
164+
165+
acceptAsyncClosure(self.asynchronous)
166+
acceptEscapingAsyncClosure(self.asynchronous)
140167
}
141168
}
142169

0 commit comments

Comments
 (0)