Skip to content

Commit 1fc836c

Browse files
gottesmmgottesmm
committed
[concurrency] Persist caller isolation inheriting through serialization.
rdar://142790023
1 parent 9851305 commit 1fc836c

File tree

3 files changed

+320
-12
lines changed

3 files changed

+320
-12
lines changed

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4708,6 +4708,24 @@ getIsolationFromAttributes(const Decl *decl, bool shouldDiagnose = true,
47084708
return std::nullopt;
47094709
}
47104710

4711+
// If the declaration is explicitly marked with 'execution', return the
4712+
// appropriate isolation.
4713+
//
4714+
// NOTE: This needs to occur before we handle an explicit nonisolated attr,
4715+
// since if @execution and nonisolated are used together, we want to ensure
4716+
// that @execution takes priority. This ensures that if we import code from a
4717+
// module that was compiled with a different value for
4718+
// NonIsolatedAsyncInheritsIsolationFromContext, we get the semantics of the
4719+
// source module.
4720+
if (concurrentExecutionAttr) {
4721+
switch (concurrentExecutionAttr->getBehavior()) {
4722+
case ExecutionKind::Concurrent:
4723+
return ActorIsolation::forNonisolated(false /*is unsafe*/);
4724+
case ExecutionKind::Caller:
4725+
return ActorIsolation::forCallerIsolationInheriting();
4726+
}
4727+
}
4728+
47114729
// If the declaration is explicitly marked 'nonisolated', report it as
47124730
// independent.
47134731
if (nonisolatedAttr) {
@@ -4812,17 +4830,6 @@ getIsolationFromAttributes(const Decl *decl, bool shouldDiagnose = true,
48124830
.withPreconcurrency(decl->preconcurrency() || isUnsafe);
48134831
}
48144832

4815-
// If the declaration is explicitly marked with 'execution', return the
4816-
// appropriate isolation.
4817-
if (concurrentExecutionAttr) {
4818-
switch (concurrentExecutionAttr->getBehavior()) {
4819-
case ExecutionKind::Concurrent:
4820-
return ActorIsolation::forNonisolated(false /*is unsafe*/);
4821-
case ExecutionKind::Caller:
4822-
return ActorIsolation::forCallerIsolationInheriting();
4823-
}
4824-
}
4825-
48264833
llvm_unreachable("Forgot about an attribute?");
48274834
}
48284835

