Skip to content

Make isolated parameter checking work with existential values. #40061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,7 @@ namespace {
ASTContext &ctx;
SmallVector<const DeclContext *, 4> contextStack;
SmallVector<ApplyExpr*, 4> applyStack;
SmallVector<std::pair<OpaqueValueExpr *, Expr *>, 4> opaqueValues;

/// Keeps track of the capture context of variables that have been
/// explicitly captured in closures.
Expand Down Expand Up @@ -1244,6 +1245,13 @@ namespace {
}

std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
if (auto *openExistential = dyn_cast<OpenExistentialExpr>(expr)) {
opaqueValues.push_back({
openExistential->getOpaqueValue(),
openExistential->getExistentialValue()});
return { true, expr };
}

if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
closure->setActorIsolation(determineClosureIsolation(closure));
contextStack.push_back(closure);
Expand Down Expand Up @@ -1363,6 +1371,12 @@ namespace {
}

Expr *walkToExprPost(Expr *expr) override {
if (auto *openExistential = dyn_cast<OpenExistentialExpr>(expr)) {
assert(opaqueValues.back().first == openExistential->getOpaqueValue());
opaqueValues.pop_back();
return expr;
}

if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
assert(contextStack.back() == closure);
contextStack.pop_back();
Expand Down Expand Up @@ -1398,7 +1412,7 @@ namespace {
private:
/// Find the directly-referenced parameter or capture of a parameter for
/// for the given expression.
static VarDecl *getReferencedParamOrCapture(Expr *expr) {
VarDecl *getReferencedParamOrCapture(Expr *expr) {
// Look through identity expressions and implicit conversions.
Expr *prior;
do {
Expand All @@ -1408,6 +1422,16 @@ namespace {

if (auto conversion = dyn_cast<ImplicitConversionExpr>(expr))
expr = conversion->getSubExpr();

// Map opaque values.
if (auto opaqueValue = dyn_cast<OpaqueValueExpr>(expr)) {
for (const auto &known : opaqueValues) {
if (known.first == opaqueValue) {
expr = known.second;
break;
}
}
}
} while (prior != expr);

// 'super' references always act on a 'self' variable.
Expand Down Expand Up @@ -1550,7 +1574,7 @@ namespace {
}

/// If the expression is a reference to `self`, the `self` declaration.
static VarDecl *getReferencedSelf(Expr *expr) {
VarDecl *getReferencedSelf(Expr *expr) {
if (auto selfVar = getReferencedParamOrCapture(expr))
if (selfVar->isSelfParameter() || selfVar->isSelfParamCapture())
return selfVar;
Expand Down Expand Up @@ -1593,8 +1617,8 @@ namespace {
// detect if it is a distributed actor, to provide better isolation notes

auto isDistributedActor = false;
if (auto dc = dyn_cast<ClassDecl>(decl->getDeclContext()))
isDistributedActor = dc->isDistributedActor();
if (auto nominal = decl->getDeclContext()->getSelfNominalTypeDecl())
isDistributedActor = nominal->isDistributedActor();

// FIXME: Make this diagnostic more sensitive to the isolation context of
// the declaration.
Expand Down Expand Up @@ -2538,8 +2562,8 @@ namespace {
case ActorIsolationRestriction::Unsafe:
// This case is hit when passing actor state inout to functions in some
// cases. The error is emitted by diagnoseInOutArg.
auto classDecl = dyn_cast<ClassDecl>(member->getDeclContext());
if (classDecl && classDecl->isDistributedActor()) {
auto nominal = member->getDeclContext()->getSelfNominalTypeDecl();
if (nominal && nominal->isDistributedActor()) {
auto funcDecl = dyn_cast<AbstractFunctionDecl>(member);
if (funcDecl && !funcDecl->isStatic()) {
member->diagnose(diag::distributed_actor_isolated_method);
Expand Down
13 changes: 13 additions & 0 deletions test/Concurrency/isolated_parameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,16 @@ func testIsolatedClosureInference(a: A) {
a2.f()
}
}

// "isolated" existential parameters.
protocol P2: Actor {
func m()
}

@available(SwiftStdlib 5.1, *)
func testExistentialIsolated(a: isolated P2, b: P2) async {
a.m()
await b.m()
b.m() // expected-error{{expression is 'async' but is not marked with 'await'}}
// expected-note@-1{{calls to instance method 'm()' from outside of its actor context are implicitly asynchronous}}
}
8 changes: 4 additions & 4 deletions test/Distributed/distributed_protocol_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ protocol DistProtocol: DistributedActor {
// FIXME(distributed): avoid issuing these warnings, these originate from the call on the DistProtocol where we marked this func as dist isolated,
func local() -> String
// (the note appears a few times, because we misuse the call many times)
// expected-note@-2{{calls to instance method 'local()' from outside of its actor context are implicitly asynchronous}}
// expected-note@-3{{calls to instance method 'local()' from outside of its actor context are implicitly asynchronous}}
// expected-note@-4{{calls to instance method 'local()' from outside of its actor context are implicitly asynchronous}}
// expected-note@-2{{distributed actor-isolated instance method 'local()' declared here}}
// expected-note@-3{{distributed actor-isolated instance method 'local()' declared here}}
// expected-note@-4{{distributed actor-isolated instance method 'local()' declared here}}

distributed func dist() -> String
distributed func dist(string: String) -> String
Expand Down Expand Up @@ -202,4 +202,4 @@ extension TacoPreparation {
// expected-error@-1{{'distributed' function can only be declared within 'distributed actor'}}
}

distributed actor TacoWorker: DistributedTacoMaker {} // implemented in extensions
distributed actor TacoWorker: DistributedTacoMaker {} // implemented in extensions