Skip to content

[Distributed] Add name parameter to recordArgument for better interop #41799

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 4 commits into from
Mar 18, 2022

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 14, 2022

This allows better interop with various other targets, for example by storing invocation arguments along with their labels if the target of the call expects a JSON object (non swift targets come to mind).

We can also store the names or use them in diagnosis errors and more.

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Mar 14, 2022
@ktoso ktoso force-pushed the wip-argumentNameRecording branch from 7ec88ea to 54b9480 Compare March 14, 2022 02:07
@ktoso
Copy link
Contributor Author

ktoso commented Mar 14, 2022

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 15, 2022

Unblocked by swiftlang/swift-syntax#371

@ktoso ktoso added swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review and removed swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review labels Mar 15, 2022
@ktoso ktoso force-pushed the wip-argumentNameRecording branch from 54b9480 to 5a5d1ba Compare March 17, 2022 13:55
@ktoso
Copy link
Contributor Author

ktoso commented Mar 17, 2022

RemoteCallArgument implemented 🎉

Was more painful than I would have expected but we managed thanks to @hamishknight's amazing help and hints how to apply the generic type info properly there.

Thanks and I'm going ahead with merging this though the exact details are pending evolution -- maybe we'll change names of labels or something, but this API is definitely much better than previous iteration.

@ktoso
Copy link
Contributor Author

ktoso commented Mar 17, 2022

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 17, 2022

@swift-ci please smoke test Linux

@ktoso
Copy link
Contributor Author

ktoso commented Mar 18, 2022

@swift-ci please smoke test windows

@ktoso ktoso merged commit 61da992 into swiftlang:main Mar 18, 2022
@ktoso ktoso deleted the wip-argumentNameRecording branch March 18, 2022 01:59
@shinancao
Copy link

shinancao commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants