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

Jump to bottom

x/tools/go/packages: type-checker uses incorrect type sizes function #28179

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

x/tools/go/packages: type-checker uses incorrect type sizes function #28179

alandonovan opened this issue Oct 12, 2018 · 3 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@alandonovan
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

matloob commented Dec 4, 2018

This was fixed by https://go-review.googlesource.com/c/tools/+/146757

@matloob matloob closed this as completed Dec 4, 2018
@golang golang locked and limited conversation to collaborators Dec 4, 2019
@rsc rsc unassigned matloob Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@jimmyfrasche @alandonovan @gopherbot @matloob