Skip to content

Commit e10a441

Browse files
committed
prevent nonisolated mutation of isolated properties through an existential
The underlying problem was that the ActorIsolationChecker was not properly recording the mutation environment in which a lookup through an existential was happening. That's because the AST looks like this: ``` (inout_expr ... (open_existential_expr ... (opaque_value_expr ... (declref_expr ... (member_ref_expr ... (opaque_value_expr ... ``` and the walker creates a link from the `open_existential_expr` to the `inout_expr`, but that kind of expression is not checked for its usage environment. Instead, we need to look through that OpenExistentialExpr recursively so that a link from its sub-expression, the `member_ref_expr`, to the `inout_expr` is formed instead. The side-effect of that missing link causing the bug is that the `usageEnv` of that `member_ref_expr` appears to be empty when we don't have the link. A missing link is assumed to mean that it's not in a mutating environment, and thus the usage is treated as a read. This is why in #59573 / rdar://95509917 we are seeing the compiler report that an `await` is needed around the assignment expr. The checker thinks it's a read, but it's actually a mutation, because the member ref is happening in an `inout` expr. Resolves rdar://95509917 Resolves #59573
1 parent ed88700 commit e10a441

File tree

2 files changed

+74
-0
lines changed

2 files changed

+74
-0
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,6 +1693,11 @@ namespace {
16931693
return recordMutableVarParent(parent, inout->getSubExpr());
16941694
}
16951695

1696+
// Look through an expression that opens an existential
1697+
if (auto openExist = dyn_cast<OpenExistentialExpr>(subExpr)) {
1698+
return recordMutableVarParent(parent, openExist->getSubExpr());
1699+
}
1700+
16961701
return false;
16971702
}
16981703

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// RUN: %target-typecheck-verify-swift -disable-availability-checking
2+
// REQUIRES: concurrency
3+
4+
protocol P: Actor {
5+
func f()
6+
var prop: Int { get set } // expected-note 2 {{mutation of this property is only permitted within the actor}}
7+
}
8+
9+
actor A: P {
10+
var prop: Int = 0 // expected-note 2 {{mutation of this property is only permitted within the actor}}
11+
func f() {}
12+
}
13+
14+
func from_isolated_existential1(_ x: isolated any P) {
15+
x.f()
16+
x.prop += 1
17+
x.prop = 100
18+
}
19+
20+
func from_isolated_existential2(_ x: isolated any P) async {
21+
x.f()
22+
x.prop += 1
23+
x.prop = 100
24+
}
25+
26+
func from_nonisolated(_ x: any P) async {
27+
await x.f()
28+
x.prop += 1 // expected-error {{actor-isolated property 'prop' can not be mutated from a non-isolated context}}
29+
x.prop = 100 // expected-error {{actor-isolated property 'prop' can not be mutated from a non-isolated context}}
30+
}
31+
32+
func from_concrete(_ x: A) async {
33+
x.prop += 1 // expected-error {{actor-isolated property 'prop' can not be mutated from a non-isolated context}}
34+
x.prop = 100 // expected-error {{actor-isolated property 'prop' can not be mutated from a non-isolated context}}
35+
}
36+
37+
func from_isolated_concrete(_ x: isolated A) async {
38+
x.prop += 1
39+
x.prop = 100
40+
}
41+
42+
43+
// from https://github.com/apple/swift/issues/59573
44+
actor Act {
45+
var i = 0 // expected-note {{mutation of this property is only permitted within the actor}}
46+
}
47+
let act = Act()
48+
49+
func bad() async {
50+
// expected-warning@+2 {{no 'async' operations occur within 'await' expression}}
51+
// expected-error@+1 {{actor-isolated property 'i' can not be mutated from a non-isolated context}}
52+
await act.i = 666
53+
}
54+
55+
protocol Proto: Actor {
56+
var i: Int { get set } // expected-note 2 {{mutation of this property is only permitted within the actor}}
57+
}
58+
extension Act: Proto {}
59+
60+
func good() async {
61+
// expected-warning@+2 {{no 'async' operations occur within 'await' expression}}
62+
// expected-error@+1 {{actor-isolated property 'i' can not be mutated from a non-isolated context}}
63+
await (act as any Proto).i = 42
64+
let aIndirect: any Proto = act
65+
66+
// expected-warning@+2 {{no 'async' operations occur within 'await' expression}}
67+
// expected-error@+1 {{actor-isolated property 'i' can not be mutated from a non-isolated context}}
68+
await aIndirect.i = 777
69+
}

0 commit comments

Comments
 (0)