Skip to content

Commit b9e1b9c

Browse files
authored
[Distributed] Fix dist property being implicitly async after refactor (#61995)
1 parent 5d74696 commit b9e1b9c

File tree

5 files changed

+74
-10
lines changed

5 files changed

+74
-10
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4981,6 +4981,9 @@ ERROR(distributed_property_accessor_only_get_can_be_distributed,none,
49814981
NOTE(distributed_actor_isolated_property,none,
49824982
"access to %0 %1 is only permitted within distributed actor %2",
49834983
(DescriptiveDeclKind, DeclName, DeclName))
4984+
NOTE(distributed_actor_synchronous_access_distributed_computed_property,none,
4985+
"access to %0 %1 from outside the distributed actor %2 must be asynchronous",
4986+
(DescriptiveDeclKind, DeclName, DeclName))
49844987

49854988
ERROR(concurrency_lib_missing,none,
49864989
"missing '%0' declaration, probably because the '_Concurrency' "

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,11 +1303,19 @@ static void noteIsolatedActorMember(
13031303
// FIXME: Make this diagnostic more sensitive to the isolation context of
13041304
// the declaration.
13051305
if (isDistributedActor) {
1306-
if (isa<VarDecl>(decl)) {
1307-
// Distributed actor properties are never accessible externally.
1308-
decl->diagnose(diag::distributed_actor_isolated_property,
1309-
decl->getDescriptiveKind(), decl->getName(),
1310-
nominal->getName());
1306+
if (auto varDecl = dyn_cast<VarDecl>(decl)) {
1307+
if (varDecl->isDistributed()) {
1308+
// This is an attempt to access a `distributed var` synchronously, so offer a more detailed error
1309+
decl->diagnose(diag::distributed_actor_synchronous_access_distributed_computed_property,
1310+
decl->getDescriptiveKind(), decl->getName(),
1311+
nominal->getName());
1312+
} else {
1313+
// Distributed actor properties are never accessible externally.
1314+
decl->diagnose(diag::distributed_actor_isolated_property,
1315+
decl->getDescriptiveKind(), decl->getName(),
1316+
nominal->getName());
1317+
}
1318+
13111319
} else {
13121320
// it's a function or subscript
13131321
decl->diagnose(diag::note_distributed_actor_isolated_method,

lib/Sema/TypeCheckEffects.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2476,15 +2476,30 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
24762476
effects.push_back(EffectKind::Async);
24772477
}
24782478

2479-
checkThrowAsyncSite(E, getter->hasThrows(),
2479+
bool requiresTry = getter->hasThrows();
2480+
checkThrowAsyncSite(E, requiresTry,
24802481
Classification::forEffect(effects,
24812482
ConditionalEffectKind::Always,
24822483
getKindOfEffectfulProp(member)));
24832484

2484-
} else if (E->isImplicitlyAsync()) {
2485-
checkThrowAsyncSite(E, /*requiresTry=*/false,
2486-
Classification::forUnconditional(EffectKind::Async,
2487-
getKindOfEffectfulProp(member)));
2485+
} else {
2486+
EffectList effects;
2487+
bool requiresTry = false;
2488+
if (E->isImplicitlyAsync()) {
2489+
effects.push_back(EffectKind::Async);
2490+
}
2491+
if (E->isImplicitlyThrows()) {
2492+
// E.g. it may be a distributed computed property, accessed across actors.
2493+
effects.push_back(EffectKind::Throws);
2494+
requiresTry = true;
2495+
}
2496+
2497+
if (!effects.empty()) {
2498+
checkThrowAsyncSite(E, requiresTry,
2499+
Classification::forEffect(effects,
2500+
ConditionalEffectKind::Always,
2501+
getKindOfEffectfulProp(member)));
2502+
}
24882503
}
24892504

24902505
return ShouldRecurse;

test/Distributed/distributed_actor_isolation.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ distributed actor DistributedActor_1 {
131131

132132
func test_inside() async throws {
133133
_ = self.name
134+
_ = self.computed
134135
_ = self.computedMutable
135136

136137
_ = try await self.distInt()
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
3+
// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unknown -disable-availability-checking -I %t 2>&1 %s
4+
// REQUIRES: concurrency
5+
// REQUIRES: distributed
6+
7+
import Distributed
8+
import FakeDistributedActorSystems
9+
10+
let globalActorSystem = LocalTestingDistributedActorSystem()
11+
12+
distributed actor MyDistributedActor {
13+
typealias ActorSystem = LocalTestingDistributedActorSystem
14+
distributed var distributedProperty: Set<Int> { [] }
15+
distributed var accessMe: Set<Int> { [] }
16+
// expected-note@-1{{access to distributed property 'accessMe' from outside the distributed actor 'MyDistributedActor' must be asynchronous}}
17+
}
18+
19+
func test(da: MyDistributedActor) async throws {
20+
_ = await da.distributedProperty // expected-error{{property access can throw but is not marked with 'try'}}
21+
// expected-note@-1{{did you mean to use 'try'?}}
22+
// expected-note@-2{{did you mean to handle error as optional value?}}
23+
// expected-note@-3{{did you mean to disable error propagation?}}
24+
25+
_ = try da.distributedProperty // expected-error{{expression is 'async' but is not marked with 'await'}}
26+
// expected-note@-1{{property access is 'async'}}
27+
28+
_ = try await da.distributedProperty // ok, implicitly async + throws
29+
}
30+
31+
func testSyncFunc(da: MyDistributedActor) throws {
32+
_ = da.accessMe // expected-error{{actor-isolated distributed property 'accessMe' can not be referenced from a non-isolated context}}
33+
// expected-error@-1{{property access can throw but is not marked with 'try'}}
34+
// expected-note@-2{{did you mean to use 'try'?}}
35+
// expected-note@-3{{did you mean to handle error as optional value?}}
36+
// expected-note@-4{{did you mean to disable error propagation?}}
37+
}

0 commit comments

Comments
 (0)