[6.0] Add isolation argument to functions taking non-sendable async closures.
#643
Conversation
This file contains 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
…res. (#624) This PR adds an `isolation` parameter to several API functions that take `async` closures that are not required to be sendable. As well, it adds `sending` to those closures' return types where appropriate. This change is necessary in Swift 6 because a non-sendable async closure could be called in the wrong isolation domain otherwise. In particular, if the caller is `@MainActor`-isolated, the closure will not be called on the main actor and will therefore always hop, and a concurrency error occurs: ```swift @mainactor func f() async { await confirmation { // this inner closure is not actor-isolated, but can't be sent across // isolation domains. } } ``` `withKnownIssue()` and `confirmation()` are affected, as are several async overloads of the internal-but-public `__check()` function family. This change is necessary for correctness, but is potentially source-breaking if calling code directly references the modified functions by full name. Resolves #622. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
grynspan
added
bug
grynspan
requested review from
stmontgomery,
dennisweissmann,
briancroom,
SeanROlszewski and
suzannaratcliff
as code owners
|
@swift-ci please test |
grynspan
changed the title
isolation argument to functions taking non-sendable async closures.isolation argument to functions taking non-sendable async closures.
|
Linux CI failure is just due to duelling PRs. |
|
@swift-ci test Linux |
briancroom
approved these changes
Aug 26, 2024
grynspan
deleted the
jgrynspan/add-isolation-argument-to-various-functions-6.0
branch
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: Fixes concurrency issues using
confirmation {},withKnownIssue {}, and#expect(throws:) {}with actor-isolated code.Scope: 6.0 branch (6.0.1)
Issue: https://github.com/swiftlang/swift-testing/issue/622
Original PR: #624
Risk: Moderate—involves changing the signatures of a number of functions which will break ABI for them as well as potential source breakage if somebody explicitly refers to these functions by name.
Testing: New unit test coverage.
Reviewer: @briancroom