-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: _ (blank) struct fields lose package info #15514
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
Comments
This is a bug that's been around for a long time - no need to fix for 1.7. Should address this in new export format only after the textual export format is gone to avoid "mixed-mode" errors. Leaving for 1.8. |
I cannot reproduce this anymore with Go1.6 (I don't think it was present in 1.6, the error case was probably flawed due to a buggy exporter/importer at that time). This bug also doesn't exist in Go1.7 - at least given the above testcase. In fact, it happens to have been fixed inadvertently by https://go-review.googlesource.com/#/c/22714/6 ; specifically: https://go-review.googlesource.com/#/c/22714/6/src/cmd/compile/internal/gc/bimport.go, line 521: The exporter doesn't export a package for the (unexported) _ field. The importer uses the builtin package for the _ field. Since any local struct type using a _ field will use the local package for _ field, an anonymous struct with a _ field and a structurally equal imported struct with _ field will have different packages for that field, leading to the expected error. But the "optimization" of using the builtin package for imported _ fields is still incorrect because it's possible to export unnamed structs with _ fields which should also not be assignment compatible. Here's a test case:
|
Here's another test case:
go/types reports an error in this case when type-checking c.go:
which is correct. go/types can report an error because its importer assigns the package from which the struct is imported to the struct's _ field. (That's not correct in general either, because a struct may have been imported transitively and then gets the last package rather than the original one. But fixing this bug for cmd/compile in the export data will also fix this case for go/types). |
Towards a fix for #15514. Change-Id: I62073e9fdcfe5ddda9b0a47fe8554b524191a77c Reviewed-on: https://go-review.googlesource.com/27638 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Code from https://go-review.googlesource.com/#/c/27639/ . Remain backward-compatible with possibly installed packages that remain in Go1.6 format. Change-Id: If424e7a881c81bbfcacf38c0946542793c406abd Reviewed-on: https://go-review.googlesource.com/27640 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- Original commit message Code from https://go-review.googlesource.com/#/c/27639/ . Remain backward-compatible with possibly installed packages that remain in Go1.6 format. - backport of golang/tools@b2560d1 Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
CL https://golang.org/cl/27912 mentions this issue. |
The Go1.7 export format didn't encode the field package for blank struct fields (#15514). Re-introduce support for that format so we can read it w/o error. For #16881. Change-Id: Ib131d41aac56dbf970aab15ae7e75ef3944b412d Reviewed-on: https://go-review.googlesource.com/27912 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a bug with the old (and currently also the new) export format. Example by mdempsky:
Compiling b succeeds even though the (anonymous) struct in b has a _ (blank) field from a different package (b) then the struct x (package a). But per the spec, such an assignment is invalid because the blank fields are different (i.e., not unique): they are not exported yet from different packages.
We should probably just fix this with the new export format.
The text was updated successfully, but these errors were encountered: