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

@@ -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

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 = &copy
 		}
 		// 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 {

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 {

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)
 		}

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

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)

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

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: