Skip to content

Commit a455ff1

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 (cherry picked from commit c06d872)
1 parent c8aee3c commit a455ff1

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
@@ -377,6 +377,11 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
377377
}
378378

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

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

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

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

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 51{{}}
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 {
@@ -1813,3 +1814,14 @@ func testThatGlobalActorTakesPrecedenceOverActorIsolationOnMethods() async {
18131814
// Meaning we would get an error here.
18141815
Task { @MainActor in print(ns) }
18151816
}
1817+
1818+
// Shouldn't get any errors from x.
1819+
//
1820+
// We used to look through the access to x.boolean and think that the closure
1821+
// was capturing x instead of y.
1822+
func testBooleanCapture(_ x: inout NonSendableKlass) {
1823+
let y = x.boolean
1824+
Task.detached { @MainActor [z = y] in
1825+
print(z)
1826+
}
1827+
}

0 commit comments

Comments
 (0)