go/types: generalize/cleanup missingMethod to handle generic methods

This simplifies the use of missingMethod again but moving the lazy
instantiation of methods into missingMethod. Method comparison now
also takes method type paramaters into account.

This change enables the first examples of parameterized methods
to type-check.

Change-Id: I4c629fb7b1f8959184c6ce5196365893d11b197d

src/go/types/NOTES

@@ -18,6 +18,7 @@ OPEN ISSUES
 - leaving away unused receiver type parameters leads to an error; e.g.: "type S(type T) struct{}; func (S) _()"
 - allow parenthesized embedded interfaces so we can embed parameterized interfaces and they won't be confused
   for methods
+- confirm that it's ok to use inference in missingMethod to compare parameterized methods
 
 DESIGN/IMPLEMENTATION
 - 11/19/2019: For type parameters with interface bounds to work, the scope of all type parameters in

src/go/types/lookup.go

@@ -265,7 +265,7 @@ func (check *Checker) lookupType(m map[Type]int, typ Type) (int, bool) {
 // x is of interface type V).
 //
 func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType bool) {
-	m, typ := (*Checker)(nil).missingMethod(V, T, static, nil)
+	m, typ := (*Checker)(nil).missingMethod(V, T, static)
 	return m, typ != nil
 }
 
@@ -273,15 +273,11 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b
 // The receiver may be nil if missingMethod is invoked through
 // an exported API call (such as MissingMethod), i.e., when all
 // methods have been type-checked.
-// If a non-nil update function is provided, it is used to update
-// the method types of V before comparing them with the methods
-// of V (usually be renaming type parameters so they can be
-// compared).
 // If the type has the correctly named method, but with the wrong
 // signature, the existing method is returned as well.
 // To improve error messages, also report the wrong signature
 // when the method exists on *V instead of V.
-func (check *Checker) missingMethod(V Type, T *Interface, static bool, update func(V Type, sig *Signature) *Signature) (method, wrongType *Func) {
+func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method, wrongType *Func) {
 	check.completeInterface(token.NoPos, T)
 
 	// fast path for common case
@@ -302,7 +298,24 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, update fu
 				continue
 			}
 
-			if !check.identical(f.typ, m.typ) {
+			// both methods must have the same number of type parameters
+			ftyp := f.typ.(*Signature)
+			mtyp := m.typ.(*Signature)
+			if len(ftyp.tparams) != len(mtyp.tparams) {
+				return m, f
+			}
+
+			// If the methods have type parameters we don't care whether they
+			// are the same or not, as long as they match up. Use inference
+			// comparison in that case.
+			// TODO(gri) is this always correct? what about type bounds?
+			// (Alternative is to rename/subst type parameters and compare.)
+			var tparams []Type
+			if len(mtyp.tparams) > 0 {
+				tparams = make([]Type, len(mtyp.tparams))
+			}
+
+			if !check.identical0(ftyp, mtyp, true, nil, tparams) {
 				return m, f
 			}
 		}
@@ -311,6 +324,8 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, update fu
 	}
 
 	// A concrete type implements T if it implements all methods of T.
+	Vd, _ := deref(V)
+	Vn, _ := Vd.(*Named)
 	for _, m := range T.allMethods {
 		obj, _, _ := check.rawLookupFieldOrMethod(V, false, m.pkg, m.name)
 
@@ -334,13 +349,35 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, update fu
 			check.objDecl(f, nil)
 		}
 
-		// update (generic) method signatures before comparing them
+		// both methods must have the same number of type parameters
 		ftyp := f.typ.(*Signature)
