From: Keith Randall Date: Wed, 2 Mar 2022 23:08:23 +0000 (-0800) Subject: [release-branch.go1.18] cmd/compile: don't include instantiating types in type hash X-Git-Tag: go1.18~29 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9f5e2849e12b10e3ca43ac85b00c40e506e83317;p=gostls13.git [release-branch.go1.18] cmd/compile: don't include instantiating types in type hash This CL is a bit overkill, but it is pretty safe for 1.18. We'll want to revisit for 1.19 so we can avoid the hash collisions between types, e.g. G[int] and G[float64], that will cause some slowdowns (but not incorrect behavior). Thanks Cherry for the simple idea. Fixes #51250 Change-Id: I68130e09ba68e7cc35687bc623f63547bc552867 Reviewed-on: https://go-review.googlesource.com/c/go/+/389474 Trust: Keith Randall Run-TryBot: Keith Randall Reviewed-by: Matthew Dempsky Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot (cherry picked from commit d3672054fb58d5eaa241f15fa9d7fb9d61e9ac05) Reviewed-on: https://go-review.googlesource.com/c/go/+/390017 Trust: Dmitri Shuralyov Run-TryBot: Dmitri Shuralyov Reviewed-by: Keith Randall --- diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go index 42ea7bac46..a4ddb1a312 100644 --- a/src/cmd/compile/internal/reflectdata/reflect.go +++ b/src/cmd/compile/internal/reflectdata/reflect.go @@ -1430,7 +1430,7 @@ func WriteBasicTypes() { type typeAndStr struct { t *types.Type - short string // "short" here means NameString + short string // "short" here means TypeSymName regular string } diff --git a/src/cmd/compile/internal/types/fmt.go b/src/cmd/compile/internal/types/fmt.go index 09814ac46d..c7d0623d40 100644 --- a/src/cmd/compile/internal/types/fmt.go +++ b/src/cmd/compile/internal/types/fmt.go @@ -72,6 +72,7 @@ const ( fmtDebug fmtTypeID fmtTypeIDName + fmtTypeIDHash ) // Sym @@ -144,10 +145,21 @@ func symfmt(b *bytes.Buffer, s *Sym, verb rune, mode fmtMode) { if q := pkgqual(s.Pkg, verb, mode); q != "" { b.WriteString(q) b.WriteByte('.') - if mode == fmtTypeIDName { + switch mode { + case fmtTypeIDName: // If name is a generic instantiation, it might have local package placeholders // in it. Replace those placeholders with the package name. See issue 49547. name = strings.Replace(name, LocalPkg.Prefix, q, -1) + case fmtTypeIDHash: + // If name is a generic instantiation, don't hash the instantiating types. + // This isn't great, but it is safe. If we hash the instantiating types, then + // we need to make sure they have just the package name. At this point, they + // either have "", or the whole package path, and it is hard to reconcile + // the two without depending on -p (which we might do someday). + // See issue 51250. + if i := strings.Index(name, "["); i >= 0 { + name = name[:i] + } } } b.WriteString(name) @@ -173,7 +185,7 @@ func pkgqual(pkg *Pkg, verb rune, mode fmtMode) string { case fmtDebug: return pkg.Name - case fmtTypeIDName: + case fmtTypeIDName, fmtTypeIDHash: // dcommontype, typehash return pkg.Name @@ -331,7 +343,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type if t == AnyType || t == ByteType || t == RuneType { // in %-T mode collapse predeclared aliases with their originals. switch mode { - case fmtTypeIDName, fmtTypeID: + case fmtTypeIDName, fmtTypeIDHash, fmtTypeID: t = Types[t.Kind()] default: sconv2(b, t.Sym(), 'S', mode) @@ -422,7 +434,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type case TPTR: b.WriteByte('*') switch mode { - case fmtTypeID, fmtTypeIDName: + case fmtTypeID, fmtTypeIDName, fmtTypeIDHash: if verb == 'S' { tconv2(b, t.Elem(), 'S', mode, visited) return @@ -484,7 +496,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type case IsExported(f.Sym.Name): sconv2(b, f.Sym, 'S', mode) default: - if mode != fmtTypeIDName { + if mode != fmtTypeIDName && mode != fmtTypeIDHash { mode = fmtTypeID } sconv2(b, f.Sym, 'v', mode) @@ -554,7 +566,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type b.WriteByte(byte(open)) fieldVerb := 'v' switch mode { - case fmtTypeID, fmtTypeIDName, fmtGo: + case fmtTypeID, fmtTypeIDName, fmtTypeIDHash, fmtGo: // no argument names on function signature, and no "noescape"/"nosplit" tags fieldVerb = 'S' } @@ -688,7 +700,7 @@ func fldconv(b *bytes.Buffer, f *Field, verb rune, mode fmtMode, visited map[*Ty if name == ".F" { name = "F" // Hack for toolstash -cmp. } - if !IsExported(name) && mode != fmtTypeIDName { + if !IsExported(name) && mode != fmtTypeIDName && mode != fmtTypeIDHash { name = sconv(s, 0, mode) // qualify non-exported names (used on structs, not on funarg) } } else { @@ -756,7 +768,7 @@ func FmtConst(v constant.Value, sharp bool) string { // TypeHash computes a hash value for type t to use in type switch statements. func TypeHash(t *Type) uint32 { - p := t.NameString() + p := tconv(t, 0, fmtTypeIDHash) // Using MD5 is overkill, but reduces accidental collisions. h := md5.Sum([]byte(p)) diff --git a/test/typeparam/issue51250a.dir/a.go b/test/typeparam/issue51250a.dir/a.go new file mode 100644 index 0000000000..12dd60a3d1 --- /dev/null +++ b/test/typeparam/issue51250a.dir/a.go @@ -0,0 +1,9 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +type G[T any] struct { + x T +} diff --git a/test/typeparam/issue51250a.dir/b.go b/test/typeparam/issue51250a.dir/b.go new file mode 100644 index 0000000000..114c9f80f7 --- /dev/null +++ b/test/typeparam/issue51250a.dir/b.go @@ -0,0 +1,24 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package b + +import "./a" + +type T struct { a int } + +var I interface{} = a.G[T]{} + +//go:noinline +func F(x interface{}) { + switch x.(type) { + case a.G[T]: + case int: + panic("bad") + case float64: + panic("bad") + default: + panic("bad") + } +} diff --git a/test/typeparam/issue51250a.dir/main.go b/test/typeparam/issue51250a.dir/main.go new file mode 100644 index 0000000000..45288be482 --- /dev/null +++ b/test/typeparam/issue51250a.dir/main.go @@ -0,0 +1,24 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "./a" + "./b" +) + +func main() { + switch b.I.(type) { + case a.G[b.T]: + case int: + panic("bad") + case float64: + panic("bad") + default: + panic("bad") + } + + b.F(a.G[b.T]{}) +} diff --git a/test/typeparam/issue51250a.go b/test/typeparam/issue51250a.go new file mode 100644 index 0000000000..aefbe67310 --- /dev/null +++ b/test/typeparam/issue51250a.go @@ -0,0 +1,7 @@ +// rundir + +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ignored