Skip to content

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
@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