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: generic code seems to produce duplicate type descriptor #53087
Comments
Thanks for the bug report. Here is a slightly trimmed down test case. For me this prints
Perhaps the type descriptor got duplicated. package main
import "fmt"
type I interface {
M()
}
type S struct {
str string
}
func (s *S) M() {}
var _ I = &S{}
type CloningMap[K comparable, V any] struct {
inner map[K]V
}
func (cm CloningMap[K, V]) With(key K, value V) CloningMap[K, V] {
result := CloneBad(cm.inner)
result[key] = value
return CloningMap[K, V]{result}
}
func CloneBad[M ~map[K]V, K comparable, V any](m M) M {
r := make(M, len(m))
for k, v := range m {
r[k] = v
}
return r
}
func main() {
s1 := &S{"one"}
s2 := &S{"two"}
m := CloningMap[string, I]{inner: make(map[string]I)}
m = m.With("a", s1)
m = m.With("b", s2)
it, found := m.inner["a"]
if !found {
panic("a not found")
}
if _, ok := it.(*S); !ok {
panic(fmt.Sprintf("got %T want *main.S", it))
}
} |
The type descriptor is not duplicated, but the itab is. |
There is one |
The conversion |
I've had a general unease about instantiating a function like
with K==string, V==int as
instead of
Or something like that. Having That said, I'm not super sure that's the cause of this bug. Just a hypothesis. |
Change https://go.dev/cl/409356 mentions this issue: |
Checking on on this as it's currently blocking beta 1. Do you expect it'll be resolved by next week? |
Honestly I don't know. |
Change https://go.dev/cl/410197 mentions this issue: |
I’ve managed to hit this without any |
Wondering if there's been any movement on this? |
It looks like both @Porges and @ianlancetaylor's repros are passing on the Go dev branch of the golang playground: https://go.dev/play/p/gxm5cDGYxKv?v=gotip Is it possible that this has been fixed in master? I don't see any issue/PR mentioning this, so unsure if it was really fixed or if something has changed so the repro doesn't work. Possibly fixed by this? 38edd9b |
This has been fixed at tip. I don't think we'll be backporting, as it would require changes to the non-unified generics implementation. |
This issue has been fixed with unified IR, so just add a test. Update #53087 Change-Id: I965d9f27529fa6b7c89e2921c65e5a100daeb9fe Reviewed-on: https://go-review.googlesource.com/c/go/+/410197 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Auto-Submit: Keith Randall <khr@google.com>
What version of Go are you using (
go version
)?Tested on 1.18 and 1.18.2.
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
After using
maps.Clone
, values retrieved from the (copied) map fail to typecast correctly.I’m still working on a minimal repro, but here is what I’m seeing.See small repro below.
The original code in question looks like this:
The expectation is that this prints
true
four times as the values retrieved from the map are the same as those put into it. However, it does not; the first value retrieved from the map fails to cast correctly.Output:
The implementation of
With
looks like this:I narrowed down the problem to
maps.Clone
. If I copy it as a local function then this continues to fail:However, if the
M
parameter is replaced withmap[K]V
directly, then it works as expected:Note that this also happens if the same value is input as both "a" and "b"; whichever value was in the map before it was
maps.Clone
d is the one that fails to cast correctly.Repro
Switch between
CloneBad
(a copy ofmaps.Clone
) andCloneGood
to see the behaviour change.The text was updated successfully, but these errors were encountered: