Skip to content

Fixing issues in analysis around generics #1351

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
Dec 9, 2024

Conversation

grantnelson-wf
Copy link
Collaborator

@grantnelson-wf grantnelson-wf commented Nov 21, 2024

While I was doing other work I ran into a few issues and fixed them:

  1. Unused Generic Functions: Unused generic functions and methods that perform calls were making requests with the type parameters, e.g. func Foo[T any]() { Bar[T]() } will make a request for the function information for Bar[T] with T being the type parameter. This was because if there were no instances, I assumed that meant that it was a non-generic function, but I totally missed the possibility that it is a generic function that just isn't used. I now check for instances and check if the function is generic and if it is unused and generic, skip over the function and don't bother making a FuncInfo for it. Since it is unused, it can't be needed by anything else other than other unused functions.

  2. Function Literal Instances: Function literals that are inside a generic function were not getting the instance information, e.g. func Foo[T any]() { func() { Bar[T]() }() } will make a request for the function information for Bar[T] with T being the type parameter. This was because when the function literal FuncInfo is created it wasn't getting the type arguments from the instance of the generic function that it is inside of. I changed the function literal to take on the instance it is inside of. To make this work I also made the function literal FuncInfo lookup require the type arguments of the specific instance of the function literal to lookup.

  3. Delay Remote Calls: I make the IsBlocking method able to use the isImportedBlocking function if the instance of the function that is being looked up isn't from the current Info. With this change I could also delay the lookup of other remote calls and defers until the end of the analysis when function information is propagating. This change will help later for getting generic changes across package boundaries.

These changes will help prepare for some of the changes needed to make function lookup in the ImportContext look for the correct instance of a generic function Decl. This is related to #1013 and #1270

@grantnelson-wf grantnelson-wf self-assigned this Nov 21, 2024
@grantnelson-wf grantnelson-wf marked this pull request as ready for review November 21, 2024 19:31
grantnelson-wf added a commit to Workiva/gopherjs that referenced this pull request Nov 21, 2024
Copy link
Member

@flimzy flimzy left a comment

Choose a reason for hiding this comment

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

LGTM, but @nevkontakte may have additional comments.

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.

Thank you! 🙏

@grantnelson-wf grantnelson-wf merged commit a3e015c into gopherjs:master Dec 9, 2024
10 checks passed
@grantnelson-wf grantnelson-wf deleted the fixAnalysisGen branch December 9, 2024 17:45
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.

3 participants