apidiff: support instantiated generic types

Use types.Types as keys in the map of correspondences, rather than
*types.TypeNames.  There was no reason not do this before, except
convenience; prior to generics, named types were the same iff their
TypeNames were.

Also, update the definition of correspondence to handle instantiated
generic types, by checking that their TypeArg lists and origins
correspond.

Also, we now don't need the hack that was typesEquivalent.
We can always compare two types from the same world with
types.Identical.

Change-Id: Ic5c7bb0be052376367c84da8680ca86b4a1dafc4
Reviewed-on: https://go-review.googlesource.com/c/exp/+/513136
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>

apidiff/apidiff.go

@@ -16,6 +16,8 @@ import (
 	"go/token"
 	"go/types"
 	"strings"
+
+	"golang.org/x/tools/go/types/typeutil"
 )
 
 // Changes reports on the differences between the APIs of the old and new packages.
@@ -108,7 +110,7 @@ type differ struct {
 	// Even though it is the named types (*types.Named) that correspond, we use
 	// *types.TypeName as a map key because they are canonical.
 	// The values can be either named types or basic types.
-	correspondMap map[*types.TypeName]types.Type
+	correspondMap typeutil.Map
 
 	// Messages.
 	incompatibles messageSet
@@ -119,7 +121,6 @@ func newDiffer(old, new *types.Package) *differ {
 	return &differ{
 		old:           old,
 		new:           new,
-		correspondMap: map[*types.TypeName]types.Type{},
 		incompatibles: messageSet{},
 		compatibles:   messageSet{},
 	}
@@ -161,28 +162,36 @@ func (d *differ) checkPackage(oldRootPackagePath string) {
 
 	// Whole-package satisfaction.
 	// For every old exposed interface oIface and its corresponding new interface nIface...
-	for otn1, nt1 := range d.correspondMap {
+	d.correspondMap.Iterate(func(k1 types.Type, v1 any) {
+		ot1 := k1.(*types.Named)
+		otn1 := ot1.Obj()
+		nt1 := v1.(types.Type)
 		oIface, ok := otn1.Type().Underlying().(*types.Interface)
 		if !ok {
-			continue
+			return
 		}
 		nIface, ok := nt1.Underlying().(*types.Interface)
 		if !ok {
 			// If nt1 isn't an interface but otn1 is, then that's an incompatibility that
 			// we've already noticed, so there's no need to do anything here.
-			continue
+			return
 		}
 		// For every old type that implements oIface, its corresponding new type must implement
 		// nIface.
-		for otn2, nt2 := range d.correspondMap {
+		d.correspondMap.Iterate(func(k2 types.Type, v2 any) {
+			ot2 := k2.(*types.Named)
+			otn2 := ot2.Obj()
+			nt2 := v2.(types.Type)
 			if otn1 == otn2 {
-				continue
+				return
 			}
 			if types.Implements(otn2.Type(), oIface) && !types.Implements(nt2, nIface) {
+				// TODO(jba): the type name is not sufficient information here; we need the type args
+				// if this is an instantiated generic type.
 				d.incompatible(objectWithSide{otn2, false}, "", "no longer implements %s", objectString(otn1, oldRootPackagePath))
 			}
-		}
-	}
+		})
+	})
 }
 
 func (d *differ) checkObjects(old, new types.Object) {

apidiff/correspondence.go

@@ -124,7 +124,6 @@ func (d *differ) corr(old, new types.Type, p *ifacePair) bool {
 		}
 		if new, ok := new.(*types.Basic); ok {
 			// Basic types are defined types, too, so we have to support them.
-
 			return d.establishCorrespondence(old, new)
 		}
 
@@ -167,8 +166,8 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool
 	oldname := old.Obj()
 	// If there already is a corresponding new type for old, check that they
 	// are the same.
-	if c := d.correspondMap[oldname]; c != nil {
-		return typesEquivalent(c, new)
+	if c := d.correspondMap.At(old); c != nil {
+		return types.Identical(c.(types.Type), new)
 	}
 	// Attempt to establish a correspondence.
 	// Assume the types don't correspond unless they have the same
@@ -196,69 +195,58 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool
 		if old.Obj().Pkg() != d.old || newn.Obj().Pkg() != d.new {
 			return old.Obj().Id() == newn.Obj().Id()
 		}
-		// Prior to generics, any two named types could correspond.
-		// Two named types cannot correspond if their type parameter lists don't match.
-		if !d.typeParamListsCorrespond(old.TypeParams(), newn.TypeParams()) {
-			return false
+		// Two generic named types correspond if their type parameter lists correspond.
+		// Since one or the other of those lists will be empty, it doesn't hurt
+		// to check both.
+		oldOrigin := old.Origin()
+		newOrigin := newn.Origin()
+		if oldOrigin != old {
+			// old is an instantiated type.
+			if newOrigin == newn {
+				// new is not; they cannot correspond.
+				return false
+			}
+			// Two instantiated types correspond if their origins correspond and
+			// their type argument lists correspond.
+			if !d.correspond(oldOrigin, newOrigin) {
+				return false
+			}
+			if !d.typeListsCorrespond(old.TypeArgs(), newn.TypeArgs()) {
+				return false
+			}
+		} else {
+			if !d.typeParamListsCorrespond(old.TypeParams(), newn.TypeParams()) {
+				return false
+			}
 		}
 	}
 	// If there is no correspondence, create one.
-	d.correspondMap[oldname] = new
+	d.correspondMap.Set(old, new)
 	// Check that the corresponding types are compatible.
 	d.checkCompatibleDefined(oldname, old, new)
 	return true
 }
 
-// Two list of type parameters correspond if they are the same length, and
-// the constraints of corresponding type parameters correspond.
-func (d *differ) typeParamListsCorrespond(tps1, tps2 *types.TypeParamList) bool {
-	if tps1.Len() != tps2.Len() {
+func (d *differ) typeListsCorrespond(tl1, tl2 *types.TypeList) bool {
+	if tl1.Len() != tl2.Len() {
 		return false
 	}
-	for i := 0; i < tps1.Len(); i++ {
-		if !d.correspond(tps1.At(i).Constraint(), tps2.At(i).Constraint()) {
+	for i := 0; i < tl1.Len(); i++ {
+		if !d.correspond(tl1.At(i), tl2.At(i)) {
 			return false
 		}
 	}
 	return true
 }
 
-// typesEquivalent reports whether two types are identical, or if
-// the types have identical type param lists except that one type has nil
-// constraints.
-//
-// This allows us to match a Type from a method receiver or arg to the Type from
-// the declaration.
-func typesEquivalent(t1, t2 types.Type) bool {
-	if types.Identical(t1, t2) {
-		return true
-	}
-	// Handle two types with the same type params, one
-	// having constraints and one not.
-	oldn, ok := t1.(*types.Named)
-	if !ok {
-		return false
-	}
-	newn, ok := t2.(*types.Named)
-	if !ok {
-		return false
-	}
-	oldps := oldn.TypeParams()
-	newps := newn.TypeParams()
-	if oldps.Len() != newps.Len() {
-		return false
-	}
-	if oldps.Len() == 0 {
-		// Not generic types.
+// Two list of type parameters correspond if they are the same length, and
+// the constraints of corresponding type parameters correspond.
+func (d *differ) typeParamListsCorrespond(tps1, tps2 *types.TypeParamList) bool {
+	if tps1.Len() != tps2.Len() {
 		return false
 	}
-	for i := 0; i < oldps.Len(); i++ {
-		oldp := oldps.At(i)
-		newp := newps.At(i)
-		if oldp.Constraint() == nil || newp.Constraint() == nil {
-			return true
-		}
-		if !types.Identical(oldp.Constraint(), newp.Constraint()) {
+	for i := 0; i < tps1.Len(); i++ {
+		if !d.correspond(tps1.At(i).Constraint(), tps2.At(i).Constraint()) {
 			return false
 		}
 	}

apidiff/testdata/generics.go

@@ -48,3 +48,37 @@ type custom interface {
 }
 
 type GT3[E custom] map[E]int
+
+// Instantiated types:
+// Two instantiations of generic types
+// with different type parameters are different.
+
+// both
+type H[T any] []T
+
+// old
+var V1 H[int]
+
+type T int
+
+var V2 H[T]
+
+// new
+// i V1: changed from H[int] to H[bool]
+var V1 H[bool]
+
+// i T: changed from int to bool
+type T bool
+
+// OK: we reported on T, so we don't need to here.
+var V2 H[T]
+
+// old
+type t int
+
+// new
+// i t: changed from int to byte
+type t byte
+
+// both
+var V3 H[t]