Skip to content

[Concurrency] Remove _unsafeInheritExecutor from public APIs, use #isolation #72578

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
merged 3 commits into from
Apr 5, 2024

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 26, 2024

We should be using the #isolation macro in all APIs where _unsafeInheritExecutor was used before;

This handles it just for the continuation APIs, more to follow soon.

resolves rdar://125307764
resolves #69659


It seems it runs into availability issues though?

/Users/ktoso/code/swift-project/swift/stdlib/public/Concurrency/CheckedContinuation.swift:292:40: error: 'isolation()' is only available in macOS 10.15 or newer
╰─ /Users/ktoso/code/swift-project/swift/stdlib/public/Concurrency/CheckedContinuation.swift:292:39: note: expanded code originates here
//    @available(SwiftStdlib 5.1, *) <<<<<<< this is macOS 10.15 (SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0)
//    @inlinable
//    @_unavailableInEmbedded
//    public func withCheckedContinuation<T>(
//      isolation: isolated (any Actor)? = #isolation,
//      function: String = #function,
290 │   _ body: (CheckedContinuation<T, Never>) -> Void
291 │ ) async -> T {
292 │   return await withUnsafeContinuation { <<<<<<< this now has #isolation
    ╭─── /Users/ktoso/code/swift-project/swift/stdlib/public/Concurrency/CheckedContinuation.swift
    │290 │
    │291 │
    │292 │                                       #isolation
    │    │                                        ╰─ error: 'isolation()' is only available in macOS 10.15 or newer
    ╰──────────────────────────────────────────────────────────────────────────────────────────────
293 │     body(CheckedContinuation(continuation: $0, function: function))
294 │   }

where the unsafe is:

@available(SwiftStdlib 5.1, *)
@_alwaysEmitIntoClient // removing this doesn't matter
public func withUnsafeContinuation<T>(
  isolation: isolated (any Actor)? = #isolation,
  _ fn: (UnsafeContinuation<T, Never>) -> Void
) async -> T {
  return await Builtin.withUnsafeContinuation {
    fn(UnsafeContinuation<T, Never>($0))
  }
}

@ktoso ktoso requested a review from a team as a code owner March 26, 2024 02:59
@ktoso ktoso requested review from DougGregor and hborla March 26, 2024 02:59
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Mar 26, 2024
@ktoso
Copy link
Contributor Author

ktoso commented Apr 3, 2024

Unblocked by #72813

@ktoso ktoso force-pushed the wip-withUncheckedContinuation-trouble branch from 7c63adf to bf27392 Compare April 4, 2024 14:38
@ktoso
Copy link
Contributor Author

ktoso commented Apr 4, 2024

Now this still needs some work about backdeployed + unavailable in embedded I think, otherwise it'd be good to go

@ktoso ktoso requested a review from tshortli April 4, 2024 14:39
@ktoso ktoso force-pushed the wip-withUncheckedContinuation-trouble branch 2 times, most recently from f5aea30 to 5e63b09 Compare April 5, 2024 02:03
@ktoso
Copy link
Contributor Author

ktoso commented Apr 5, 2024

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Apr 5, 2024

So it turns out solving this issue took a few fixes in related language features and quite some time, but the hackathon is where we kicked off the diagnosis and fixes, so I'd like to thank you @kntkymt for collaborating on this back then!

@ktoso ktoso changed the title [Concurrency] Correct withContinuation APIs isolation handling [Concurrency] Remove _unsafeInheritExecutor from public APIs, use #isolation Apr 5, 2024
@ktoso ktoso force-pushed the wip-withUncheckedContinuation-trouble branch from 5e63b09 to 1bd0463 Compare April 5, 2024 07:47
@ktoso
Copy link
Contributor Author

ktoso commented Apr 5, 2024

@swift-ci please smoke test

@ktoso ktoso enabled auto-merge (squash) April 5, 2024 08:06
@ktoso ktoso merged commit dfcf105 into swiftlang:main Apr 5, 2024
@ktoso ktoso deleted the wip-withUncheckedContinuation-trouble branch April 16, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

withUnsafeContinuation doesn't inherit Actor executor during single await call
1 participant