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: type-checker uses incorrect type sizes function #28179

Closed
alandonovan opened this issue Oct 12, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@alandonovan
Copy link
Contributor

commented Oct 12, 2018

The go/types.Config struct has a Sizes field that specifies the function that the type checker should use to compute the size of a type. This function is itself a function of the CPU architecture and potentially the OS or toolchain as well. Only the underlying build system (go list, blaze, bazel) knows authoritatively what size to use, so go/packages should obtain this information from the build system rather than guessing based on GOARCH as it does now. One way to do this is to ask the underlying build system to compile a tiny program and inspect the compiler output.

@gopherbot gopherbot added this to the Unreleased milestone Oct 12, 2018

@jimmyfrasche

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

When using the go command, go env -json provides GOOS/GOARCH but not the compiler.

go list -e -f {{context}} provides all three (semi-related #27915)

But you can only use that info reliably when compiler = gc since go/types only has size info for gc (though presumably they're the same for gccgo and it's approximately safe to assume no other compilers exist).

In general, compiling a test program sounds necessary but it might be faster to skip that step when using the go command and the go command is using the gc compiler?

@alandonovan

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2018

Yes, it's fine to skip the compilation step if you can compute the result accurately some other way. The necessary logic will be driver-specific, and each writer should know what range of OS/ARCH pairs are feasible.

@matloob

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2018

@matloob matloob closed this Dec 4, 2018

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