Skip to content

🍒[5.10][Distributed] Don't crash in thunk generation when missing SR conformance #69502

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 Oct 30, 2023

Problem: We would properly diagnose the missing conformance; but the compiler would still try to emit a thunk for the errored func. Tests which only -typecheck -verify would not catch this issue because it is after type checking.
Solution: This prevents distributed thunks from being generated when the func they are based on already has errors -- otherwise we can run into missing types and null pointers as we try to generate the thunk.
Risk: Low, affects only this specific situation and just avoids emitting thunks when they cannot be correct to begin with -- when the funk is broken.
Impact: Only distributed functions; This provides a better failure path - rather than a crash, just the expected error reporting.

Review by: @xedin @slavapestov
Testing: CI testing
Original PR: #69501
Radar: rdar://116261701

Related radar: rdar://117677422

@ktoso ktoso requested a review from a team as a code owner October 30, 2023 11:46
@ktoso ktoso changed the title [Distributed] Don't crash in thunk generation when missing SR conform… 🍒[5.10][Distributed] Don't crash in thunk generation when missing SR conformance Oct 30, 2023
@ktoso
Copy link
Contributor Author

ktoso commented Oct 30, 2023

@swift-ci please test

@ktoso ktoso marked this pull request as draft November 1, 2023 05:01
@ktoso ktoso force-pushed the pick-wip-dont-crash-missing-conformance-param branch from 924bbb9 to 3d47f70 Compare November 14, 2023 11:25
@ktoso
Copy link
Contributor Author

ktoso commented Nov 14, 2023

@swift-ci please test

@ktoso ktoso marked this pull request as ready for review November 14, 2023 14:21
@ktoso
Copy link
Contributor Author

ktoso commented Nov 14, 2023

@swift-ci please test

@ktoso ktoso force-pushed the pick-wip-dont-crash-missing-conformance-param branch from 5ea3f0b to ed25185 Compare November 15, 2023 01:42
@ktoso ktoso force-pushed the pick-wip-dont-crash-missing-conformance-param branch from ed25185 to 7cdeda7 Compare November 15, 2023 01:43
@ktoso
Copy link
Contributor Author

ktoso commented Nov 15, 2023

@swift-ci please test

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Nov 15, 2023
@ktoso ktoso enabled auto-merge November 15, 2023 11:22
@ktoso
Copy link
Contributor Author

ktoso commented Nov 16, 2023

Included another 1 line cleanup from #69901

@ktoso
Copy link
Contributor Author

ktoso commented Nov 16, 2023

@swift-ci please 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.

3 participants