From 9c75653a798e1389d495dac4cf17b36660af8d80 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 6 Jan 2020 22:40:21 -0800 Subject: [PATCH] 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 | 1 + src/go/types/lookup.go | 61 ++++++++++++++++++++++++++++------- src/go/types/operand.go | 22 +------------ src/go/types/predicates.go | 16 +++++++++ src/go/types/subst.go | 22 +------------ src/go/types/testdata/tmp.go2 | 20 ++++++++---- 6 files changed, 81 insertions(+), 61 deletions(-) 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)