Skip to content

Commit 077f62c

Browse files
gottesmmgottesmm
committed
Fix how we look through (or not) and find the isolation ofunchecked_take_enum_data_addr/struct_element_addr fields.
1 parent 14f5623 commit 077f62c

File tree

4 files changed

+321
-23
lines changed

4 files changed

+321
-23
lines changed

‎lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -156,48 +156,38 @@ struct UseDefChainVisitor
156156
isMerge = true;
157157
break;
158158
case ProjectionKind::Enum: {
159-
// Enum is never a merge since it always has a single tuple field... but
160-
// it can be actor isolated.
161-
if (!bool(actorIsolation)) {
162-
auto *uedi = cast<UncheckedTakeEnumDataAddrInst>(inst);
163-
auto i = getActorIsolation(uedi->getEnumDecl());
164-
// If our operand decl is actor isolated, then we want to stop looking
165-
// through since it is Sendable.
166-
if (i.isActorIsolated()) {
167-
actorIsolation = i;
168-
return SILValue();
169-
}
170-
}
159+
auto op = cast<UncheckedTakeEnumDataAddrInst>(inst)->getOperand();
160+
161+
// See if our operand type is a sendable type. In such a case, we do not
162+
// want to look through our operand.
163+
if (!isNonSendableType(op->getType(), op->getFunction()))
164+
return SILValue();
165+
171166
break;
172167
}
173168
case ProjectionKind::Tuple: {
174169
// These are merges if we have multiple fields.
175-
auto *tti = cast<TupleElementAddrInst>(inst);
170+
auto op = cast<TupleElementAddrInst>(inst)->getOperand();
176171

177-
// See if our result type is a sendable type. In such a case, we do not
178-
// want to look through the tuple_element_addr since we do not want to
179-
// identify the sendable type with the non-sendable operand. These we
180-
// are always going to ignore anyways since a sendable let/var field of
181-
// a struct can always be used.
182-
if (!isNonSendableType(tti->getType(), tti->getFunction()))
172+
if (!isNonSendableType(op->getType(), op->getFunction()))
183173
return SILValue();
184174

185-
isMerge |= tti->getOperand()->getType().getNumTupleElements() > 1;
175+
isMerge |= op->getType().getNumTupleElements() > 1;
186176
break;
187177
}
188178
case ProjectionKind::Struct:
189-
auto *sea = cast<StructElementAddrInst>(inst);
179+
auto op = cast<StructElementAddrInst>(inst)->getOperand();
190180

191181
// See if our result type is a sendable type. In such a case, we do not
192182
// want to look through the struct_element_addr since we do not want to
193183
// identify the sendable type with the non-sendable operand. These we
194184
// are always going to ignore anyways since a sendable let/var field of
195185
// a struct can always be used.
196-
if (!isNonSendableType(sea->getType(), sea->getFunction()))
186+
if (!isNonSendableType(op->getType(), op->getFunction()))
197187
return SILValue();
198188

199189
// These are merges if we have multiple fields.
200-
isMerge |= sea->getOperand()->getType().getNumNominalFields() > 1;
190+
isMerge |= op->getType().getNumNominalFields() > 1;
201191
break;
202192
}
203193
}
@@ -1446,6 +1436,8 @@ class PartitionOpTranslator {
14461436
LLVM_DEBUG(llvm::dbgs() << " %%" << state->getID() << ": ";
14471437
state->print(llvm::dbgs()); llvm::dbgs() << *arg);
14481438
nonSendableJoinedIndices.push_back(state->getID());
1439+
} else {
1440+
LLVM_DEBUG(llvm::dbgs() << " Sendable: " << *arg);
14491441
}
14501442
}
14511443

‎lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,12 +243,23 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
243243
sei->getStructDecl());
244244
}
245245

246+
if (auto *seai = dyn_cast<StructElementAddrInst>(inst)) {
247+
return SILIsolationInfo::getActorIsolated(seai, SILValue(),
248+
seai->getStructDecl());
249+
}
250+
246251
// See if we have an unchecked_enum_data from a global actor isolated type.
247252
if (auto *uedi = dyn_cast<UncheckedEnumDataInst>(inst)) {
248253
return SILIsolationInfo::getActorIsolated(uedi, SILValue(),
249254
uedi->getEnumDecl());
250255
}
251256

257+
// See if we have an unchecked_enum_data from a global actor isolated type.
258+
if (auto *utedi = dyn_cast<UncheckedTakeEnumDataAddrInst>(inst)) {
259+
return SILIsolationInfo::getActorIsolated(utedi, SILValue(),
260+
utedi->getEnumDecl());
261+
}
262+
252263
// Check if we have an unsafeMutableAddressor from a global actor, mark the
253264
// returned value as being actor derived.
254265
if (auto applySite = dyn_cast<ApplyInst>(inst)) {

‎test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,31 @@ var booleanFlag: Bool { false }
3030
@MainActor var mainActorIsolatedGlobal = NonSendableKlass()
3131
@CustomActor var customActorIsolatedGlobal = NonSendableKlass()
3232

33+
@MainActor
34+
class NonSendableGlobalActorIsolatedKlass {}
35+
36+
@available(*, unavailable)
37+
extension NonSendableGlobalActorIsolatedKlass: Sendable {}
38+
39+
@MainActor
40+
struct NonSendableGlobalActorIsolatedStruct {
41+
var k = NonSendableKlass()
42+
}
43+
44+
@available(*, unavailable)
45+
extension NonSendableGlobalActorIsolatedStruct: Sendable {}
46+
47+
@MainActor
48+
enum NonSendableGlobalActorIsolatedEnum {
49+
case first
50+
case second(NonSendableKlass)
51+
case third(SendableKlass)
52+
}
53+
54+
@available(*, unavailable)
55+
extension NonSendableGlobalActorIsolatedEnum: Sendable {}
56+
57+
3358
/////////////////
3459
// MARK: Tests //
3560
/////////////////

‎test/Concurrency/transfernonsendable_global_actor_nonsendable.swift

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
// RUN: %target-swift-frontend -emit-sil -swift-version 6 -disable-availability-checking -verify %s -o /dev/null -parse-as-library -enable-experimental-feature TransferringArgsAndResults
2+
3+
// READ THIS: This test is testing specifically behavior around global actor
4+
// isolated types that are nonsendable. This is a bit of a corner case so we use
5+
// a separate test case from the main global actor test case.
6+
7+
// REQUIRES: concurrency
8+
// REQUIRES: asserts
9+
10+
////////////////////////
11+
// MARK: Declarations //
12+
////////////////////////
13+
14+
class NonSendableKlass {}
15+
final class SendableKlass : Sendable {}
16+
17+
actor CustomActorInstance {}
18+
19+
@globalActor
20+
struct CustomActor {
21+
static let shared = CustomActorInstance()
22+
}
23+
24+
func transferToNonIsolated<T>(_ t: T) async {}
25+
@MainActor func transferToMainActor<T>(_ t: T) async {}
26+
@CustomActor func transferToCustomActor<T>(_ t: T) async {}
27+
func useValue<T>(_ t: T) {}
28+
func useValueAsync<T>(_ t: T) async {}
29+
@MainActor func useValueMainActor<T>(_ t: T) {}
30+
@MainActor func mainActorFunction() {}
31+
32+
var booleanFlag: Bool { false }
33+
@MainActor var mainActorIsolatedGlobal = NonSendableKlass()
34+
@CustomActor var customActorIsolatedGlobal = NonSendableKlass()
35+
36+
@MainActor
37+
class NonSendableGlobalActorIsolatedKlass {
38+
var k = NonSendableKlass()
39+
var p: (any GlobalActorIsolatedProtocol)? = nil
40+
var p2: OtherProtocol? = nil
41+
}
42+
43+
@available(*, unavailable)
44+
extension NonSendableGlobalActorIsolatedKlass: Sendable {}
45+
46+
@MainActor
47+
final class FinalNonSendableGlobalActorIsolatedKlass {
48+
var k = NonSendableKlass()
49+
var p: (any GlobalActorIsolatedProtocol)? = nil
50+
var p2: OtherProtocol? = nil
51+
}
52+
53+
@available(*, unavailable)
54+
extension FinalNonSendableGlobalActorIsolatedKlass: Sendable {}
55+
56+
57+
@MainActor
58+
struct NonSendableGlobalActorIsolatedStruct {
59+
var k = NonSendableKlass()
60+
var p: (any GlobalActorIsolatedProtocol)? = nil
61+
var p2: OtherProtocol? = nil
62+
}
63+
64+
@available(*, unavailable)
65+
extension NonSendableGlobalActorIsolatedStruct: Sendable {}
66+
67+
@MainActor protocol GlobalActorIsolatedProtocol {
68+
var k: NonSendableKlass { get }
69+
var p: GlobalActorIsolatedProtocol { get }
70+
var p2: OtherProtocol { get }
71+
}
72+
73+
protocol OtherProtocol {
74+
var k: NonSendableKlass { get }
75+
}
76+
77+
@MainActor
78+
enum NonSendableGlobalActorIsolatedEnum {
79+
case first
80+
case second(NonSendableKlass)
81+
case third(SendableKlass)
82+
case fourth(GlobalActorIsolatedProtocol)
83+
case fifth(OtherProtocol)
84+
}
85+
86+
@available(*, unavailable)
87+
extension NonSendableGlobalActorIsolatedEnum: Sendable {}
88+
89+
/////////////////
90+
// MARK: Tests //
91+
/////////////////
92+
93+
extension NonSendableGlobalActorIsolatedStruct {
94+
mutating func test() {
95+
_ = self.k
96+
}
97+
98+
mutating func test2() -> NonSendableKlass {
99+
self.k
100+
}
101+
102+
mutating func test3() -> transferring NonSendableKlass {
103+
self.k
104+
} // expected-error {{transferring 'self.k' may cause a data race}}
105+
// expected-note @-1 {{main actor-isolated 'self.k' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
106+
107+
mutating func test4() -> (any GlobalActorIsolatedProtocol)? {
108+
self.p
109+
}
110+
111+
// TODO: Should error here.
112+
mutating func test5() -> transferring (any GlobalActorIsolatedProtocol)? {
113+
self.p
114+
}
115+
116+
mutating func test6() -> (any OtherProtocol)? {
117+
self.p2
118+
}
119+
120+
// TODO: Should error here.
121+
mutating func test7() -> transferring (any OtherProtocol)? {
122+
self.p2
123+
}
124+
}
125+
126+
extension NonSendableGlobalActorIsolatedEnum {
127+
mutating func test() {
128+
if case let .fourth(x) = self {
129+
print(x)
130+
}
131+
switch self {
132+
case .first:
133+
break
134+
case .second(let x):
135+
print(x)
136+
break
137+
case .third(let x):
138+
print(x)
139+
break
140+
case .fourth(let x):
141+
print(x)
142+
break
143+
case .fifth(let x):
144+
print(x)
145+
break
146+
}
147+
}
148+
149+
mutating func test2() -> (any GlobalActorIsolatedProtocol)? {
150+
guard case let .fourth(x) = self else {
151+
return nil
152+
}
153+
return x
154+
}
155+
156+
// TODO: This should error.
157+
mutating func test3() -> transferring (any GlobalActorIsolatedProtocol)? {
158+
guard case let .fourth(x) = self else {
159+
return nil
160+
}
161+
return x
162+
}
163+
164+
mutating func test3() -> transferring NonSendableKlass? {
165+
guard case let .second(x) = self else {
166+
return nil
167+
}
168+
return x
169+
} // expected-error {{transferring 'x.some' may cause a data race}}
170+
// expected-note @-1 {{main actor-isolated 'x.some' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
171+
}
172+
173+
extension NonSendableGlobalActorIsolatedKlass {
174+
func test() {
175+
_ = self.k
176+
}
177+
178+
func test2() -> NonSendableKlass {
179+
self.k
180+
}
181+
182+
func test3() -> transferring NonSendableKlass {
183+
self.k
184+
} // expected-error {{transferring 'self.k' may cause a data race}}
185+
// expected-note @-1 {{main actor-isolated 'self.k' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
186+
187+
func test4() -> (any GlobalActorIsolatedProtocol)? {
188+
self.p
189+
}
190+
191+
// TODO: Should error here.
192+
func test5() -> transferring (any GlobalActorIsolatedProtocol)? {
193+
self.p
194+
}
195+
196+
func test6() -> (any OtherProtocol)? {
197+
self.p2
198+
}
199+
200+
// TODO: Should error here.
201+
func test7() -> transferring (any OtherProtocol)? {
202+
self.p2
203+
}
204+
}
205+
206+
extension FinalNonSendableGlobalActorIsolatedKlass {
207+
func test() {
208+
_ = self.k
209+
}
210+
211+
func test2() -> NonSendableKlass {
212+
self.k
213+
}
214+
215+
func test3() -> transferring NonSendableKlass {
216+
self.k
217+
} // expected-error {{transferring 'self.k' may cause a data race}}
218+
// expected-note @-1 {{main actor-isolated 'self.k' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
219+
220+
func test4() -> (any GlobalActorIsolatedProtocol)? {
221+
self.p
222+
}
223+
224+
// TODO: Should error here.
225+
func test5() -> transferring (any GlobalActorIsolatedProtocol)? {
226+
self.p
227+
}
228+
229+
func test6() -> (any OtherProtocol)? {
230+
self.p2
231+
}
232+
233+
// TODO: Should error here.
234+
func test7() -> transferring (any OtherProtocol)? {
235+
self.p2
236+
}
237+
}
238+
239+
extension GlobalActorIsolatedProtocol {
240+
mutating func test() {
241+
_ = self.k
242+
}
243+
244+
mutating func test2() -> NonSendableKlass {
245+
self.k
246+
}
247+
248+
mutating func test3() -> transferring NonSendableKlass {
249+
self.k
250+
} // expected-error {{transferring 'self.k' may cause a data race}}
251+
// expected-note @-1 {{main actor-isolated 'self.k' cannot be a transferring result. main actor-isolated uses may race with caller uses}}
252+
253+
mutating func test4() -> (any GlobalActorIsolatedProtocol)? {
254+
self.p
255+
}
256+
257+
// TODO: Should error here.
258+
mutating func test5() -> transferring (any GlobalActorIsolatedProtocol)? {
259+
self.p
260+
}
261+
262+
mutating func test6() -> (any OtherProtocol)? {
263+
self.p2
264+
}
265+
266+
// TODO: Should error here.
267+
mutating func test7() -> transferring (any OtherProtocol)? {
268+
self.p2
269+
}
270+
}

0 commit comments

Comments
 (0)