-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[6.0][sending] Change CheckedContinuation/AsyncThrowingStream.Continuation APIs to use sending parameters and results. #74097
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
gottesmm
merged 10 commits into
swiftlang:release/6.0
from
gottesmm:release/6.0-128961672
Jun 4, 2024
Merged
[6.0][sending] Change CheckedContinuation/AsyncThrowingStream.Continuation APIs to use sending parameters and results. #74097
gottesmm
merged 10 commits into
swiftlang:release/6.0
from
gottesmm:release/6.0-128961672
Jun 4, 2024
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
(cherry picked from commit f45a87c)
… use the isolated parameter, not self. This was ok in the small since most of the time we were processing a self parameter as isolated... but that isn't always true... (cherry picked from commit 1d8ea84)
…actors as global actors rather than actor instance. The specific example I ran into was in sendable_continuation.swift where we were passing in the @mainactor instance as a sil_isolated parameter. We were thinking it was an actor instance so we emitted the wrong message. (cherry picked from commit 0cd0868)
…pplies merged region as a value, just propagate its isolation. The reason that I am doing this is it ensures that if we have a region isolation merge failure due to a mismatch in between the actual args in the region and the propagated callee isolation, we see it immediately when we translate the apply into the pseudo-IR instead of later when we perform the actual diagnostic emission. This makes it far easier to diagnose these issues since we get an unknown pattern very early which can be asserted on via the option -sil-region-isolation-assert-on-unknown-pattern. (cherry picked from commit b7249e7)
…ce to infer disconnected if the alloc stack is used as a sending indirect result. I also fixed an issue that I found where we were not substituting SILResultInfo flags which was causing us to drop when substituting sil_sending. I added a SILVerifier check to make sure that we do not break this again. (cherry picked from commit c7124e4)
…ding result. (cherry picked from commit b8305cc)
…ng into vars/storage. We want to ensure that functions/methods themselves do not have sending mangled into their names, but we do want sending mangled in non-top level positions. For example: we do not want to mangle sending into a function like the following: ```swift // We don't want to mangle this. func test(_ x: sending NonSendableKlass) -> () ``` But when it comes to actually storing functions into memory, we do want to distinguish in between function values that use sending vs those that do not since we do not want to allow for them to alias. Thus we want to mangle sending into things like the following: ```swift // We want to distinguish in between Array<(sending T) -> ()> and // Array((T) -> ()> let a = Array<(sending T) -> ()> // We want to distinguish in between a global contianing (sending T) -> () and a // global containing (T) -> (). var global: (sending T) -> () ``` This commit achieves that by making changes to the ASTMangler in getDeclType which causes getDeclType to set a flag that says that we have not yet recursed through the system and thus should suppress the printing of sendable. Once we get further into the system and recurse, that flag is by default set to true, so we get the old sending parameter without having to update large amounts of code. rdar://127383107 (cherry picked from commit 2bf497d)
…aired with sending. I am doing this b/c we are going to ban borrowing sending so that we can leave open that space for further design. In the short term, we need the ability to create +0 sending parameters without messing with mangling. By special casing this, we get what we want. rdar://129116141 (cherry picked from commit a890c60)
…E instead of an UPCOMING_FEATURE. TLDR: This makes it so that we always can parse sending/transferring but changes the semantic language effects to be keyed on RegionBasedIsolation instead. ---- The key thing that makes this all work is that I changed all of the "special" semantic changes originally triggered on *ArgsAndResults to now be triggered based on RegionBasedIsolation being enabled. This makes a lot of sense since we want these semantic changes specifically to be combined with the checkers that RegionBasedIsolation turns on. As a result, even though this causes these two features to always be enabled, we just parse it but we do not use it for anything semantically. rdar://128961672 (cherry picked from commit 88729b9) Conflicts: include/swift/Basic/Features.def lib/DriverTool/sil_opt_main.cpp
… APIs to use sending parameters and results. Specifically: 1. CheckedContinuation.resume now takes a sending parameter. 2. Async{Throwing,}Stream.yield takes a sending parameter. 3. withCheckedContinuation returns a transferring parameter. Importantly due to the previous changes around mangling, this is a mangling neutral change. rdar://120420024 (cherry picked from commit ce1dd43)
@swift-ci test |
DougGregor
approved these changes
Jun 3, 2024
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.
Explanation: This PR contains a bunch of work that culminates in adding sending to CheckedContinuation/Async{Throwing,}Stream.Continuation APIs. It also changes withCheckedContinuation to have a sending return value.
To implement this, I had to fix a bunch of blocking issues:
@backDeploy
that does not support silgen_name. So I decided to just move forward some work I already needed to do for mangling so that I did not need to use silgen_name for these APIs. The specific mangling changes is that sending on a top level function does not affect mangling, but in recursive mangling contexts, we do mangle it in. This ensures that we do not mangling sending into:fun c foo(_ x: sending T) { ... }
, but we do mangle it into:let f: (sending T) -> ()
. It also ensures that we do not mangle __shared into the mangling if __shared is combined with sending. This ensures that our adopters do not need to silgen_name if they use this workaround to avoid needing to use borrowing sending.Radars:
Original PRs:
Risk: Low. Just affects sending.
Testing: Added tests to the test suite.
Reviewer: N/A