Skip to content

[Distributed] Correct generic parameter handling in distributed func accessor (debug builds) #68859

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

Closed

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Sep 29, 2023

The distributed func accessor does not need to carry forward generic parameters EXCEPT the Decoder and the self type. We adjust the implementation to pass those and correct handling of those parameters. This will now no longer produce crashes when distributed funcs are used in nested generic types in debug builds. This does not affect stable releases because those assertions are disabled there, and don't actually happen to cause issues here. But we should do the right thing in any case.

Resolves rdar://115497090
Resolves #68517

Old analysis: If we don't do this, the generic parameter has no requirements at all, which a) is incorrect to begin with, it should have the exact same signature as the method it is the accessor for, and b) it would trip up verification which is enabled on linux in snapshot builds -- causing crashes.

@ktoso ktoso requested review from xedin and slavapestov September 29, 2023 06:26
@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Sep 29, 2023
@ktoso ktoso force-pushed the wip-generic-reqs-from-actor-da-for-accessor branch from cb34c82 to 17e6ac3 Compare October 4, 2023 21:27
@ktoso ktoso requested a review from hborla as a code owner October 4, 2023 21:27
@ktoso ktoso force-pushed the wip-generic-reqs-from-actor-da-for-accessor branch 2 times, most recently from 3729dca to 3af6777 Compare October 5, 2023 01:59
@ktoso
Copy link
Contributor Author

ktoso commented Oct 5, 2023

Okey, this should be good to go now -- we don't actually want generics from the target on the accessor, but along the way we fixed how we handle the accessor signature somewhat. This has no wire impact, manglings remain the same -- just the accessor changed which isn't exposed externally.

Testing all the confs is difficult for that so I expect a CI failure that I will follow up

@ktoso
Copy link
Contributor Author

ktoso commented Oct 5, 2023

@swift-ci please test

@ktoso ktoso changed the title [Distributed] Generic reqs must be forwarded to accessor from enclosing actor [Distributed] Correct generic parameter handling in distributed func accessor (debug builds) Oct 5, 2023
@ktoso ktoso force-pushed the wip-generic-reqs-from-actor-da-for-accessor branch from 3af6777 to e09411f Compare November 27, 2023 10:28
@ktoso
Copy link
Contributor Author

ktoso commented Nov 27, 2023

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Nov 27, 2023

We landed a bunch of other fixes with #68517 so now this PR seems is all good.

@ktoso
Copy link
Contributor Author

ktoso commented Nov 28, 2023

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Nov 28, 2023

Pending on #70057 to unbreak CI

@ktoso
Copy link
Contributor Author

ktoso commented Dec 4, 2023

@swift-ci please smoke test

hmm why is this failing on CI; lost the job need to re-run

@ktoso ktoso force-pushed the wip-generic-reqs-from-actor-da-for-accessor branch from 6330df7 to df2acf2 Compare December 8, 2023 09:13
@ktoso
Copy link
Contributor Author

ktoso commented Dec 8, 2023

@swift-ci please smoke test

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.

Swift 5.9 compiler crash on Linux when compile a generic distributed actor with type constraints
3 participants