Skip to content

Adding DCE integration tests in compiler #1343

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 3 commits into from
Oct 16, 2024

Conversation

grantnelson-wf
Copy link
Collaborator

@grantnelson-wf grantnelson-wf commented Sep 25, 2024

This adds tests for the dead-code elimination (DCE) package into the compiler. The compiler performs many tasks that are related to DCE such as setting Decl's names and dependencies. These tests are designed like an integration test to ensure that the correct information is being given to the DCE and that the DCE is doing the correct things with that information.

This will introduce some TODO's that are resolved by the next part #1340. These TODO's exemplify the changes needed to be made to the DCE to properly handle generics and type instances. Double check that I'm not taking crazy pills with these TODO's and that they make sense to be the thing that needs to be fixed in the next part.

This is related to #1013 and #1270


🍰 As an added bonus; I upgraded the existing test to stop using the deprecated golang.org/x/tools/go/loader and instead start using golang.org/x/tools/go/packages. It's nothing that exciting since it only is for source code for strings, but it's a start 😄

@grantnelson-wf grantnelson-wf marked this pull request as ready for review September 26, 2024 17:37
Copy link
Member

@nevkontakte nevkontakte left a comment

Choose a reason for hiding this comment

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

Thanks, this is excellent work! Some test cases revealed DCE behavior that was surprising, even though it made sense in the end.

Copy link
Member

@nevkontakte nevkontakte left a comment

Choose a reason for hiding this comment

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

Overall I'm happy with this PR. If you don't think that generating stable identifiers for Decls is worth it, I'll merge it as-is.

@nevkontakte nevkontakte merged commit 4dc2318 into gopherjs:master Oct 16, 2024
10 checks passed
@grantnelson-wf grantnelson-wf deleted the dceIntegrationTests branch October 17, 2024 14:43
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