Skip to content

🍒[5.7][Distributed] Prevent overloading on async in distributed methods, for wire compat #59881

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

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Jul 5, 2022

Description: In order to be better for wire compatibility, we actually encode the YaK (async throws) distributed thunk based names for the distributed accessible functions. This currently leads to crashes if one does overload on "just asyncness". After discussing if we should allow these overloads (#59831) or ban them (this PR) we came to the conclusion this is better for distributed calls. This PR bans async overloading on distributed methods.
Risk: Low, prevents a crash by preventing bad declarations.
Review by: @DougGregor
Testing: PR testing
Original PR: #59880
Radar: rdar://96179707

@ktoso ktoso requested a review from a team as a code owner July 5, 2022 11:06
@ktoso
Copy link
Contributor Author

ktoso commented Jul 5, 2022

@swift-ci please test

@ktoso ktoso requested review from DougGregor and xedin July 5, 2022 11:09
@ktoso
Copy link
Contributor Author

ktoso commented Jul 5, 2022

This PR does not change the mangling strategy, but just prevents a compile time crash when declaring conflicting methods, as such, it can land a bit later if it has to.

@ktoso ktoso changed the title [Distributed] Prevent overloading on async in distributed methods, for wire compat 🍒[5.7][Distributed] Prevent overloading on async in distributed methods, for wire compat Jul 5, 2022
@ktoso ktoso added r5.7 distributed Feature → concurrency: distributed actor labels Jul 5, 2022
@xedin xedin removed their request for review July 7, 2022 19:04
@ktoso
Copy link
Contributor Author

ktoso commented Jul 27, 2022

@swift-ci please test Linux

@ktoso
Copy link
Contributor Author

ktoso commented Jul 27, 2022

Tho it seems this missed the train -- heh my bad.
Thankfully it does not affect mangling or ABI in any way; it is just a "nicer error message"

@ktoso
Copy link
Contributor Author

ktoso commented Aug 24, 2022

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Aug 24, 2022

Thanks for review Doug!

@ktoso ktoso merged commit d08d898 into swiftlang:release/5.7 Aug 25, 2022
@ktoso ktoso deleted the pick-wip-prevent-async-overloads-dist branch August 25, 2022 00:30
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants