Skip to content

[Demangler] Fix isThunkSymbol() to handle continuations. #61330

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 1 commit into from
Sep 30, 2022

Conversation

al45tair
Copy link
Contributor

isThunkSymbol() was returning false for await resume and suspend resume thunks because the Node tree for those has an AsyncAwaitResumePartialFunction and/or AsyncSuspendResumePartialFunction as the first child of the top level Global, with the actual thunk in the second child location.

rdar://100424460

@al45tair
Copy link
Contributor Author

@swift-ci Please test

@al45tair al45tair requested a review from eeckstein September 28, 2022 15:36
@al45tair
Copy link
Contributor Author

TODO: I need to update the Demangle/demangle.swift test cases with some examples for this.

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

And yes, please add a test

@al45tair al45tair force-pushed the eng/PR-100424460 branch 2 times, most recently from 5d63dfe to dd6d6e0 Compare September 29, 2022 11:15
…uations.

`isThunkSymbol()` was returning false for await resume and suspend resume thunks
because the `Node` tree for those has an `AsyncAwaitResumePartialFunction`
and/or `AsyncSuspendResumePartialFunction` as the first child of the top level
`Global`, with the actual thunk in the _second_ child location.

rdar://100424460
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

Added some tests and fixed a bug they exposed, which also made the fix simpler :-)

@al45tair al45tair merged commit 93c7643 into swiftlang:main Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants