[5.6] Hop to the generic executor in non-actor-isolated async functions #41055
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
|
@swift-ci Please test |
rjmccall
changed the title
rjmccall
changed the title
|
Build failed |
|
Build failed |
Async functions are now expected to set ExpectedExecutor in their prologue (and, generally, immediately hop to it). I updated the prologue code for a bunch of function emission, most of which was uninteresting. Top-level code was not returning to the main executor, which is now fixed; fortunately, we weren't assuming that we were on the main executor yet. We had some code that only kicked in when an ExpectedExecutor wasn't set which made us capture the current executor before a hop and then return to it later. This code has been removed; there's no situation in which save-and-return is the semantically correct thing to do given the possibility of hop optimization. I suspect it could also have led to crashes if the current executor is being kept alive only because it's currently running code. If we ever add async functions that are supposed to inherit their caller's executor, we should have the caller pass the right executor down to it. This is the first half of SE-0338; the second, sendability enforcement, is much more complicated, and Doug has volunteered to do it. Fixes rdar://79284465, as well as some tests that were XFAILed on Windows.
rjmccall
force-pushed
the
hop-to-generic-executor-5.6
branch
from
35e5b52 to
0106bde
Compare
|
@swift-ci Please test |
|
Build failed |
|
Please test with pull request: @swift-ci Please test macOS |
DougGregor
approved these changes
Jan 28, 2022
kavon
approved these changes
Jan 29, 2022
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.
Async functions are now expected to set ExpectedExecutor in their prologue (and, generally, immediately hop to it). I updated the prologue code for a bunch of function emission, most of which was uninteresting. Top-level code was not returning to the main executor, which is now fixed; fortunately, we weren't assuming that we were on the main executor yet.
We had some code that only kicked in when an ExpectedExecutor wasn't set which made us capture the current executor before a hop and then return to it later. This code has been removed; there's no situation in which save-and-return is the semantically correct thing to do given the possibility of hop optimization. I suspect it could also have led to crashes if the current executor is being kept alive only because it's currently running code. If we ever add async functions that are supposed to inherit their caller's executor, we should have the caller pass the right executor down to it.
This is the first half of SE-0338; the second, sendability enforcement, is much more complicated, and Doug has volunteered to do it.
Fixes rdar://79284465, as well as some tests that were XFAILed on Windows.
This is the 5.6 version of #40910.
Explanation: fixes a widely-reported semantic problem with non-actor-isolated async functions
Scope: affects the emission of SIL for all non-actor-isolated async functions
Risk: moderate because of the scope; however, the resulting SIL closely resembles that used by actor-isolated async functions, so there shouldn't be huge surprises waiting here. Biggest risk is that there are uncommon paths for generating special async functions that are somehow untested in the test suite
Testing: common code path extensively tested by the test suite
Reviewer: @DougGregor (on main PR)