Skip to content

Commit 37c3a52

Browse files
committed
[Concurrency] Handle non-escaping partial application.
1 parent fc42640 commit 37c3a52

File tree

3 files changed

+132
-33
lines changed

3 files changed

+132
-33
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,69 @@ 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+
// Check for the inner closure in the thunk.
362+
auto innerAutoclosure = dyn_cast<AutoClosureExpr>(
363+
outerAutoclosure->getSingleExpressionBody());
364+
if (!innerAutoclosure ||
365+
innerAutoclosure->getThunkKind()
366+
!= AutoClosureExpr::Kind::SingleCurryThunk)
367+
return None;
368+
369+
auto innerCall = dyn_cast<CallExpr>(
370+
innerAutoclosure->getSingleExpressionBody());
371+
if (!innerCall)
372+
return None;
373+
374+
auto memberCall = dyn_cast<DotSyntaxCallExpr>(innerCall->getFn());
375+
if (!memberCall)
376+
return None;
377+
378+
// Determine whether the partial apply thunk was immediately converted to
379+
// noescape.
380+
bool isEscaping = true;
381+
if (auto conversion = dyn_cast_or_null<FunctionConversionExpr>(parent)) {
382+
auto fnType = conversion->getType()->getAs<FunctionType>();
383+
isEscaping = fnType && !fnType->isNoEscape();
384+
}
385+
386+
return PartialApplyThunkInfo{
387+
apply->getArg(), memberCall->getFn(), isEscaping};
388+
}
389+
390+
/// Find the immediate member reference in the given expression.
391+
static Optional<std::pair<ValueDecl *, SourceLoc>>
392+
findMemberReference(Expr *expr) {
393+
if (auto declRef = dyn_cast<DeclRefExpr>(expr))
394+
return std::make_pair(declRef->getDecl(), declRef->getLoc());
395+
396+
if (auto otherCtor = dyn_cast<OtherConstructorDeclRefExpr>(expr)) {
397+
return std::make_pair(
398+
static_cast<ValueDecl *>(otherCtor->getDecl()), otherCtor->getLoc());
399+
}
400+
401+
return None;
402+
}
403+
341404
void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
342405
class ActorIsolationWalker : public ASTWalker {
343406
ASTContext &ctx;
@@ -377,18 +440,27 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
377440
return { true, expr };
378441
}
379442

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 };
443+
if (auto apply = dyn_cast<ApplyExpr>(expr)) {
444+
// If this is a call to a partial apply thunk, decompose it to check it
445+
// like based on the original written syntax, e.g., "self.method".
446+
if (auto partialApply = decomposePartialApplyThunk(
447+
apply, Parent.getAsExpr())) {
448+
if (auto memberRef = findMemberReference(partialApply->fn)) {
449+
checkMemberReference(
450+
partialApply->base, memberRef->first, memberRef->second,
451+
partialApply->isEscaping);
452+
453+
partialApply->base->walk(*this);
454+
return { false, expr };
455+
}
387456
}
457+
}
388458

389-
if (auto otherCtor = dyn_cast<OtherConstructorDeclRefExpr>(fn)) {
459+
if (auto call = dyn_cast<SelfApplyExpr>(expr)) {
460+
Expr *fn = call->getFn()->getValueProvidingExpr();
461+
if (auto memberRef = findMemberReference(fn)) {
390462
checkMemberReference(
391-
call->getArg(), otherCtor->getDecl(), otherCtor->getLoc());
463+
call->getArg(), memberRef->first, memberRef->second);
392464
call->getArg()->walk(*this);
393465
return { false, expr };
394466
}
@@ -557,7 +629,8 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
557629

558630
/// Check a reference with the given base expression to the given member.
559631
bool checkMemberReference(
560-
Expr *base, ValueDecl *member, SourceLoc memberLoc) {
632+
Expr *base, ValueDecl *member, SourceLoc memberLoc,
633+
bool isEscapingPartialApply = false) {
561634
if (!base || !member)
562635
return false;
563636

@@ -566,24 +639,6 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
566639
return false;
567640

568641
case IsolationRestriction::ActorSelf: {
569-
// Cannot refer to actor-isolated state in a partial application.
570-
if (auto autoclosure = dyn_cast<AutoClosureExpr>(getDeclContext())) {
571-
switch (autoclosure->getThunkKind()) {
572-
case AutoClosureExpr::Kind::DoubleCurryThunk:
573-
case AutoClosureExpr::Kind::SingleCurryThunk:
574-
ctx.Diags.diagnose(
575-
memberLoc, diag::actor_isolated_partial_apply,
576-
member->getDescriptiveKind(),
577-
member->getName());
578-
noteIsolatedActorMember(member);
579-
return true;
580-
581-
case AutoClosureExpr::Kind::None:
582-
// Not in a partial application.
583-
break;
584-
}
585-
}
586-
587642
// Must reference actor-isolated state on 'self'.
588643
auto selfVar = getSelfReference(base);
589644
if (!selfVar) {
@@ -602,6 +657,18 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
602657
cast<ValueDecl>(selfVar->getDeclContext()->getAsDecl()))) {
603658
case ActorIsolation::ActorInstance:
604659
case ActorIsolation::ActorPrivileged:
660+
// An escaping partial application of something that is part of
661+
// the actor's isolated state is never permitted.
662+
if (isEscapingPartialApply) {
663+
ctx.Diags.diagnose(
664+
memberLoc, diag::actor_isolated_partial_apply,
665+
member->getDescriptiveKind(),
666+
member->getName());
667+
noteIsolatedActorMember(member);
668+
return true;
669+
}
670+
break;
671+
605672
case ActorIsolation::Unspecified:
606673
// Okay
607674
break;
@@ -614,7 +681,7 @@ void swift::checkActorIsolation(const Expr *expr, const DeclContext *dc) {
614681
member->getDescriptiveKind(),
615682
member->getName());
616683
noteIsolatedActorMember(member);
617-
break;
684+
return true;
618685
}
619686

620687
// 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: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ 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
// ----------------------------------------------------------------------
@@ -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 8{{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

@@ -62,8 +65,14 @@ extension MyActor {
6265
_ = mutableGlobal // expected-warning{{reference to var 'mutableGlobal' is not concurrency-safe because it involves shared mutable state}}
6366

6467
// Partial application
65-
_ = synchronous // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
66-
_ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' can not be partially applied}}
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'}}
6776

6877
return 5
6978
}
@@ -145,7 +154,16 @@ extension MyActor {
145154

146155
// Partial application
147156
_ = synchronous // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
148-
_ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' 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)
149167
}
150168
}
151169

0 commit comments

Comments
 (0)