diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 8fd6ffcc06..d912bc838d 100644 --- a/src/go/types/NOTES +++ b/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 diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index 456d55a2da..ab21d04c00 100644 --- a/src/go/types/lookup.go +++ b/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. diff --git a/src/go/types/operand.go b/src/go/types/operand.go index da398d56f4..4c6f1f15cd 100644 --- a/src/go/types/operand.go +++ b/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) { diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go index c44702a09e..b2693d4c75 100644 --- a/src/go/types/predicates.go +++ b/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. diff --git a/src/go/types/subst.go b/src/go/types/subst.go index e168c1aa26..60f13e3298 100644 --- a/src/go/types/subst.go +++ b/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) diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 36a20173e7..2d18ac3d8d 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/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)