-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[transfer-non-sendable] Teach SILIsolationInfo how to handle "look through instructions" when finding actor instances. #74673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[transfer-non-sendable] Teach SILIsolationInfo how to handle "look through instructions" when finding actor instances. #74673
Conversation
…rough instructions" when finding actor instances. From the perspective of the IR, we are changing SILIsolationInfo such that inferring an actor instance means looking at equivalence classes of values where we consider operands to look through instructions to be equivalent to their dest value. The result is that cases where the IR maybe puts in a copy_value or the like, we consider the copy_value to have the same isolation info as using the actor directly. This prevents a class of crashes due to merge failings. Example: ```swift actor MyActor { init() async { init(ns: NonSendableKlass) async { self.k = NonSendableKlass() self.helper(ns) } func helper(_ newK: NonSendableKlass) {} } ``` Incidently, we already had a failing test case from this behavior rather than the one that was the original genesis. Specifically: 1. If a function's SILIsolationInfo is the same as the isolation info of a SILValue, we assume that no transfer actually occurs. 2. Since we were taking too static of a view of actor instances when comparing, we would think that a SILIsolationInfo of a #isolation parameter to as an argument would be different than the ambient's function isolation which is also that same one. So we would emit a transfer non transferrable error if we pass in any parameters of the ambient function into another isolated function. Example: ```swift actor Test { @TaskLocal static var local: Int? func withTaskLocal(isolation: isolated (any Actor)? = #isolation, _ body: (consuming NonSendableValue, isolated (any Actor)?) -> Void) async { Self.$local.withValue(12) { // We used to get these errors here since we thought that body's isolation // was different than the body's isolation. // // warning: sending 'body' risks causing data races // note: actor-isolated 'body' is captured by a actor-isolated closure... body(NonSendableValue(), isolation) } } } ``` rdar://129400019
@swift-ci smoke test |
|
||
// TODO: This crashes the compiler since we attempt to hop_to_executor to the | ||
// value that is materialized onto disk. | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While doing this list of look through, I wanted to see how having consuming/borrowing as actor self worked... and I found in the consuming case we crashed. I am going to fix that in a different PR. But I thought it was worth putting in tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, thanks for followup then as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
From the perspective of the IR, we are changing SILIsolationInfo such that
inferring an actor instance means looking at equivalence classes of values where
we consider operands to look through instructions to be equivalent to their dest
value. The result is that cases where the IR maybe puts in a copy_value or the
like, we consider the copy_value to have the same isolation info as using the
actor directly. This prevents a class of crashes due to merge failings. Example:
Incidently, we already had a failing test case from this behavior rather than
the one that was the original genesis. Specifically:
If a function's SILIsolationInfo is the same as the isolation info of a
SILValue, we assume that no transfer actually occurs.
Since we were taking too static of a view of actor instances when comparing,
we would think that a SILIsolationInfo of a #isolation parameter to as an
argument would be different than the ambient's function isolation which is also
that same one. So we would emit a transfer non transferrable error if we pass in
any parameters of the ambient function into another isolated function. Example:
rdar://129400019