-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[rbi] Teach RegionIsolation how to properly error when 'inout sending' params are returned. #83903
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…asier. I am going to use this in the next commit. So it makes sense to hoist it as a separate commit to make it easy to just look through the diff.
@swift-ci smoke test |
…' params are returned. We want 'inout sending' parameters to have the semantics that not only are they disconnected on return from the function but additionally they are guaranteed to be in their own disconnected region on return. This implies that we must emit errors when an 'inout sending' parameter or any element that is in the same region as the current value within an 'inout sending' parameter is returned. This commit contains a new diagnostic for RegionIsolation that adds specific logic for detecting and emitting errors in these situations. To implement this, we introduce 3 new diagnostics with each individual diagnostic being slightly different to reflect the various ways that this error can come up in source: * Returning 'inout sending' directly: ```swift func returnInOutSendingDirectly(_ x: inout sending NonSendableKlass) -> NonSendableKlass { return x // expected-warning {{cannot return 'inout sending' parameter 'x' from global function 'returnInOutSendingDirectly'}} // expected-note @-1 {{returning 'x' risks concurrent access since caller assumes that 'x' and the result of global function 'returnInOutSendingDirectly' can be safely sent to different isolation domains}} } ``` * Returning a value in the same region as an 'inout sending' parameter. E.x.: ```swift func returnInOutSendingRegionVar(_ x: inout sending NonSendableKlass) -> NonSendableKlass { var y = x y = x return y // expected-warning {{cannot return 'y' from global function 'returnInOutSendingRegionVar'}} // expected-note @-1 {{returning 'y' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' and the result of global function 'returnInOutSendingRegionVar' can be safely sent to different isolation domains}} } ``` * Returning the result of a function or computed property that is in the same region as the 'inout parameter'. ```swift func returnInOutSendingViaHelper(_ x: inout sending NonSendableKlass) -> NonSendableKlass { let y = x return useNonSendableKlassAndReturn(y) // expected-warning {{cannot return result of global function 'useNonSendableKlassAndReturn' from global function 'returnInOutSendingViaHelper'}} // expected-note @-1 {{returning result of global function 'useNonSendableKlassAndReturn' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' and the result of global function 'returnInOutSendingViaHelper' can be safely sent to different isolation domains}} } ``` Additionally, I had to introduce a specific variant for each of these diagnostics for cases where due to us being in a method, we are actually in our caller causing the 'inout sending' parameter to be in the same region as an actor isolated value: * Returning 'inout sending' directly: ```swift extension MyActor { func returnInOutSendingDirectly(_ x: inout sending NonSendableKlass) -> NonSendableKlass { return x // expected-warning {{cannot return 'inout sending' parameter 'x' from instance method 'returnInOutSendingDirectly'}} // expected-note @-1 {{returning 'x' risks concurrent access since caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingDirectly' is 'self'-isolated}} } } ``` * Returning a value in the same region as an 'inout sending' parameter. E.x.: ```swift extension MyActor { func returnInOutSendingRegionLet(_ x: inout sending NonSendableKlass) -> NonSendableKlass { let y = x return y // expected-warning {{cannot return 'y' from instance method 'returnInOutSendingRegionLet'}} // expected-note @-1 {{returning 'y' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingRegionLet' is 'self'-isolated}} } } ``` * Returning the result of a function or computed property that is in the same region as the 'inout parameter'. ```swift extension MyActor { func returnInOutSendingViaHelper(_ x: inout sending NonSendableKlass) -> NonSendableKlass { let y = x return useNonSendableKlassAndReturn(y) // expected-warning {{cannot return result of global function 'useNonSendableKlassAndReturn' from instance method 'returnInOutSendingViaHelper'; this is an error in the Swift 6 language mode}} // expected-note @-1 {{returning result of global function 'useNonSendableKlassAndReturn' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingViaHelper' is 'self'-isolated}} } } ``` To implement this, I used two different approaches depending on whether or not the returned value was generic or not. * Concrete In the case where we had a concrete value, I was able to in simple cases emit diagnostics based off of the values returned by the return inst. In cases where we phied together results due to multiple results in the same function, we determine which of the incoming phied values caused the error by grabbing the exit partition information of each of the incoming value predecessors and seeing if an InOutSendingAtFunctionExit would emit an error. * Generic In the case of generic code, it is a little more interesting since the result is a value stored in an our parameter instead of being a value directly returned by a return inst. To work around this, I use PrunedLiveness to determine the last values stored into the out parameter in the function to avoid having to do a full dataflow. Then I take the exit blocks where we assign each of those values and run the same check as we do in the direct phi case to emit the appropriate error. rdar://152454571
ee8e915
to
8745ab0
Compare
@swift-ci smoke test |
I want to tweak these diagnostics a little more, but I am going to do that in a different PR since I want to prevent a merge conflict with another change I am making out of tree. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want 'inout sending' parameters to have the semantics that not only are they
disconnected on return from the function but additionally they are guaranteed to
be in their own disconnected region on return. This implies that we must emit
errors when an 'inout sending' parameter or any element that is in the same
region as the current value within an 'inout sending' parameter is
returned. This commit contains a new diagnostic for RegionIsolation that adds
specific logic for detecting and emitting errors in these situations.
To implement this, we introduce 3 new diagnostics with each individual
diagnostic being slightly different to reflect the various ways that this error
can come up in source:
region as the 'inout parameter'.
Additionally, I had to introduce a specific variant for each of these
diagnostics for cases where due to us being in a method, we are actually in our
caller causing the 'inout sending' parameter to be in the same region as an
actor isolated value:
To implement this, I used two different approaches depending on whether or not
the returned value was generic or not.
In the case where we had a concrete value, I was able to in simple cases emit
diagnostics based off of the values returned by the return inst. In cases where
we phied together results due to multiple results in the same function, we
determine which of the incoming phied values caused the error by grabbing the
exit partition information of each of the incoming value predecessors and seeing
if an InOutSendingAtFunctionExit would emit an error.
In the case of generic code, it is a little more interesting since the result is
a value stored in an our parameter instead of being a value directly returned by
a return inst. To work around this, I use PrunedLiveness to determine the last
values stored into the out parameter in the function to avoid having to do a
full dataflow. Then I take the exit blocks where we assign each of those values
and run the same check as we do in the direct phi case to emit the appropriate
error.
rdar://152454571