Skip to content

Commit

Permalink
go/types/typeutil: don't recurse into constraints when hashing tparams
Browse files Browse the repository at this point in the history
When hashing signature type parameters, we recursed into their
constraints in an attempt to improve the accuracy of the hash
(signatures are considered identical modulo type parameter renaming).

This introduces a mechanism for infinite recursion via recursive
constraints, as reported in golang/go#51314. To fix this, we can hash
just the type parameter index and rely on types.Identical for
de-duplicating map entries.

Fixes golang/go#51314

Change-Id: Ifd7596d978b98293391bbe64f72f1da2b2a5d51c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387754
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
findleyr committed Feb 24, 2022
1 parent 258e473 commit e6ef770
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
10 changes: 6 additions & 4 deletions go/types/typeutil/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func (h Hasher) hashFor(t types.Type) uint32 {
func (h Hasher) hashTuple(tuple *types.Tuple) uint32 {
// See go/types.identicalTypes for rationale.
n := tuple.Len()
var hash uint32 = 9137 + 2*uint32(n)
hash := 9137 + 2*uint32(n)
for i := 0; i < n; i++ {
hash += 3 * h.Hash(tuple.At(i).Type())
}
Expand All @@ -398,7 +398,7 @@ func (h Hasher) hashUnion(t *typeparams.Union) uint32 {
}

func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 {
var hash uint32 = 9157 + 2*uint32(len(terms))
hash := 9157 + 2*uint32(len(terms))
for _, term := range terms {
// term order is not significant.
termHash := h.Hash(term.Type())
Expand All @@ -416,14 +416,16 @@ func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 {
// If h.sigTParams is set and contains t, then we are in the process of hashing
// a signature, and the hash value of t must depend only on t's index and
// constraint: signatures are considered identical modulo type parameter
// renaming.
// renaming. To avoid infinite recursion, we only hash the type parameter
// index, and rely on types.Identical to handle signatures where constraints
// are not identical.
//
// Otherwise the hash of t depends only on t's pointer identity.
func (h Hasher) hashTypeParam(t *typeparams.TypeParam) uint32 {
if h.sigTParams != nil {
i := t.Index()
if i >= 0 && i < h.sigTParams.Len() && t == h.sigTParams.At(i) {
return 9173 + 2*h.Hash(t.Constraint()) + 3*uint32(i)
return 9173 + 3*uint32(i)
}
}
return h.hashPtr(t.Obj())
Expand Down
26 changes: 26 additions & 0 deletions go/types/typeutil/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,17 @@ var ME2 = G2[int].M
// ME1Type should have identical type as ME1.
var ME1Type func(G1[int], G1[int], G2[int])
// Examples from issue #51314
type Constraint[T any] interface{}
func Foo[T Constraint[T]]() {}
func Fn[T1 ~*T2, T2 ~*T1](t1 T1, t2 T2) {}
// Bar and Baz are identical to Foo.
func Bar[P Constraint[P]]() {}
func Baz[Q any]() {} // The underlying type of Constraint[P] is any.
// But Quux is not.
func Quux[Q interface{ quux() }]() {}
`

fset := token.NewFileSet()
Expand Down Expand Up @@ -284,6 +295,13 @@ var ME1Type func(G1[int], G1[int], G2[int])
ME1 = scope.Lookup("ME1").Type()
ME1Type = scope.Lookup("ME1Type").Type()
ME2 = scope.Lookup("ME2").Type()

Constraint = scope.Lookup("Constraint").Type()
Foo = scope.Lookup("Foo").Type()
Fn = scope.Lookup("Fn").Type()
Bar = scope.Lookup("Foo").Type()
Baz = scope.Lookup("Foo").Type()
Quux = scope.Lookup("Quux").Type()
)

tmap := new(typeutil.Map)
Expand Down Expand Up @@ -345,6 +363,14 @@ var ME1Type func(G1[int], G1[int], G2[int])
{ME1, "ME1", true},
{ME1Type, "ME1Type", false},
{ME2, "ME2", true},

// See golang/go#51314: avoid infinite recursion on cyclic type constraints.
{Constraint, "Constraint", true},
{Foo, "Foo", true},
{Fn, "Fn", true},
{Bar, "Bar", false},
{Baz, "Baz", false},
{Quux, "Quux", true},
}

for _, step := range steps {
Expand Down

0 comments on commit e6ef770

Please sign in to comment.