Skip to content
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

x/tools/go/packages: provide alternative to deprecated LoadAllSyntax #32365

Open
dominikh opened this issue May 31, 2019 · 2 comments

Comments

@dominikh
Copy link
Member

commented May 31, 2019

A fair number of tools will need the equivalent of the now deprecated LoadAllSyntax constant. Instead of forcing everyone to write their own bitwise OR of 9 constants, it would be nice if we could provide a constant for this. Unfortunately we can't call it NeedEverything, as LoadAllSyntax doesn't include NeedExportsFile, and including it unnecessarily probably causes unnecessary work by the compiler.

/cc @matloob

@gopherbot gopherbot added this to the Unreleased milestone May 31, 2019

@matloob

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

I'm actually a bit surprised by this. There are that many tools that need to be able to see (almost) all the fields on the package struct? My instinct is to avoid adding a NeedX field that has more than one bit set if we can avoid doing so

@dominikh

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

It's possible that the tools I looked at used LoadAllSyntax out of convenience, not necessity.

However, intuitively, it seems to me that all tools that do full-fledged source based analysis will need the majority of fields in Package. They will need ASTs, they will need to resolve imports, and they will need type information. This amounts to LoadAllSyntax. Tools that need this amount of information include go/analysis runners, pre-go/analysis linters (golint, errcheck, ...) and, to a degree, code manipulation tools like goreturns, though these may not need NeedDeps, but then I'd argue that LoadAllSyntax ^ NeedDeps is easier than ORing 8 constants.

Am I missing something? Do most tools actually need much less information than I am thinking?

@gopherbot gopherbot added the Tools label Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.