@@ -5489,6 +5496,9 @@ static void addAttributesForActorIsolation(ValueDecl *value,
54895496
ASTContext &ctx = value->getASTContext();
54905497
switch (isolation) {
54915498
case ActorIsolation::CallerIsolationInheriting:
5499+
value->getAttrs().add(new (ctx) ExecutionAttr(ExecutionKind::Caller,
5500+
/*implicit=*/true));
5501+
break;
54925502
case ActorIsolation::Nonisolated:
54935503
case ActorIsolation::NonisolatedUnsafe: {
54945504
value->getAttrs().add(new (ctx) NonisolatedAttr(
@@ -5643,6 +5653,19 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
56435653
value->getAttrs().add(preconcurrency);
56445654
}
56455655

5656+
// Check if we inferred CallerIsolationInheriting from our isolation attr, but
5657+
// did not have an ExecutionKind::Caller attached to it.
5658+
//
5659+
// DISCUSSION: This occurs when we have a value decl that is explicitly marked
5660+
// as nonisolated but since NonIsolatedAsyncInheritsIsolationFromContext is
5661+
// enabled, we return CallerIsolationInheriting.
5662+
if (isolationFromAttr && isolationFromAttr->getKind() ==
5663+
ActorIsolation::CallerIsolationInheriting &&
5664+
!value->getAttrs().hasAttribute<ExecutionAttr>()) {
5665+
value->getAttrs().add(new (ctx) ExecutionAttr(ExecutionKind::Caller,
5666+
/*implicit=*/true));
5667+
}
5668+
56465669
if (auto *fd = dyn_cast<FuncDecl>(value)) {
56475670
// Main.main() and Main.$main are implicitly MainActor-protected.
56485671
// Any other isolation is an error.
@@ -5728,6 +5751,7 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
57285751
addAttributesForActorIsolation(value, inferred);
57295752
break;
57305753
case ActorIsolation::CallerIsolationInheriting:
5754+
addAttributesForActorIsolation(value, inferred);
57315755
break;
57325756
case ActorIsolation::Erased:
57335757
llvm_unreachable("cannot infer erased isolation");
@@ -5958,7 +5982,17 @@ InferredActorIsolation ActorIsolationRequest::evaluate(
59585982
}
59595983
}
59605984

5961-
// Default isolation for this member.
5985+
// We did not invoke any earlier rules... so just return the default
5986+
// isolation.
5987+
if (defaultIsolation.isolation.getKind() ==
5988+
ActorIsolation::CallerIsolationInheriting) {
5989+
// If we have caller isolation inheriting, attach the attribute for it so
5990+
// that we preserve that we chose CallerIsolationInheriting through
5991+
// serialization. We do this since we need to support compiling where
5992+
// nonisolated is the default and where caller isolation inheriting is the
5993+
// default.
5994+
addAttributesForActorIsolation(value, defaultIsolation.isolation);
5995+
}
59625996
return defaultIsolation;
59635997
}
59645998

‎test/Serialization/Inputs/caller_inheriting_isolation.swift

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
2+
public func unspecifiedAsync<T>(_ t: T) async {
3+
}
4+
5+
@execution(caller)
6+
public func unspecifiedAsyncCaller<T>(_ t: T) async {
7+
}
8+
9+
@execution(concurrent)
10+
public func unspecifiedAsyncConcurrent<T>(_ t: T) async {
11+
}
12+
13+
nonisolated public func nonisolatedAsync<T>(_ t: T) async {
14+
}
15+
16+
@execution(caller)
17+
nonisolated public func nonisolatedAsyncCaller<T>(_ t: T) async {
18+
}
19+
20+
@execution(concurrent)
21+
nonisolated public func nonisolatedAsyncConcurrent<T>(_ t: T) async {
22+
}
23+
24+
public struct S {
25+
public init() {}
26+
public func unspecifiedAsync<T>(_ t: T) async {
27+
}
28+
29+
@execution(caller)
30+
public func unspecifiedAsyncCaller<T>(_ t: T) async {
31+
}
32+
33+
@execution(concurrent)
34+
public func unspecifiedAsyncConcurrent<T>(_ t: T) async {
35+
}
36+
37+
nonisolated public func nonisolatedAsync<T>(_ t: T) async {
38+
}
39+
40+
@execution(caller)
41+
nonisolated public func nonisolatedAsyncCaller<T>(_ t: T) async {
42+
}
43+
44+
@execution(concurrent)
45+
nonisolated public func nonisolatedAsyncConcurrent<T>(_ t: T) async {
46+
}
47+
}

‎test/Serialization/caller_isolation_inherit.swift

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -enable-experimental-feature NonIsolatedAsyncInheritsIsolationFromContext -emit-module-path %t/WithFeature.swiftmodule -module-name WithFeature %S/Inputs/caller_inheriting_isolation.swift -swift-version 6
3+
// RUN: %target-swift-frontend -emit-module-path %t/WithoutFeature.swiftmodule -module-name WithoutFeature %S/Inputs/caller_inheriting_isolation.swift -swift-version 6
4+
5+
// RUN: %target-swift-frontend -module-name main -I %t %s -emit-sil -o - | %FileCheck %s
6+
7+
// RUN: %target-swift-frontend -enable-experimental-feature NonIsolatedAsyncInheritsIsolationFromContext -module-name main -I %t %s -emit-sil -verify -swift-version 6
8+
9+
// REQUIRES: asserts
10+
// REQUIRES: swift_feature_NonIsolatedAsyncInheritsIsolationFromContext
11+
12+
import WithFeature
13+
import WithoutFeature
14+
15+
class NonSendable {}
16+
17+
actor A {
18+
var ns = NonSendable()
19+
20+
// CHECK-LABEL: // unspecifiedAsync<A>(_:)
21+
// CHECK-NEXT: // Isolation: caller_isolation_inheriting
22+
// CHECK-NEXT: sil @$s11WithFeature16unspecifiedAsyncyyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
23+
func test1() async {
24+
// If unspecifiedAsync does not inherit the isolation of A, then we will get
25+
// an error.
26+
await WithFeature.unspecifiedAsync(ns)
27+
}
28+
29+
// CHECK-LABEL: // unspecifiedAsyncConcurrent<A>(_:)
30+
// CHECK: // Isolation: nonisolated
31+
// CHECK: sil @$s11WithFeature26unspecifiedAsyncConcurrentyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
32+
func test1a() async {
33+
await WithFeature.unspecifiedAsyncConcurrent(ns)
34+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
35+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'unspecifiedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
36+
}
37+
38+
// CHECK-LABEL: // unspecifiedAsyncCaller<A>(_:)
39+
// CHECK: // Isolation: caller_isolation_inheriting
40+
// CHECK: sil @$s11WithFeature22unspecifiedAsyncCalleryyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
41+
func test1b() async {
42+
await WithFeature.unspecifiedAsyncCaller(ns)
43+
}
44+
45+
// CHECK-LABEL: // unspecifiedAsync<A>(_:)
46+
// CHECK: // Isolation: unspecified
47+
// CHECK: sil @$s14WithoutFeature16unspecifiedAsyncyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
48+
func test2() async {
49+
await WithoutFeature.unspecifiedAsync(ns)
50+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
51+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'unspecifiedAsync' risks causing data races between nonisolated and 'self'-isolated uses}}
52+
}
53+
54+
// CHECK-LABEL: // unspecifiedAsyncConcurrent<A>(_:)
55+
// CHECK: // Isolation: nonisolated
56+
// CHECK: sil @$s14WithoutFeature26unspecifiedAsyncConcurrentyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
57+
func test2a() async {
58+
// If unspecifiedAsync does not inherit the isolation of A, then we will get
59+
// an error.
60+
await WithoutFeature.unspecifiedAsyncConcurrent(ns)
61+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
62+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'unspecifiedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
63+
}
64+
65+
// CHECK-LABEL: // unspecifiedAsyncCaller<A>(_:)
66+
// CHECK: // Isolation: caller_isolation_inheriting
67+
// CHECK: sil @$s14WithoutFeature22unspecifiedAsyncCalleryyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
68+
func test2b() async {
69+
await WithoutFeature.unspecifiedAsyncCaller(ns)
70+
}
71+
72+
// CHECK-LABEL: // nonisolatedAsync<A>(_:)
73+
// CHECK: // Isolation: caller_isolation_inheriting
74+
// CHECK: sil @$s11WithFeature16nonisolatedAsyncyyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
75+
func test3() async {
76+
await WithFeature.nonisolatedAsync(ns)
77+
}
78+
79+
// CHECK-LABEL: // nonisolatedAsyncConcurrent<A>(_:)
80+
// CHECK: // Isolation: nonisolated
81+
// CHECK: sil @$s11WithFeature26nonisolatedAsyncConcurrentyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
82+
func test3a() async {
83+
await WithFeature.nonisolatedAsyncConcurrent(ns)
84+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
85+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'nonisolatedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
86+
}
87+
88+
// CHECK-LABEL: // nonisolatedAsyncCaller<A>(_:)
89+
// CHECK: // Isolation: caller_isolation_inheriting
90+
// CHECK: sil @$s11WithFeature22nonisolatedAsyncCalleryyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
91+
func test3b() async {
92+
await WithFeature.nonisolatedAsyncCaller(ns)
93+
}
94+
95+
// CHECK-LABEL: // nonisolatedAsync<A>(_:)
96+
// CHECK: // Isolation: nonisolated
97+
// CHECK: sil @$s14WithoutFeature16nonisolatedAsyncyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
98+
func test4() async {
99+
await WithoutFeature.nonisolatedAsync(ns)
100+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
101+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'nonisolatedAsync' risks causing data races between nonisolated and 'self'-isolated uses}}
102+
}
103+
104+
// CHECK-LABEL: // nonisolatedAsyncConcurrent<A>(_:)
105+
// CHECK: // Isolation: nonisolated
106+
// CHECK: sil @$s14WithoutFeature26nonisolatedAsyncConcurrentyyxYalF : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
107+
func test4a() async {
108+
await WithoutFeature.nonisolatedAsyncConcurrent(ns)
109+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
110+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated global function 'nonisolatedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
111+
}
112+
113+
// CHECK-LABEL: // nonisolatedAsyncCaller<A>(_:)
114+
// CHECK: // Isolation: caller_isolation_inheriting
115+
// CHECK: sil @$s14WithoutFeature22nonisolatedAsyncCalleryyxYalF : $@convention(thin) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0) -> ()
116+
func test4b() async {
117+
await WithoutFeature.nonisolatedAsyncCaller(ns)
118+
}
119+
120+
// CHECK-LABEL: // S.unspecifiedAsync<A>(_:)
121+
// CHECK: // Isolation: caller_isolation_inheriting
122+
// CHECK: sil @$s11WithFeature1SV16unspecifiedAsyncyyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
123+
func test5() async {
124+
let s = WithFeature.S()
125+
await s.unspecifiedAsync(ns)
126+
}
127+
128+
// CHECK-LABEL: // S.unspecifiedAsyncConcurrent<A>(_:)
129+
// CHECK: // Isolation: nonisolated
130+
// CHECK: sil @$s11WithFeature1SV26unspecifiedAsyncConcurrentyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
131+
func test5a() async {
132+
let s = WithFeature.S()
133+
await s.unspecifiedAsyncConcurrent(ns)
134+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
135+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'unspecifiedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
136+
}
137+
138+
// CHECK-LABEL: // S.unspecifiedAsyncCaller<A>(_:)
139+
// CHECK: // Isolation: caller_isolation_inheriting
140+
// CHECK: sil @$s11WithFeature1SV22unspecifiedAsyncCalleryyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
141+
func test5b() async {
142+
let s = WithFeature.S()
143+
await s.unspecifiedAsyncCaller(ns)
144+
}
145+
146+
// CHECK-LABEL: // S.nonisolatedAsync<A>(_:)
147+
// CHECK: // Isolation: caller_isolation_inheriting
148+
// CHECK: sil @$s11WithFeature1SV16nonisolatedAsyncyyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
149+
func test6() async {
150+
let s = WithFeature.S()
151+
await s.nonisolatedAsync(ns)
152+
}
153+
154+
// CHECK-LABEL: // S.nonisolatedAsyncConcurrent<A>(_:)
155+
// CHECK: // Isolation: nonisolated
156+
// CHECK: sil @$s11WithFeature1SV26nonisolatedAsyncConcurrentyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
157+
func test6a() async {
158+
let s = WithFeature.S()
159+
await s.nonisolatedAsyncConcurrent(ns)
160+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
161+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'nonisolatedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
162+
}
163+
164+
// CHECK-LABEL: // S.nonisolatedAsyncCaller<A>(_:)
165+
// CHECK: // Isolation: caller_isolation_inheriting
166+
// CHECK: sil @$s11WithFeature1SV22nonisolatedAsyncCalleryyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
167+
func test6b() async {
168+
let s = WithFeature.S()
169+
await s.nonisolatedAsyncCaller(ns)
170+
}
171+
172+
// CHECK-LABEL: // S.unspecifiedAsync<A>(_:)
173+
// CHECK: // Isolation: unspecified
174+
// CHECK: sil @$s14WithoutFeature1SV16unspecifiedAsyncyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
175+
func test7() async {
176+
let s = WithoutFeature.S()
177+
await s.unspecifiedAsync(ns)
178+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
179+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'unspecifiedAsync' risks causing data races between nonisolated and 'self'-isolated uses}}
180+
}
181+
182+
// CHECK-LABEL: // S.unspecifiedAsyncConcurrent<A>(_:)
183+
// CHECK: // Isolation: nonisolated
184+
// CHECK: sil @$s14WithoutFeature1SV26unspecifiedAsyncConcurrentyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
185+
func test7a() async {
186+
let s = WithoutFeature.S()
187+
await s.unspecifiedAsyncConcurrent(ns)
188+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
189+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'unspecifiedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
190+
}
191+
192+
// CHECK-LABEL: // S.unspecifiedAsyncCaller<A>(_:)
193+
// CHECK: // Isolation: caller_isolation_inheriting
194+
// CHECK: sil @$s14WithoutFeature1SV22unspecifiedAsyncCalleryyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
195+
func test7b() async {
196+
let s = WithoutFeature.S()
197+
await s.unspecifiedAsyncCaller(ns)
198+
}
199+
200+
// CHECK-LABEL: // S.nonisolatedAsync<A>(_:)
201+
// CHECK: // Isolation: nonisolated
202+
// CHECK: sil @$s14WithoutFeature1SV16nonisolatedAsyncyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
203+
func test8() async {
204+
let s = WithoutFeature.S()
205+
await s.nonisolatedAsync(ns)
206+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
207+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'nonisolatedAsync' risks causing data races between nonisolated and 'self'-isolated uses}}
208+
}
209+
210+
// CHECK-LABEL: // S.nonisolatedAsyncConcurrent<A>(_:)
211+
// CHECK: // Isolation: nonisolated
212+
// CHECK: sil @$s14WithoutFeature1SV26nonisolatedAsyncConcurrentyyxYalF : $@convention(method) @async <τ_0_0> (@in_guaranteed τ_0_0, S) -> ()
213+
func test8a() async {
214+
let s = WithoutFeature.S()
215+
await s.nonisolatedAsyncConcurrent(ns)
216+
// expected-error @-1 {{sending 'self.ns' risks causing data races}}
217+
// expected-note @-2 {{sending 'self'-isolated 'self.ns' to nonisolated instance method 'nonisolatedAsyncConcurrent' risks causing data races between nonisolated and 'self'-isolated uses}}
218+
}
219+
220+
// CHECK-LABEL: // S.nonisolatedAsyncCaller<A>(_:)
221+
// CHECK: // Isolation: caller_isolation_inheriting
222+
// CHECK: sil @$s14WithoutFeature1SV22nonisolatedAsyncCalleryyxYalF : $@convention(method) @async <τ_0_0> (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, @in_guaranteed τ_0_0, S) -> ()
223+
func test8b() async {
224+
let s = WithoutFeature.S()
225+
await s.nonisolatedAsyncCaller(ns)
226+
}
227+
}

0 commit comments

Comments
 (0)