-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[6.2][Concurrency] Change isIsolatingCurrent... to return Bool? #81135
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
ktoso
merged 6 commits into
swiftlang:release/6.2
from
ktoso:pick-22b20e731eaa70746d3c21bf8c44836aa49dfabb
May 1, 2025
Merged
[6.2][Concurrency] Change isIsolatingCurrent... to return Bool? #81135
ktoso
merged 6 commits into
swiftlang:release/6.2
from
ktoso:pick-22b20e731eaa70746d3c21bf8c44836aa49dfabb
May 1, 2025
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
….cpp`"" (swiftlang#80692) * Revert "Revert "Concurrency: Move code between `Executor{Bridge,Impl}.cpp`"" * Update CMakeLists.txt
This changes the isIsolatingCurrentContext function to return `Bool?` and removes all the witness table trickery we did previously to detect if it was implemented or not. This comes at a cost of trying to invoke it always, before `checkIsolated`, but it makes for an simpler implementation and more checkable even by third party Swift code which may want to ask this question. Along with the `withSerialExecutor` function, this now enables us to check the isolation at runtime when we have an `any Actor` e.g. from `#isolation`. Updates SE-0471 according to https://forums.swift.org/t/se-0471-improved-custom-serialexecutor-isolation-checking-for-concurrency-runtime/78834/ review discussions
1bc765a
to
c6e38ce
Compare
@swift-ci please test |
c6e38ce
to
a63356e
Compare
@swift-ci please test |
@swift-ci please test |
1 similar comment
@swift-ci please test |
Meanwhile we decided we won't be doing the withSerialExecutor so I've hidden it right away even though we use it in a test, so folks don't accidentally start using it. This should be good to land though and is "complete" if we want the SE suggested cc @DougGregor |
@swift-ci please test |
DougGregor
approved these changes
May 1, 2025
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.
Description: This changes the isIsolatingCurrentContext function to return
Bool?
and removes all the witness table trickery we did previously to detect if it was implemented or not. This comes at a cost of trying to invoke it always, beforecheckIsolated
, but it makes for an simpler implementation and more checkable even by third party Swift code which may want to ask this question.Along with the
withSerialExecutor
function, this now enables us to check the isolation at runtime when we have anany Actor
e.g. from#isolation
.Updates SE-0471 according to
https://forums.swift.org/t/se-0471-improved-custom-serialexecutor-isolation-checking-for-concurrency-runtime/78834/ review discussions
Scope/Impact: Adjusts protocol requirement which didn't ship nor was adopted yet in other APIs.
Risk: Low, this API wasn't adopted in many places yet.
Testing: CI testing
Reviewed by: @DougGregor
Original PR:
Radar: