go/types/typeutil: break recursion through anonymous interfaces

In a type such as
    type X interface { m() []*interface { X } }
the traversal never encounters the named type X in the result of m
since Interface.Methods expands it to a set of methods, one that
includes m, causing the traversal to get stuck.

This change uses an alternative, shallow hash function on the types
of interface methods to avoid the possibility of getting stuck in
such a cycle.

(An earlier draft used a stack of interface types to detect cycles,
but the logic of caching made this approach quite tricky.)

Fixes golang/go#56048
Fixes golang/go#26863

Change-Id: I28a604e6affae5dfdd05a62e405d49a3efc8d709
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439117
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>

Author: adonovan

go/types/typeutil/map.go

@@ -332,7 +332,9 @@ func (h Hasher) hashFor(t types.Type) uint32 {
 			// Method order is not significant.
 			// Ignore m.Pkg().
 			m := t.Method(i)
-			hash += 3*hashString(m.Name()) + 5*h.Hash(m.Type())
+			// Use shallow hash on method signature to
+			// avoid anonymous interface cycles.
+			hash += 3*hashString(m.Name()) + 5*h.shallowHash(m.Type())
 		}
 
 		// Hash type restrictions.
@@ -434,3 +436,76 @@ func (h Hasher) hashPtr(ptr interface{}) uint32 {
 	h.ptrMap[ptr] = hash
 	return hash
 }
+
+// shallowHash computes a hash of t without looking at any of its
+// element Types, to avoid potential anonymous cycles in the types of
+// interface methods.
+//
+// When an unnamed non-empty interface type appears anywhere among the
+// arguments or results of an interface method, there is a potential
+// for endless recursion. Consider:
+//
+//	type X interface { m() []*interface { X } }
+//
+// The problem is that the Methods of the interface in m's result type
+// include m itself; there is no mention of the named type X that
+// might help us break the cycle.
+// (See comment in go/types.identical, case *Interface, for more.)
+func (h Hasher) shallowHash(t types.Type) uint32 {
+	// t is the type of an interface method (Signature),
+	// its params or results (Tuples), or their immediate
+	// elements (mostly Slice, Pointer, Basic, Named),
+	// so there's no need to optimize anything else.
+	switch t := t.(type) {
+	case *types.Signature:
+		var hash uint32 = 604171
+		if t.Variadic() {
+			hash *= 971767
+		}
+		// The Signature/Tuple recursion is always finite
+		// and invariably shallow.
+		return hash + 1062599*h.shallowHash(t.Params()) + 1282529*h.shallowHash(t.Results())
+
+	case *types.Tuple:
+		n := t.Len()
+		hash := 9137 + 2*uint32(n)
+		for i := 0; i < n; i++ {
+			hash += 53471161 * h.shallowHash(t.At(i).Type())
+		}
+		return hash
+
+	case *types.Basic:
+		return 45212177 * uint32(t.Kind())
+
+	case *types.Array:
+		return 1524181 + 2*uint32(t.Len())
+
+	case *types.Slice:
+		return 2690201
+
+	case *types.Struct:
+		return 3326489
+
+	case *types.Pointer:
+		return 4393139
+
+	case *typeparams.Union:
+		return 562448657
+
+	case *types.Interface:
+		return 2124679 // no recursion here
+
+	case *types.Map:
+		return 9109
+
+	case *types.Chan:
+		return 9127
+
+	case *types.Named:
+		return h.hashPtr(t.Obj())
+
+	case *typeparams.TypeParam:
+		return h.hashPtr(t.Obj())
+	}
+	panic(fmt.Sprintf("shallowHash: %T: %v", t, t))
+}

go/types/typeutil/map_test.go

@@ -244,6 +244,14 @@ 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() }]() {}
+
+
+type Issue56048_I interface{ m() interface { Issue56048_I } }
+var Issue56048 = Issue56048_I.m
+
+type Issue56048_Ib interface{ m() chan []*interface { Issue56048_Ib } }
+var Issue56048b = Issue56048_Ib.m
+
 `
 
 	fset := token.NewFileSet()
@@ -296,12 +304,14 @@ func Quux[Q interface{ quux() }]() {}
 		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()
+		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()
+		Issue56048  = scope.Lookup("Issue56048").Type()
+		Issue56048b = scope.Lookup("Issue56048b").Type()
 	)
 
 	tmap := new(typeutil.Map)
@@ -371,6 +381,9 @@ func Quux[Q interface{ quux() }]() {}
 		{Bar, "Bar", false},
 		{Baz, "Baz", false},
 		{Quux, "Quux", true},
+
+		{Issue56048, "Issue56048", true},   // (not actually about generics)
+		{Issue56048b, "Issue56048b", true}, // (not actually about generics)
 	}
 
 	for _, step := range steps {