Skip to content

Commit 8b899b8

Browse files
committed
[region-isolation] Teach variable_name_inference how to infer through store_borrow initialized variables.
This eliminates a bunch of cases where we couldn't infer the name of a variable and used the type based diagnostic. It also eliminates an 'unknown' case for move checking.
1 parent 7368de2 commit 8b899b8

10 files changed

+176
-89
lines changed

lib/SILOptimizer/Utils/VariableNameUtils.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ findRootValueForNonTupleTempAllocation(AllocationInst *allocInst,
5151
}
5252
}
5353

54+
if (auto *sbi = dyn_cast<StoreBorrowInst>(&inst)) {
55+
if (sbi->getDest() == allocInst)
56+
return sbi->getSrc();
57+
}
58+
5459
// If we do not identify the write... return SILValue(). We weren't able
5560
// to understand the write.
5661
break;
@@ -183,7 +188,7 @@ static SILValue findRootValueForTupleTempAllocation(AllocationInst *allocInst,
183188

184189
SILValue VariableNameInferrer::getRootValueForTemporaryAllocation(
185190
AllocationInst *allocInst) {
186-
struct AddressWalker : public TransitiveAddressWalker<AddressWalker> {
191+
struct AddressWalker final : public TransitiveAddressWalker<AddressWalker> {
187192
AddressWalkerState &state;
188193

189194
AddressWalker(AddressWalkerState &state) : state(state) {}
@@ -194,6 +199,12 @@ SILValue VariableNameInferrer::getRootValueForTemporaryAllocation(
194199
return true;
195200
}
196201

202+
TransitiveUseVisitation visitTransitiveUseAsEndPointUse(Operand *use) {
203+
if (auto *sbi = dyn_cast<StoreBorrowInst>(use->getUser()))
204+
return TransitiveUseVisitation::OnlyUser;
205+
return TransitiveUseVisitation::OnlyUses;
206+
}
207+
197208
void onError(Operand *use) { state.foundError = true; }
198209
};
199210

@@ -288,6 +299,13 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
288299
return allocInst;
289300
}
290301

302+
// If we have a store_borrow, always look at the dest. We are going to see
303+
// if we can determine if dest is a temporary alloc_stack.
304+
if (auto *sbi = dyn_cast<StoreBorrowInst>(searchValue)) {
305+
searchValue = sbi->getDest();
306+
continue;
307+
}
308+
291309
if (auto *globalAddrInst = dyn_cast<GlobalAddrInst>(searchValue)) {
292310
variableNamePath.push_back(globalAddrInst);
293311
return globalAddrInst;
@@ -487,7 +505,9 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
487505
isa<ConvertFunctionInst>(searchValue) ||
488506
isa<MarkUninitializedInst>(searchValue) ||
489507
isa<CopyableToMoveOnlyWrapperAddrInst>(searchValue) ||
490-
isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue)) {
508+
isa<MoveOnlyWrapperToCopyableAddrInst>(searchValue) ||
509+
isa<MoveOnlyWrapperToCopyableValueInst>(searchValue) ||
510+
isa<CopyableToMoveOnlyWrapperValueInst>(searchValue)) {
491511
searchValue = cast<SingleValueInstruction>(searchValue)->getOperand(0);
492512
continue;
493513
}

test/Concurrency/transfernonsendable.swift

Lines changed: 87 additions & 60 deletions
Large diffs are not rendered by default.

test/Concurrency/transfernonsendable_asynclet.swift

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,12 +290,13 @@ func asyncLet_Let_ActorIsolated_CallBuriedInOtherExpr3() async {
290290
func asyncLet_Let_ActorIsolated_CallBuriedInOtherExpr4() async {
291291
let x = NonSendableKlass()
292292

293-
async let y = useValue(transferToMainInt(x) + transferToMainInt(x)) // expected-tns-warning {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to main actor-isolated context; later accesses could race}}
294-
// expected-tns-note @-1:67 {{access here could race}}
293+
async let y = useValue(transferToMainInt(x) + transferToMainInt(x)) // expected-tns-warning {{transferring 'x' may cause a race}}
294+
// expected-tns-note @-1 {{'x' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
295+
// expected-tns-note @-2:67 {{access here could race}}
295296

296-
// expected-complete-warning @-3 {{capture of 'x' with non-sendable type 'NonSendableKlass' in 'async let' binding}}
297-
// expected-complete-warning @-4 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
297+
// expected-complete-warning @-4 {{capture of 'x' with non-sendable type 'NonSendableKlass' in 'async let' binding}}
298298
// expected-complete-warning @-5 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
299+
// expected-complete-warning @-6 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
299300

300301
let _ = await y
301302
}
@@ -745,7 +746,8 @@ func asyncLetWithoutCapture() async {
745746
async let x: NonSendableKlass = await returnValueFromMain()
746747
// expected-warning @-1 {{non-sendable type 'NonSendableKlass' returned by implicitly asynchronous call to main actor-isolated function cannot cross actor boundary}}
747748
let y = await x
748-
await transferToMain(y) // expected-tns-warning {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to main actor-isolated context}}
749-
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
749+
await transferToMain(y) // expected-tns-warning {{transferring 'y' may cause a race}}
750+
// expected-tns-note @-1 {{'y' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
751+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
750752
useValue(y) // expected-tns-note {{access here could race}}
751753
}

test/Concurrency/transfernonsendable_global_actor.swift

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,15 @@ private class NonSendableLinkedListNode<T> { // expected-complete-note 3{{}}
4848
@GlobalActor func useGlobalActor1() async {
4949
let x = firstList
5050

51-
// TODO: This should say global actor isolated isolated.
52-
await transferToMainActor(x) // expected-tns-warning {{global actor 'GlobalActor'-isolated value of type 'NonSendableLinkedList<Int>' transferred to main actor-isolated context}}
53-
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableLinkedList<Int>' into main actor-isolated context may introduce data races}}
51+
await transferToMainActor(x) // expected-tns-warning {{transferring 'x' may cause a race}}
52+
// expected-tns-note @-1 {{transferring global actor 'GlobalActor'-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and global actor 'GlobalActor'-isolated uses}}
53+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableLinkedList<Int>' into main actor-isolated context may introduce data races}}
5454

5555
let y = secondList.listHead!.next!
5656

57-
await transferToMainActor(y) // expected-tns-warning {{global actor 'GlobalActor'-isolated value of type 'NonSendableLinkedListNode<Int>' transferred to main actor-isolated context}}
58-
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' into main actor-isolated context may introduce data races}}
57+
await transferToMainActor(y) // expected-tns-warning {{transferring 'y' may cause a race}}
58+
// expected-tns-note @-1 {{transferring global actor 'GlobalActor'-isolated 'y' to main actor-isolated callee could cause races between main actor-isolated and global actor 'GlobalActor'-isolated uses}}
59+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' into main actor-isolated context may introduce data races}}
5960
}
6061

6162
@GlobalActor func useGlobalActor2() async {
@@ -91,8 +92,9 @@ private class NonSendableLinkedListNode<T> { // expected-complete-note 3{{}}
9192
@GlobalActor func useGlobalActor5() async {
9293
let x = NonSendableLinkedListNode<Int>()
9394

94-
await transferToNonIsolated(x) // expected-tns-warning {{transferring value of non-Sendable type 'NonSendableLinkedListNode<Int>' from global actor 'GlobalActor'-isolated context to nonisolated context; later accesses could race}}
95-
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
95+
await transferToNonIsolated(x) // expected-tns-warning {{transferring 'x' may cause a race}}
96+
// expected-tns-note @-1 {{'x' is transferred from global actor 'GlobalActor'-isolated caller to nonisolated callee. Later uses in caller could race with potential uses in callee}}
97+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableLinkedListNode<Int>' outside of global actor 'GlobalActor'-isolated context may introduce data races}}
9698

9799
useValue(x) // expected-tns-note {{access here could race}}
98100
}

test/Concurrency/transfernonsendable_isolationcrossing_partialapply.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ func normalFunc_testLocal_2() {
9393
// diagnostic.
9494
func transferBeforeCaptureErrors() async {
9595
let x = NonSendableKlass()
96-
await transferToCustom(x) // expected-warning {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to global actor 'CustomActor'-isolated context}}
96+
await transferToCustom(x) // expected-warning {{transferring 'x' may cause a race}}
97+
// expected-note @-1 {{'x' is transferred from nonisolated caller to global actor 'CustomActor'-isolated callee. Later uses in caller could race with potential uses in callee}}
9798
let _ = { @MainActor in // expected-note {{access here could race}}
9899
useValue(x)
99100
}

test/Concurrency/transfernonsendable_ownership.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,25 @@ func testConsumingUseAfterConsumeError(_ x: consuming Klass) async { // expected
6767
}
6868

6969
func testBorrowing(_ x: borrowing Klass) async {
70-
await transferToMain(x) // expected-warning {{task-isolated value of type 'Klass' transferred to main actor-isolated context}}
70+
await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
71+
// expected-note @-1 {{transferring task-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
7172
}
7273

7374
func testBorrowingError(_ x: borrowing Klass) async { // expected-error {{'x' is borrowed and cannot be consumed}}
74-
await transferToMain(x) // expected-warning {{task-isolated value of type 'Klass' transferred to main actor-isolated context}}
75+
await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
76+
// expected-note @-1 {{transferring task-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
7577
print(x) // expected-note {{consumed here}}
7678
}
7779

7880
@CustomActor func testBorrowingErrorGlobalActor(_ x: borrowing Klass) async { // expected-error {{'x' is borrowed and cannot be consumed}}
79-
await transferToMain(x) // expected-warning {{global actor 'CustomActor'-isolated value of type 'Klass' transferred to main actor-isolated context}}
81+
await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
82+
// expected-note @-1 {{transferring global actor 'CustomActor'-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and global actor 'CustomActor'-isolated uses}}
8083
print(x) // expected-note {{consumed here}}
8184
}
8285

8386
func testInOut(_ x: inout Klass) async {
8487
await transferToMain(x) // expected-warning {{transferring 'x' may cause a race}}
85-
// TODO: This is wrong. Should say task isolated!
86-
// expected-note @-2 {{transferring task-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
88+
// expected-note @-1 {{transferring task-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
8789
}
8890

8991
func testInOutError(_ x: inout Klass) async {

test/Concurrency/transfernonsendable_strong_transferring_params.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ func testNonStrongTransferDoesntMerge() async {
107107

108108
func testTransferringParameter_canTransfer(_ x: transferring Klass, _ y: Klass) async {
109109
await transferToMain(x)
110-
await transferToMain(y) // expected-warning {{task-isolated value of type 'Klass' transferred to main actor-isolated context; later accesses to value could race}}
110+
await transferToMain(y) // expected-warning {{transferring 'y' may cause a race}}
111+
// expected-note @-1 {{transferring task-isolated 'y' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
111112
}
112113

113114
func testTransferringParameter_cannotTransferTwice(_ x: transferring Klass, _ y: Klass) async {
@@ -127,7 +128,8 @@ actor MyActor {
127128

128129
func canTransferWithTransferringMethodArg(_ x: transferring Klass, _ y: Klass) async {
129130
await transferToMain(x)
130-
await transferToMain(y) // expected-warning {{actor-isolated value of type 'Klass' transferred to main actor-isolated context; later accesses to value could race}}
131+
await transferToMain(y) // expected-warning {{transferring 'y' may cause a race}}
132+
// expected-note @-1 {{transferring actor-isolated 'y' to main actor-isolated callee could cause races between main actor-isolated and actor-isolated uses}}
131133
}
132134

133135
func getNormalErrorIfTransferTwice(_ x: transferring Klass) async {
@@ -206,7 +208,8 @@ func assigningIsAMergeError(_ x: transferring Klass) async {
206208
x = y
207209

208210
// We can still transfer y since x is disconnected.
209-
await transferToMain(y) // expected-warning {{transferring value of non-Sendable type 'Klass' from nonisolated context to main actor-isolated context}}
211+
await transferToMain(y) // expected-warning {{transferring 'y' may cause a race}}
212+
// expected-note @-1 {{'y' is transferred from nonisolated caller to main actor-isolated callee}}
210213

211214
useValue(x) // expected-note {{access here could race}}
212215
}
@@ -312,7 +315,8 @@ func doubleArgument() async {
312315

313316
func testTransferSrc(_ x: transferring Klass) async {
314317
let y = Klass()
315-
await transferToMain(y) // expected-warning {{transferring value of non-Sendable type 'Klass' from nonisolated context to main actor-isolated context}}
318+
await transferToMain(y) // expected-warning {{transferring 'y' may cause a race}}
319+
// expected-note @-1 {{'y' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
316320
x = y // expected-note {{access here could race}}
317321
}
318322

test/Concurrency/transfernonsendable_warning_until_swift6.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ func transferValue<T>(_ t: transferring T) {}
2020

2121
func testIsolationError() async {
2222
let x = NonSendableType()
23-
await transferToMain(x) // expected-error {{transferring value of non-Sendable type 'NonSendableType' from nonisolated context to main actor-isolated context; later accesses could race}}
23+
await transferToMain(x) // expected-error {{transferring 'x' may cause a race}}
24+
// expected-note @-1 {{'x' is transferred from nonisolated caller to main actor-isolated callee. Later uses in caller could race with potential uses in callee}}
2425
useValue(x) // expected-note {{access here could race}}
2526
}
2627

2728
func testTransferArgumentError(_ x: NonSendableType) async {
28-
await transferToMain(x) // expected-error {{task-isolated value of type 'NonSendableType' transferred to main actor-isolated context; later accesses to value could race}}
29+
await transferToMain(x) // expected-error {{transferring 'x' may cause a race}}
30+
// expected-note @-1 {{transferring task-isolated 'x' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
2931
}
3032

3133
func testPassArgumentAsTransferringParameter(_ x: NonSendableType) async {

test/SILOptimizer/moveonly_addresschecker_diagnostics_library_evolution.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public func callerConsumeClassLetFieldForArgumentSpillingTestInOutArg(_ x: inout
103103

104104
// TODO: more specific error message path than "unknown"
105105
public func callerConsumeClassLetFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
106-
consumeVal(x.letS.e) // expected-error{{cannot partially consume 'unknown'}}
106+
consumeVal(x.letS.e) // expected-error{{cannot partially consume 'x.letS'}}
107107
}
108108

109109
public func callerConsumeClassLetFieldForArgumentSpillingTestLetGlobal() {
@@ -170,7 +170,7 @@ public func callerConsumeClassVarFieldForArgumentSpillingTestInOutArg(_ x: inout
170170

171171
// TODO: more precise error path reporting than 'unknown'
172172
public func callerConsumeClassVarFieldForArgumentSpillingTestConsumingArg(_ x: consuming CopyableKlass) {
173-
consumeVal(x.varS.e) // expected-error{{cannot partially consume 'unknown'}}
173+
consumeVal(x.varS.e) // expected-error{{cannot partially consume 'x.varS'}}
174174
}
175175

176176
public func callerConsumeClassVarFieldForArgumentSpillingTestLetGlobal() {

test/SILOptimizer/variable_name_inference.sil

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,3 +597,30 @@ bb8:
597597
debug_value [trace] %4 : $FirstSecondThirdEnum<Klass>
598598
return %4 : $FirstSecondThirdEnum<Klass>
599599
}
600+
601+
// Test that we can properly handle code generation from a borrowing loadable parameter
602+
//
603+
// CHECK-LABEL: begin running test 1 of 1 on loadable_borrowing_test: variable-name-inference with: @trace[0]
604+
// CHECK: Input Value: %8 = store_borrow %6 to %7 : $*Klass // user: %9
605+
// CHECK: Name: 'x'
606+
// CHECK: Root: %3 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2
607+
// CHECK: end running test 1 of 1 on loadable_borrowing_test: variable-name-inference with: @trace[0]
608+
sil hidden [ossa] @loadable_borrowing_test : $@convention(thin) @async (@guaranteed Klass) -> () {
609+
bb0(%0 : @noImplicitCopy @guaranteed $Klass):
610+
specify_test "variable-name-inference @trace[0]"
611+
%1 = copyable_to_moveonlywrapper [guaranteed] %0 : $Klass
612+
%2 = copy_value %1 : $@moveOnly Klass
613+
%3 = mark_unresolved_non_copyable_value [no_consume_or_assign] %2 : $@moveOnly Klass
614+
debug_value %3 : $@moveOnly Klass, let, name "x", argno 1
615+
%7 = begin_borrow %3 : $@moveOnly Klass
616+
%8 = moveonlywrapper_to_copyable [guaranteed] %7 : $@moveOnly Klass
617+
%9 = alloc_stack $Klass
618+
%10 = store_borrow %8 to %9 : $*Klass
619+
debug_value [trace] %10 : $*Klass
620+
end_borrow %10 : $*Klass
621+
dealloc_stack %9 : $*Klass
622+
end_borrow %7 : $@moveOnly Klass
623+
destroy_value %3 : $@moveOnly Klass
624+
%25 = tuple ()
625+
return %25 : $()
626+
}

0 commit comments

Comments
 (0)