-
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: cannot convert v (variable of type *Bar[T]) to type *Foo[T] #52529
Comments
It does seem that the behavior changes based on the location of the line CC @griesemer |
Changing the // func (v *Bar[T]) M() {
// _ = (*Foo[T])(v)
// }
func _[T any](v *Bar[T]) {
_ = (*Foo[T])(v)
} Definitely a bug. Possibly some bad interaction with method type parameter instantiation/renaming. |
We tracked it down. The issue is that with this set-up, the call to under() in the sanity here is happening eagerly, when it should be lazy: This can be easily fixed by deferring this sanity check, but it does of course beg the question of how to make the type-checker less sensitive to accidental eager evaluation. For long-term maintainability, we must introduce invariants that allow us to detect or even prevent such bugs. |
@gopherbot, please open a backport issue for 1.18, this is a bug causing valid code not to compile. |
Backport issue(s) opened: #52558 (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. |
This turned out not to be entirely trivial, so I will fix it later this week. The root cause of the problem is that when we collect a methods, we check that method names are unique among existing methods and fields before adding each method. As a result, we have an invariant that recorded method names are always unique among methods and fields. But enforcing this invariant results in us expanding underlying types too soon, before they are fully set-up. |
Change https://go.dev/cl/403455 mentions this issue: |
Change https://go.dev/cl/403754 mentions this issue: |
…ing struct field names In #52529, we observed that checking types for duplicate fields and methods during method collection can result in incorrect early expansion of the base type. Fix this by delaying the check for duplicate fields. Notably, we can't delay the check for duplicate methods as we must preserve the invariant that added method names are unique. After this change, it may be possible in the presence of errors to have a type-checked type containing a method name that conflicts with a field name. With the previous logic conflicting methods would have been skipped. This is a change in behavior, but only for invalid code. Preserving the existing behavior would likely require delaying method collection, which could have more significant consequences. As a result of this change, the compiler test fixedbugs/issue28268.go started passing with types2, being previously marked as broken. The fix was not actually related to the duplicate method error, but rather the fact that we stopped reporting redundant errors on the calls to x.b() and x.E(), because they are now (valid!) methods. Updates #52529 Fixes #52558 Change-Id: I850ce85c6ba76d79544f46bfd3deb8538d8c7d00 Reviewed-on: https://go-review.googlesource.com/c/go/+/403455 Reviewed-by: Robert Griesemer <gri@google.com> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit b75e492) Reviewed-on: https://go-review.googlesource.com/c/go/+/403754
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?Irrelevant.
What did you do?
What did you expect to see?
The code compiles.
What did you see instead?
main.go:10:16: cannot convert v (variable of type *Bar[T]) to type *Foo[T]
The code does compile if the compiler sees type Bar first.
The text was updated successfully, but these errors were encountered: