Skip to content

Commit c06d872

Browse files
committed
[region-isolation] Be more aggressive about not looking through Sendable values when getting underlying objects.
Otherwise, in cases like the following, we look through the load to x.boolean and think that the closure is actually capturing x instead of y: ```swift func testBooleanCapture(_ x: inout NonSendableKlass) { let y = x.boolean Task.detached { @mainactor [z = y] in print(z) } } ``` rdar://131369987
1 parent 588a7db commit c06d872

File tree

4 files changed

+24
-5
lines changed

4 files changed

+24
-5
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,11 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
378378
}
379379

380380
static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
381+
// Before a check if the value we are attempting to access is Sendable. In
382+
// such a case, just return early.
383+
if (!SILIsolationInfo::isNonSendableType(value))
384+
return UnderlyingTrackedValueInfo(value);
385+
381386
// Look through a project_box, so that we process it like its operand object.
382387
if (auto *pbi = dyn_cast<ProjectBoxInst>(value)) {
383388
value = pbi->getOperand();
@@ -386,6 +391,7 @@ static UnderlyingTrackedValueInfo getUnderlyingTrackedValue(SILValue value) {
386391
if (!value->getType().isAddress()) {
387392
SILValue underlyingValue = getUnderlyingTrackedObjectValue(value);
388393

394+
// If we do not have a load inst, just return the value.
389395
if (!isa<LoadInst, LoadBorrowInst>(underlyingValue)) {
390396
return UnderlyingTrackedValueInfo(underlyingValue);
391397
}

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,9 +1131,11 @@ void SILIsolationInfo::printForOneLineLogging(llvm::raw_ostream &os) const {
11311131
// NOTE: We special case RawPointer and NativeObject to ensure they are
11321132
// treated as non-Sendable and strict checking is applied to it.
11331133
bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
1134-
// Treat Builtin.NativeObject and Builtin.RawPointer as non-Sendable.
1134+
// Treat Builtin.NativeObject, Builtin.RawPointer, and Builtin.BridgeObject as
1135+
// non-Sendable.
11351136
if (type.getASTType()->is<BuiltinNativeObjectType>() ||
1136-
type.getASTType()->is<BuiltinRawPointerType>()) {
1137+
type.getASTType()->is<BuiltinRawPointerType>() ||
1138+
type.getASTType()->is<BuiltinBridgeObjectType>()) {
11371139
return true;
11381140
}
11391141

test/Concurrency/async_task_groups.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ extension Collection where Self: Sendable, Element: Sendable, Self.Index: Sendab
201201
// TODO: When we have isolation history, isolation history will be able
202202
// to tell us what is going on.
203203
group.addTask { [submitted,i] in // expected-error {{escaping closure captures non-escaping parameter 'transform'}}
204-
// expected-tns-warning @-1 {{task-isolated value of type '() async throws -> SendableTuple2<Int, T>' passed as a strongly transferred parameter}}
205204
let _ = try await transform(self[i]) // expected-note {{captured here}}
206205
let value: T? = nil
207206
return SendableTuple2(submitted, value!)

test/Concurrency/transfernonsendable.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class NonSendableKlass { // expected-complete-note 53{{}}
1818
// expected-typechecker-only-note @-1 3{{}}
1919
// expected-tns-note @-2 {{}}
2020
var field: NonSendableKlass? = nil
21+
var boolean: Bool = false
2122

2223
init() {}
2324
init(_ x: NonSendableKlass) {
@@ -1331,9 +1332,9 @@ func varSendableNonTrivialLetTupleFieldTest() async {
13311332
await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
13321333
// expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
13331334
// expected-complete-warning @-2 {{passing argument of non-sendable type '(Int, SendableKlass, NonSendableKlass)' into main actor-isolated context may introduce data races}}
1334-
let z = test.1 // expected-tns-note {{access can happen concurrently}}
1335+
let z = test.1
13351336
useValue(z)
1336-
useValue(test)
1337+
useValue(test) // expected-tns-note {{access can happen concurrently}}
13371338
}
13381339

13391340
func varNonSendableNonTrivialLetTupleFieldTest() async {
@@ -1823,3 +1824,14 @@ func testThatGlobalActorTakesPrecedenceOverActorIsolationOnMethods() async {
18231824
// Meaning we would get an error here.
18241825
Task { @MainActor in print(ns) }
18251826
}
1827+
1828+
// Shouldn't get any errors from x.
1829+
//
1830+
// We used to look through the access to x.boolean and think that the closure
1831+
// was capturing x instead of y.
1832+
func testBooleanCapture(_ x: inout NonSendableKlass) {
1833+
let y = x.boolean
1834+
Task.detached { @MainActor [z = y] in
1835+
print(z)
1836+
}
1837+
}

0 commit comments

Comments
 (0)