-		if update != nil {
-			ftyp = update(V, ftyp)
+		mtyp := m.typ.(*Signature)
+		if len(ftyp.tparams) != len(mtyp.tparams) {
+			return m, f
 		}
 
-		if !check.identical(ftyp, m.typ) {
+		// If V is a (instantiated) generic type, its methods are still
+		// parameterized using the original (declaration) receiver type
+		// parameters (subst simply copies the existing method list, it
+		// does not instantiate the methods).
+		// In order to compare the signatures, substitute the receiver
+		// type parameters of ftyp with V's instantiation type arguments.
+		// This lazily instantiates the method f.
+		if Vn != nil {
+			ftyp = check.subst(token.NoPos, ftyp, ftyp.rparams, Vn.targs).(*Signature)
+		}
+
+		// If the methods have type parameters we don't care whether they
+		// are the same or not, as long as they match up. Use inference
+		// comparison in that case.
+		// TODO(gri) is this always correct? what about type bounds?
+		// (Alternative is to rename/subst type parameters and compare.)
+		var tparams []Type
+		if len(mtyp.tparams) > 0 {
+			tparams = make([]Type, len(mtyp.tparams))
+		}
+
+		if !check.identical0(ftyp, mtyp, true, nil, tparams) {
 			return m, f
 		}
 	}
@@ -360,7 +397,7 @@ func (check *Checker) assertableTo(V *Interface, T Type) (method, wrongType *Fun
 	if _, ok := T.Underlying().(*Interface); ok && !strict {
 		return
 	}
-	return check.missingMethod(T, V, false, nil)
+	return check.missingMethod(T, V, false)
 }
 
 // deref dereferences typ if it is a *Pointer and returns its base and true.

src/go/types/operand.go

@@ -267,27 +267,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) bool {
 
 	// T is an interface type and x implements T
 	if Ti, ok := Tu.(*Interface); ok {
-		// update targ method signatures
-		// TODO(gri) This needs documentation and clean up!
-		update := func(V Type, sig *Signature) *Signature {
-			V, _ = deref(V)
-			pos := token.NoPos // TODO(gri) can we do better?
-			// check.dump(">>> %s: V = %s", pos, V)
-			var targs []Type
-			if vnamed, _ := V.(*Named); vnamed != nil {
-				targs = vnamed.targs
-			} else {
-				// nothing to do
-				return sig
-			}
-			// check.dump(">>> %s: targs = %s", pos, targs)
-			// check.dump(">>> %s: sig.rparams = %s", pos, sig.rparams)
-			// check.dump(">>> %s: sig before: %s, tparams = %s, targs = %s", pos, sig, sig.rparams, targs)
-			sig = check.subst(pos, sig, sig.rparams, targs).(*Signature)
-			// check.dump(">>> %s: sig after : %s", pos, sig)
-			return sig
-		}
-		if m, wrongType := check.missingMethod(V, Ti, true, update); m != nil /* Implements(V, Ti) */ {
+		if m, wrongType := check.missingMethod(V, Ti, true); m != nil /* Implements(V, Ti) */ {
 			if reason != nil {
 				if wrongType != nil {
 					if check.identical(m.typ, wrongType.typ) {

src/go/types/predicates.go

@@ -200,8 +200,11 @@ func (check *Checker) identical0(x, y Type, cmpTags bool, p *ifacePair, tparams
 		// and result values, corresponding parameter and result types are identical,
 		// and either both functions are variadic or neither is. Parameter and result
 		// names are not required to match.
+		// Generic functions must also have matching type parameter lists, but for the
+		// parameter names.
 		if y, ok := y.(*Signature); ok {
 			return x.variadic == y.variadic &&
+				check.identicalTParams(x.tparams, y.tparams, cmpTags, p, tparams) &&
 				check.identical0(x.params, y.params, cmpTags, p, tparams) &&
 				check.identical0(x.results, y.results, cmpTags, p, tparams)
 		}
@@ -331,6 +334,19 @@ func (check *Checker) identical0(x, y Type, cmpTags bool, p *ifacePair, tparams
 	return false
 }
 
+func (check *Checker) identicalTParams(x, y []*TypeName, cmpTags bool, p *ifacePair, tparams []Type) bool {
+	if len(x) != len(y) {
+		return false
+	}
+	for i, x := range x {
+		y := y[i]
+		if !check.identical0(x.typ.(*TypeParam).bound, y.typ.(*TypeParam).bound, cmpTags, p, tparams) {
+			return false
+		}
+	}
+	return true
+}
+
 // Default returns the default "typed" type for an "untyped" type;
 // it returns the incoming type for all other types. The default type
 // for untyped nil is untyped nil.

src/go/types/subst.go

@@ -79,28 +79,8 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist
 		// the parameterized type.
 		iface = check.subst(pos, iface, tparams, targs).(*Interface)
 
-		// update targ method signatures
-		// TODO(gri) This needs documentation and clean up!
-		update := func(V Type, sig *Signature) *Signature {
-			V, _ = deref(V)
-			// check.dump(">>> %s: V = %s", pos, V)
-			var targs []Type
-			if vnamed, _ := V.(*Named); vnamed != nil {
-				targs = vnamed.targs
-			} else {
-				// nothing to do
-				return sig
-			}
-			// check.dump(">>> %s: targs = %s", pos, targs)
-			// check.dump(">>> %s: sig.mtparams = %s", pos, sig.mtparams)
-			// check.dump(">>> %s: sig before: %s, tparams = %s, targs = %s", pos, sig, sig.mtparams, targs)
-			sig = check.subst(pos, sig, sig.rparams, targs).(*Signature)
-			// check.dump(">>> %s: sig after : %s", pos, sig)
-			return sig
-		}
-
 		// targ must implement iface (methods)
-		if m, _ := check.missingMethod(targ, iface, true, update); m != nil {
+		if m, _ := check.missingMethod(targ, iface, true); m != nil {
 			// TODO(gri) needs to print updated name to avoid major confusion in error message!
 			//           (print warning for now)
 			check.softErrorf(pos, "%s does not satisfy %s (warning: name not updated) = %s (missing method %s)", targ, tpar.bound, iface, m)

src/go/types/testdata/tmp.go2

@@ -5,12 +5,18 @@
 package p
 
 type I interface{
-        m(type T)(T)
+        m(type A)(A)
+}
+ 
+type S struct{}
+ 
+func (S) m(type B)(B)
+ 
+var _ I = S{}
+
+type II interface{
+        m(type C)(C)
+        mm() int
 }
 
-type S struct{}
-
-func (S) m(type T)(T)
-
-// TODO(gri) this should work
-var _ I = S /* ERROR wrong type for method m */ {}
+var _ I = II(nil)