From 97251af9c473dc379040fb5c44590d98c0895a97 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Sun, 22 Dec 2019 20:42:57 -0800 Subject: [PATCH] go/types: move tparams field out of Func, clean up Signature Type parameters are now collected as part of signature type checking, as they are part of the type not the object. This opens the door to more uniform handling of type parameters, also for concrete methods and interface methods (future use). Cleaned up confusion between Signature.mtparams and tparams: tparams are explicitly declared function type parameters. mtparams is gone and replaced with rparams which are the receiver type parameters used in the function. Various related cleanups. Change-Id: Id8422b57cc8fbc18ffdca12a69d890aef83c3f80 --- src/go/types/NOTES | 4 +-- src/go/types/call.go | 11 +++---- src/go/types/decl.go | 60 ++------------------------------------ src/go/types/object.go | 25 ++-------------- src/go/types/subst.go | 18 ++---------- src/go/types/type.go | 8 ++--- src/go/types/typestring.go | 2 ++ src/go/types/typexpr.go | 51 ++++++++++++++++++++++++++++++-- 8 files changed, 68 insertions(+), 111 deletions(-) diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 1d22f05d6a..4e84837229 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -2,6 +2,8 @@ This file works as a sort of notebook/implementation log. It replaces my noteboo so we have a better track record. I only switched to this file recently, hence it is incomplete. TODO +- Are function type parameters in the same scope as other parameters? TBD. + (Right now they are in an enclosing scope.) - use Underlying() to return a type parameter's bound? investigate! - better error message when declaring a contract local to a function (parser gets out of sync) - if type parameters are repeated in recursive instantiation, they must be the same order (not yet checked) @@ -9,8 +11,6 @@ TODO - interface embedding doesn't take care of literal type constraints yet (need an allTypes list, like we have an allMethods list?) - type assertions on/against parameterized types -- move Func.tparams to Signature (as we have done for TypeName.tparams to Named) -- distinguish more clearly between Signature.tparams and mtparams (the latter are solely for recv type params) - use []*TypeParam for tparams in subst? (unclear) OPEN ISSUES diff --git a/src/go/types/call.go b/src/go/types/call.go index e6ed5773a3..4f89802fa8 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -489,20 +489,21 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { // If m has a parameterized receiver type, infer the type parameter // values from the actual receiver provided and then substitute the // type parameters in the signature accordingly. - if len(m.tparams) > 0 { + sig := m.typ.(*Signature) + if len(sig.rparams) > 0 { // check.dump("### recv typ = %s", x.typ) - // check.dump("### method = %s tparams = %s", m, m.tparams) - recv := m.typ.(*Signature).recv - targs := check.infer(recv.pos, m.tparams, NewTuple(recv), []*operand{x}) + // check.dump("### method = %s tparams = %s", m, m.rparams) + targs := check.infer(sig.recv.pos, sig.rparams, NewTuple(sig.recv), []*operand{x}) // check.dump("### inferred targs = %s", targs) // Don't modify m. Instead - for now - make a copy of m and use that instead. // (If we modify m, some tests will fail; possibly because the m is in use.) copy := *m - copy.typ = check.subst(e.Pos(), m.typ, m.tparams, targs) + copy.typ = check.subst(e.Pos(), m.typ, sig.rparams, targs) obj = © } // TODO(gri) we also need to do substitution for parameterized interface methods // (this breaks code in testdata/linalg.go2 at the moment) + // 12/20/2019: Is this TODO still correct? } if x.mode == typexpr { diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 7579737ff8..a81a85c5c7 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -748,52 +748,6 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) { // func declarations cannot use iota assert(check.iota == nil) - fdecl := decl.fdecl - if fdecl.IsMethod() { - _, rname, tparams := check.unpackRecv(fdecl.Recv.List[0].Type, true) - if len(tparams) > 0 { - // declare the method's receiver type parameters - check.openScope(fdecl, "receiver type parameters") - defer check.closeScope() - // collect and declare the type parameters - // TODO(gri) should assign this to a specific recv tparam list - obj.tparams = check.declareTypeParams(nil, tparams, nil) - // determine receiver type to get its type parameters - // and the respective type parameter bounds - var recvTParams []*TypeName - if rname != nil { - // recv should be a Named type (otherwise an error is reported elsewhere) - if recv, _ := check.genericType(rname).(*Named); recv != nil { - recvTParams = recv.tparams - } - } - // provide type parameter bounds - // - only do this if we have the right number (otherwise an error is reported elsewhere) - if len(obj.tparams) == len(recvTParams) { - list := make([]Type, len(obj.tparams)) - for i, t := range obj.tparams { - list[i] = t.typ - } - for i, tname := range obj.tparams { - bound := recvTParams[i].typ.(*TypeParam).bound - // bound is (possibly) parameterized in the context of the - // receiver type declaration. Substitute parameters for the - // current context. - // TODO(gri) should we assume now that bounds always exist? - // (no bound == empty interface) - if bound != nil { - bound = check.subst(tname.pos, bound, recvTParams, list) - tname.typ.(*TypeParam).bound = bound - } - } - } - } - } else if fdecl.TParams != nil { - check.openScope(fdecl, "function type parameters") - defer check.closeScope() - obj.tparams = check.collectTypeParams(fdecl.TParams) - } - sig := new(Signature) obj.typ = sig // guard against cycles @@ -805,20 +759,10 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) { // 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) + fdecl := decl.fdecl + check.funcType(sig, fdecl.Recv, fdecl.TParams, fdecl.Type) obj.color_ = saved - // 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 - sig.tparams = obj.tparams - } - // function body must be type-checked after global declarations // (functions implemented elsewhere have no body) if !check.conf.IgnoreFuncBodies && fdecl.Body != nil { diff --git a/src/go/types/object.go b/src/go/types/object.go index 45517280e0..2880c24123 100644 --- a/src/go/types/object.go +++ b/src/go/types/object.go @@ -293,8 +293,7 @@ func (*Var) isDependency() {} // a variable may be a dependency of an initializa // An abstract method may belong to many interfaces due to embedding. type Func struct { object - hasPtrRecv bool // only valid for methods that don't have a type yet - tparams []*TypeName // type parameters from left to right (rcvr parameters for methods); or nil + hasPtrRecv bool // only valid for methods that don't have a type yet } // NewFunc returns a new function with the given signature, representing @@ -305,7 +304,7 @@ func NewFunc(pos token.Pos, pkg *Package, name string, sig *Signature) *Func { if sig != nil { typ = sig } - return &Func{object{nil, pos, pkg, name, typ, 0, colorFor(typ), token.NoPos}, false, nil} + return &Func{object{nil, pos, pkg, name, typ, 0, colorFor(typ), token.NoPos}, false} } // FullName returns the package- or receiver-type-qualified name of @@ -396,26 +395,6 @@ func writeObject(buf *bytes.Buffer, obj Object, qf Qualifier) { case *Func: buf.WriteString("func ") writeFuncName(buf, obj, qf) - // Func.tparams is used for functions and methods; but for methods - // these are the receiver parameters. Don't print them twice. - // TODO(gri) Consider putting receiver and type parameters in the Func - // object, not the signature. That might simplify things. - if len(obj.tparams) > 0 && (typ == nil || typ.(*Signature).recv == nil) { - buf.WriteString("(type ") - for i, tname := range obj.tparams { - if i > 0 { - buf.WriteString(", ") - } - buf.WriteString(tname.name) - if tname.typ != nil && tname.typ.(*TypeParam).bound != nil { - // TODO(gri) Instead of writing the bound with each type - // parameter, consider bundling them up. - buf.WriteByte(' ') - WriteType(buf, tname.typ.(*TypeParam).bound, nil) - } - } - buf.WriteByte(')') - } if typ != nil { WriteSignature(buf, typ.(*Signature), qf) } diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 22bab1d521..369b79e943 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -80,7 +80,7 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist iface = check.subst(pos, iface, tparams, targs).(*Interface) // update targ method signatures - // TODO(gri) This needs documentation and cleanups! + // 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) @@ -94,7 +94,7 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist // 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) + sig = check.subst(pos, sig, sig.rparams, targs).(*Signature) // check.dump(">>> %s: sig after : %s", pos, sig) return sig } @@ -146,19 +146,6 @@ 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: @@ -400,7 +387,6 @@ func (subst *subster) varList(in []*Var) (out []*Var, copied bool) { } func (subst *subster) func_(f *Func) *Func { - assert(len(f.tparams) == 0) if f != nil { if typ := subst.typ(f.typ); typ != f.typ { copy := *f diff --git a/src/go/types/type.go b/src/go/types/type.go index 8a118f00a3..a23e7b9c33 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -199,12 +199,10 @@ type Signature struct { // and store it in the Func Object) because when type-checking a function // literal we call the general type checker which returns a general Type. // We then unpack the *Signature and use the scope for the literal body. - scope *Scope // function scope, present for package-local signatures - recv *Var // nil if not a method - // 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?) + scope *Scope // function scope, present for package-local signatures + recv *Var // nil if not a method + rparams []*TypeName // reveiver type parameters from left to right; or nil 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) diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go index 15ba361d55..c8f23ffa17 100644 --- a/src/go/types/typestring.go +++ b/src/go/types/typestring.go @@ -295,6 +295,8 @@ func writeTParamList(buf *bytes.Buffer, list []*TypeName, qf Qualifier, visited } buf.WriteString(p.name) if ptyp, _ := p.typ.(*TypeParam); ptyp != nil && ptyp.bound != nil { + // TODO(gri) Instead of writing the bound with each type + // parameter, consider bundling them up. buf.WriteByte(' ') writeType(buf, ptyp.bound, qf, visited) // TODO(gri) if this is a generic type bound, we should print diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index e43dcc8211..a3a7f20978 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -157,7 +157,54 @@ func (check *Checker) genericType(e ast.Expr) Type { } // funcType type-checks a function or method type. -func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast.FuncType) { +func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftparams *ast.FieldList, ftyp *ast.FuncType) { + if recvPar != nil && len(recvPar.List) > 0 { + _, rname, rparams := check.unpackRecv(recvPar.List[0].Type, true) + if len(rparams) > 0 { + // declare the method's receiver type parameters + check.openScope(recvPar, "receiver type parameters") + defer check.closeScope() + // collect and declare the type parameters + sig.rparams = check.declareTypeParams(nil, rparams, nil) + // determine receiver type to get its type parameters + // and the respective type parameter bounds + var recvTParams []*TypeName + if rname != nil { + // recv should be a Named type (otherwise an error is reported elsewhere) + if recv, _ := check.genericType(rname).(*Named); recv != nil { + recvTParams = recv.tparams + } + } + // provide type parameter bounds + // - only do this if we have the right number (otherwise an error is reported elsewhere) + if len(sig.rparams) == len(recvTParams) { + list := make([]Type, len(sig.rparams)) + for i, t := range sig.rparams { + list[i] = t.typ + } + for i, tname := range sig.rparams { + bound := recvTParams[i].typ.(*TypeParam).bound + // bound is (possibly) parameterized in the context of the + // receiver type declaration. Substitute parameters for the + // current context. + // TODO(gri) should we assume now that bounds always exist? + // (no bound == empty interface) + if bound != nil { + bound = check.subst(tname.pos, bound, recvTParams, list) + tname.typ.(*TypeParam).bound = bound + } + } + } + } + } + + if ftparams != nil { + // TODO(gri) should this be the same scope as for value parameters? + check.openScope(ftparams, "function type parameters") + defer check.closeScope() + sig.tparams = check.collectTypeParams(ftparams) + } + scope := NewScope(check.scope, token.NoPos, token.NoPos, "function") scope.isFunc = true check.recordScope(ftyp, scope) @@ -328,7 +375,7 @@ func (check *Checker) typInternal(e ast.Expr, def *Named) (T Type) { case *ast.FuncType: typ := new(Signature) def.setUnderlying(typ) - check.funcType(typ, nil, e) + check.funcType(typ, nil, nil, e) return typ case *ast.InterfaceType: