From a28b69c5d0ac3a7fde041c9d2876050dfe7b038c Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 18 Dec 2019 17:23:46 -0800 Subject: [PATCH] go/types: various attempts at lazy substitution of methods for lookups (snapshot) When instantiating a type, we should also instantiate its methods (at least their signatures) so that they have the correct type when being looked up. Unfortunately, methods may not yet be type-checked; and worse, when we type-check them, typing their receiver means instantiating that type again which would require updating (= instantiating) the methods... Instead, we keep the original list of methods (*Funcs) with each (named) type and only update the method signatures when looking them up. The problem with this approach is that we need to know with which values to substitute the method (i.e., receiver) type parameters, and this is currently not working. Change-Id: Ie1835919dc8bfb8161a6a9e3d784c3bbbeb958c4 --- src/go/types/decl.go | 8 +++- src/go/types/lookup.go | 14 ++----- src/go/types/subst.go | 72 +++++++++++++++++++++++++++++------ src/go/types/testdata/tmp.go2 | 60 ++++++++++++++++++++++++----- src/go/types/type.go | 3 +- 5 files changed, 122 insertions(+), 35 deletions(-) diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 2d665659ec..8b5fab30ee 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -798,13 +798,17 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) { // since the method is not a type, we get an error. If we have a parameterized // receiver type, instantiating the receiver type leads to the instantiation of // its methods, and we don't want a cycle error in that case. - // TODO(gri) review if this is correct for the latter case + // TODO(gri) review if this is correct and/or whether we still need this? saved := obj.color_ obj.color_ = black check.funcType(sig, fdecl.Recv, fdecl.Type) obj.color_ = saved - if !fdecl.IsMethod() { + // TODO(gri) we should just use one tparam list - clean up + if fdecl.IsMethod() { + // needed for updating methods during instantiation bounds checking + sig.mtparams = obj.tparams + } else { // only functions can have type parameters that need to be passed // (the obj.tparams for methods are the receiver parameters) // TODO(gri) remove the need for storing tparams in signatures diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index 30ef4f942b..456d55a2da 100644 --- a/src/go/types/lookup.go +++ b/src/go/types/lookup.go @@ -281,7 +281,7 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b // 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(Type) Type) (method, wrongType *Func) { +func (check *Checker) missingMethod(V Type, T *Interface, static bool, update func(V Type, sig *Signature) *Signature) (method, wrongType *Func) { check.completeInterface(token.NoPos, T) // fast path for common case @@ -302,13 +302,7 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, update fu continue } - // update (generic) method signatures before comparing them - ftyp := f.Type() - if update != nil { - ftyp = update(ftyp) - } - - if !check.identical(ftyp, m.typ) { + if !check.identical(f.typ, m.typ) { return m, f } } @@ -341,9 +335,9 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, update fu } // update (generic) method signatures before comparing them - ftyp := f.typ + ftyp := f.typ.(*Signature) if update != nil { - ftyp = update(ftyp) + ftyp = update(V, ftyp) } if !check.identical(ftyp, m.typ) { diff --git a/src/go/types/subst.go b/src/go/types/subst.go index fe28931caa..e3ca4d3ab1 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -76,22 +76,36 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist // determine type parameter bound iface := tpar.Interface() - // TODO(gri) document/explain why the substitution below is correct - //check.dump(">>> %s: iface before: %s", pos, iface) + // The type parameter bound is parameterized with the same type parameters + // as the instantiated type; before we can use it for bounds checking we + // need to instantiate it with the type arguments with which we instantiate + // the parameterized type. iface = check.subst(pos, iface, tparams, targs).(*Interface) - //check.dump(">>> %s: iface after : %s", pos, iface) // update targ method signatures - update := func(typ Type) Type { - // check.dump(">>> %s: sig before: %s (tparams = %s)", pos, typ, typ.(*Signature).tparams) - typ = check.subst(pos, typ, tparams, targs) - // check.dump(">>> %s: sig after : %s", pos, typ) - return typ + 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.mtparams, 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 { - check.softErrorf(pos, "%s does not satisfy %s (missing method %s)", targ, tpar.bound, m) + // 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) break } @@ -126,7 +140,7 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist return check.subst(pos, typ, tparams, targs) } -// subst returns the type typ with its type parameters tparams replaced by +// subst returns the type typ with its type parameters tpars replaced by // the corresponding type arguments targs, recursively. func (check *Checker) subst(pos token.Pos, typ Type, tpars []*TypeName, targs []Type) Type { assert(len(tpars) == len(targs)) @@ -134,6 +148,19 @@ func (check *Checker) subst(pos token.Pos, typ Type, tpars []*TypeName, targs [] return typ } + if debug { + for i, tpar := range tpars { + if tpar == nil { + check.dump("%s: tpars[%d] == nil", pos, i) + panic("nil type parameter") + } + if targs[i] == nil { + check.dump("%s: targ[%d] == nil", pos, i) + panic("nil type argument") + } + } + } + // common cases switch t := typ.(type) { case *Basic: @@ -162,6 +189,9 @@ type subster struct { func (subst *subster) typ(typ Type) Type { switch t := typ.(type) { + case nil: + panic("nil typ") + case *Basic: // nothing to do @@ -253,7 +283,7 @@ func (subst *subster) typ(typ Type) Type { // already instantiated dump(">>> %s already instantiated", t) - assert(len(t.targs) == len(t.obj.tparams)) + assert(len(t.targs) == len(t.targs)) // For each (existing) type argument targ, determine if it needs // to be substituted; i.e., if it is or contains a type parameter // that has a type argument for it. @@ -301,7 +331,25 @@ func (subst *subster) typ(typ Type) Type { subst.cache[t] = named dump(">>> subst %s(%s) with %s (new: %s)", t.underlying, subst.tpars, subst.targs, new_targs) named.underlying = subst.typ(t.underlying) - named.methods = t.methods // methods will be customized when used + + // update all method signatures + named.methods = t.methods + /* + if len(t.methods) > 0 { + dump(">>> update method signatures:") + for _, m := range t.methods { + subst.check.objDecl(m, nil) + copy := *m + copy.typ = subst.typ(copy.typ) + dump(">>> - %s: %s -> %s", m, m.typ, copy.typ) + named.methods = append(named.methods, ©) + } + dump(">>> done with methods") + } else { + dump(">>> no methods to update") + } + */ + return named case *TypeParam: diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 188c4f0a5d..24adce03fe 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -4,19 +4,14 @@ package p -type NumericAbs(type T) interface { - Abs() T -} +func f(type P interface{ m() P })(x P) -func AbsDifference(type T NumericAbs(T))(x T) +type T(type P) P -type OrderedAbs(type T) T +func (_ T(P)) m() P -func (a OrderedAbs(T)) Abs() T - -func OrderedAbsDifference(type T)(x T) { - // TODO(gri) this should work - AbsDifference /* ERROR does not satisfy */ (OrderedAbs(T)(x)) +func _(type Q)(x Q) { + f(T /* ERROR does not satisfy */ (Q))(T(Q)(x)) } // TODO(gri) Once the code above works, check the code below. @@ -31,6 +26,51 @@ type OrderedAbs(type T) T func (a OrderedAbs(T)) Abs() T +func OrderedAbsDifference(type T)() { + AbsDifference(OrderedAbs(T))() +} +*/ + +/* +package p + +type T(type _ interface { a() }, _ interface{}) struct{} + +type A(type P) struct{ x P } + +func (_ A(P)) a() {} + +var _ T(A(int), int) +*/ + +/* +type NumericAbs(type T) interface { + Abs() T +} + +func AbsDifference(type T NumericAbs(T))(x T) + +type OrderedAbs(type T) T + +func (a OrderedAbs(T)) Abs() T + +func OrderedAbsDifference(type T)(x T) { + AbsDifference(OrderedAbs(T)(x)) +} +*/ + +// TODO(gri) Once the code above works, check the code below. +/* +type NumericAbs(type T) interface { + Abs() T +} + +func AbsDifference(type T NumericAbs(T))() + +type OrderedAbs(type T) T + +func (a OrderedAbs(T)) Abs() T + func OrderedAbsDifference(type T)() { AbsDifference(OrderedAbs(T))() } diff --git a/src/go/types/type.go b/src/go/types/type.go index 19fde623a0..682309e035 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -204,6 +204,7 @@ type Signature struct { // TODO(gri) do we need to keep tparams in the Signature, rather than the Func object? // (how are they different from type parameters which we keep with the TypeName?) tparams []*TypeName // type parameters from left to right; or nil + mtparams []*TypeName // method type parameters params *Tuple // (incoming) parameters from left to right; or nil results *Tuple // (outgoing) results from left to right; or nil variadic bool // true if the last parameter's type is of the form ...T (or string, for append built-in only) @@ -223,7 +224,7 @@ func NewSignature(recv *Var, params, results *Tuple, variadic bool) *Signature { panic("types.NewSignature: variadic parameter must be of unnamed slice type") } } - return &Signature{nil, recv, nil, params, results, variadic} + return &Signature{nil, recv, nil, nil, params, results, variadic} } // Recv returns the receiver of signature s (if a method), or nil if a