-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: 1.18 produces different type descriptor for identical struct in different packages #52856
Comments
A possibly minified repro (with |
Although both values have type |
@gopherbot Please open a backport for 1.18. This should be fixed in a 1.18 minor release. The bug does not exist in earlier releases. |
Backport issue(s) opened: #52858 (for 1.18). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
The spec says https://go.dev/ref/spec#Type_identity
Here the field name ( The embedding makes it an interesting case. I guess one could argue that the field name for both types is from the |
Thanks. You're right: with 1.18 the I guess this may be technically correct. Anybody want to argue otherwise? |
I agree the 1.18 behavior is correct. |
FWIW, I suspect https://go-review.googlesource.com/c/go/+/378434/ is the CL that introduced the changed (fixed) behavior here. |
I'd also say the 1.18 behavior is correct. The name of an embedded field is simply the (unqualified) name of the embedded type, there's no knowledge of that name coming from a built-in (predeclared) type. The usual export rules apply. The behavior for |
Working as intended. |
To finally clarify the question If you replace the example with one like this, it is true everywhere.
And if you replace the example with one like this, it is false everywhere.
Is this behavior correct?
|
@pfi79 The code at https://go.dev/play/p/aAwW6Hhyous appears to behaving correctly, yes. Edit: To elaborate, defined types declared by different type declarations are always different, and reflect.DeepEqual documents that values of different types are always unequal. |
I have to say it's a little bit unexpected, while I understand why it should be true speaking of unexported fields, IMO with embedding, it is not that obvious, and breaking backwards compatibility. |
@C0rWin I'm sorry for surprise. |
Change https://go.dev/cl/414235 mentions this issue: |
For #52856 Change-Id: Iab3e8352f64d774058391f0422cd01c53c3e711d Reviewed-on: https://go-review.googlesource.com/c/go/+/414235 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org>
Change https://go.dev/cl/414294 mentions this issue: |
The test case is https://go.dev/cl/414235. Fixes golang/go#52856 Change-Id: I1e85cd56b0603c0480d5694ba2ceb058d0cb5cf9 Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/414294 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
The test case is https://go.dev/cl/414235. Fixes golang/go#52856 Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/414294
The test case is https://go.dev/cl/414235. Fixes golang/go#52856 Change-Id: I1e85cd56b0603c0480d5694ba2ceb058d0cb5cf9 Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/414294 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
For golang#52856 Change-Id: Iab3e8352f64d774058391f0422cd01c53c3e711d Reviewed-on: https://go-review.googlesource.com/c/go/+/414235 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
go 1.18
go 1.17
What did you expect to see?
identical behaviour
What did you see instead?
different behavior
If the change in behavior is normal, please show the comit in which it has changed
The text was updated successfully, but these errors were encountered: