Skip to content

Commit cb6ac6c

Browse files
committed
[region-isolation] Make sure to look through begin_access and don't treat it like an assignment.
If we treat a begin_access as an assignment then in cases like below we assume that a value was used before we reassign. ``` func testVarReassignStopActorDerived() async { var closure = {} await transferToMain(closure) // This re-assignment shouldn't error. closure = {} // (1) } ``` rdar://117437299
1 parent e1b17b9 commit cb6ac6c

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-4
lines changed

lib/SILOptimizer/Mandatory/SendNonSendable.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,6 @@ class PartitionOpTranslator {
885885
// treating some of these as lookthroughs in getUnderlyingTrackedValue
886886
// instead of as assignments
887887
case SILInstructionKind::AddressToPointerInst:
888-
case SILInstructionKind::BeginAccessInst:
889888
case SILInstructionKind::CopyValueInst:
890889
case SILInstructionKind::ConvertEscapeToNoEscapeInst:
891890
case SILInstructionKind::ConvertFunctionInst:
@@ -1070,6 +1069,25 @@ class PartitionOpTranslator {
10701069
case SILInstructionKind::YieldInst: // TODO: yield should be handled
10711070
return;
10721071

1072+
// We ignore begin_access since we look through them. We look through them
1073+
// since we want to treat the use of the begin_access as the semantic giving
1074+
// instruction. Otherwise, if we have a store after a consume we will emit
1075+
// an error on the begin_access rather than allowing for the store to
1076+
// overwrite the original value. This would then cause an error.
1077+
//
1078+
// Example:
1079+
//
1080+
// %0 = alloc_stack
1081+
// store %1 to %0
1082+
// apply %transfer(%0)
1083+
// %2 = begin_access [modify] [static] %0
1084+
// store %2 to %0
1085+
//
1086+
// If we treated a begin_access as an assign, we would require %0 to not be
1087+
// transferred at %2 even though we are about to overwrite it.
1088+
case SILInstructionKind::BeginAccessInst:
1089+
return;
1090+
10731091
default:
10741092
break;
10751093
}

test/Concurrency/sendnonsendable_basic.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,20 @@ extension Actor {
174174
let closure: () -> () = {
175175
print(self.klass)
176176
}
177-
let x = (closure, 1)
177+
let x = (1, closure)
178178
await transferToMain(x) // expected-sns-warning {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
179+
// expected-complete-warning @-1 {{passing argument of non-sendable type '(Int, () -> ())' into main actor-isolated context may introduce data races}}
180+
// expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
181+
}
182+
183+
func simpleClosureCaptureSelfAndTransferThroughTupleBackwards() async {
184+
let closure: () -> () = {
185+
print(self.klass)
186+
}
187+
// NOTE: We do not error on this today since we assign into 1 and that makes
188+
// x assign fresh. It will be fixed in a forthcoming commit.
189+
let x = (closure, 1)
190+
await transferToMain(x)
179191
// expected-complete-warning @-1 {{passing argument of non-sendable type '(() -> (), Int)' into main actor-isolated context may introduce data races}}
180192
// expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
181193
}
@@ -451,11 +463,11 @@ extension Actor {
451463

452464
// This re-assignment shouldn't error.
453465
closure = {}
454-
await transferToMain(closure)
466+
await transferToMain(closure) // expected-sns-warning {{passing argument of non-sendable type '() -> ()' from actor-isolated context to main actor-isolated context at this call site could yield a race with accesses later in this function}}
455467
// expected-complete-warning @-1 {{passing argument of non-sendable type '() -> ()' into main actor-isolated context may introduce data races}}
456468
// expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
457469

458470
// But this will error since we race.
459-
closure()
471+
closure() // expected-sns-note {{access here could race}}
460472
}
461473
}

0 commit comments

Comments
 (0)