[dev.typeparams] go/types: unify methods in missingMethod

Unify methods in Checker.missingMethod. This code was accidentally
dropped from the merge, while dropping support for method type
parameters, but is needed for checking implementations of generic
interfaces.

Put the logic back, including checks that are only needed for method
type parameters. It makes the code no simpler to assume that method type
parameters are disallowed, and we have checks elsewhere that produce
errors for methods with type parameters.

Change-Id: I91f0c9d3e04537fdb9f7ae23a4ce4cec9f1da10e
Reviewed-on: https://go-review.googlesource.com/c/go/+/284252
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go
index f385ac993f7bd3a8467c9ee5525f03a900cd3edf..a0e7f3cc0dba7fc6fe1bf830259f51e75b126149 100644 (file)
--- a/src/go/types/lookup.go
+++ b/src/go/types/lookup.go
@@ -325,21 +325,30 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method,
                                return m, f
                        }
 
-                       if !check.identical(f.Type(), m.Type()) {
+                       ftyp := f.typ.(*Signature)
+                       mtyp := m.typ.(*Signature)
+                       if len(ftyp.tparams) != len(mtyp.tparams) {
                                return m, f
                        }
 
-                       // TODO(rFindley) delete this note once the spec has stabilized to
-                       //                exclude method type parameters.
-                       // NOTE: if enabling method type parameters, we need to unify f.Type()
-                       // and m.Type() here to verify that their type parameters align (assuming
-                       // this behaves correctly with respect to type bounds).
+                       // If the methods have type parameters we don't care whether they
+                       // are the same or not, as long as they match up. Use unification
+                       // to see if they can be made to match.
+                       // TODO(gri) is this always correct? what about type bounds?
+                       // (Alternative is to rename/subst type parameters and compare.)
+                       u := newUnifier(check, true)
+                       u.x.init(ftyp.tparams)
+                       if !u.unify(ftyp, mtyp) {
+                               return m, f
+                       }
                }
 
                return
        }
 
        // A concrete type implements T if it implements all methods of T.
+       Vd, _ := deref(V)
+       Vn := asNamed(Vd)
        for _, m := range T.allMethods {
                // TODO(gri) should this be calling lookupFieldOrMethod instead (and why not)?
                obj, _, _ := check.rawLookupFieldOrMethod(V, false, m.pkg, m.name)
@@ -368,16 +377,43 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method,
                        check.objDecl(f, nil)
                }
 
-               if !check.identical(f.Type(), m.Type()) {
+               // 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
                }
 
-               // TODO(rFindley) delete this note once the spec has stabilized to exclude
-               //                method type parameters.
-               // NOTE: if enabling method type parameters, one needs to subst any
-               // receiver type parameters for V here, and unify f.Type() with m.Type() to
-               // verify that their type parameters align (assuming this behaves correctly
-               // with respect to type bounds).
+               // 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 signature of method f.
+               if Vn != nil && len(Vn.tparams) > 0 {
+                       // Be careful: The number of type arguments may not match
+                       // the number of receiver parameters. If so, an error was
+                       // reported earlier but the length discrepancy is still
+                       // here. Exit early in this case to prevent an assertion
+                       // failure in makeSubstMap.
+                       // TODO(gri) Can we avoid this check by fixing the lengths?
+                       if len(ftyp.rparams) != len(Vn.targs) {
+                               return
+                       }
+                       ftyp = check.subst(token.NoPos, ftyp, makeSubstMap(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 unification
+               // to see if they can be made to match.
+               // TODO(gri) is this always correct? what about type bounds?
+               // (Alternative is to rename/subst type parameters and compare.)
+               u := newUnifier(check, true)
+               u.x.init(ftyp.tparams)
+               if !u.unify(ftyp, mtyp) {
+                       return m, f
+               }
        }
 
        return