go/types: fixed various issues in check.subst

Next known issue:

type List(type E) []E
var _ List(List(List(int))) = [](List(List(int))){}

This won't work because the RHS's composite literal type
is not fully instantiated - it has non-instantiated elements
which are ignored by Checker.instantiatedType. We could just
call Checker.subst w/o any parameters and have it walk the
incoming type and instantiate any element Parameterized types
but that is expensive and also appears to lead to stack over-
flow. Need to investigate.

Change-Id: I80bd09ab5f06e3991f198965424ce3322c7d6402

src/go/types/call.go

@@ -187,7 +187,7 @@ func (check *Checker) instantiate(typ Type, tparams []*TypeName, args []*operand
 		targs[i] = a.typ
 	}
 	// result is instantiated typ
-	return check.subst(typ, targs)
+	return check.subst(typ, tparams, targs)
 }
 
 func (check *Checker) exprList(elist []ast.Expr, allowCommaOk bool) (xlist []*operand, commaOk bool) {
@@ -312,8 +312,8 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, args []*oper
 		if targs == nil {
 			return
 		}
-		rsig = check.subst(sig, targs).(*Signature)
-		params = check.subst(params, targs).(*Tuple)
+		rsig = check.subst(sig, sig.tparams, targs).(*Signature)
+		params = check.subst(params, sig.tparams, targs).(*Tuple)
 		// TODO(gri) Optimization: We don't need to check arguments
 		//           from which we inferred parameter types.
 	}

src/go/types/infer.go

@@ -39,7 +39,7 @@ func (check *Checker) infer(pos token.Pos, tparams []*TypeName, params *Tuple, a
 		}
 		par := params.At(i)
 		if !check.identical0(par.typ, arg.typ, true, nil, targs) {
-			check.errorf(arg.pos(), "type %s for %s does not match %s = %s", arg.typ, arg.expr, par.typ, check.subst(par.typ, targs))
+			check.errorf(arg.pos(), "type %s for %s does not match %s = %s", arg.typ, arg.expr, par.typ, check.subst(par.typ, tparams, targs))
 			return nil
 		}
 	}
@@ -52,7 +52,7 @@ func (check *Checker) infer(pos token.Pos, tparams []*TypeName, params *Tuple, a
 		}
 		par := params.At(i)
 		if !check.identical0(par.typ, Default(arg.typ), true, nil, targs) {
-			check.errorf(arg.pos(), "default type %s for %s does not match %s = %s", Default(arg.typ), arg.expr, par.typ, check.subst(par.typ, targs))
+			check.errorf(arg.pos(), "default type %s for %s does not match %s = %s", Default(arg.typ), arg.expr, par.typ, check.subst(par.typ, tparams, targs))
 			return nil
 		}
 	}

src/go/types/subst.go

@@ -12,24 +12,48 @@ import (
 	"bytes"
 )
 
-func (check *Checker) subst(typ Type, targs []Type) Type {
-	if len(targs) == 0 {
+// inst returns the instantiated type of tname.
+func (check *Checker) inst(tname *TypeName, targs []Type) (res Type) {
+	if trace {
+		check.trace(tname.pos, "-- instantiating %s", tname)
+		check.indent++
+		defer func() {
+			check.indent--
+			check.trace(tname.pos, "=> %s", res)
+		}()
+	}
+
+	return check.subst(tname.typ, tname.tparams, targs)
+}
+
+// subst returns the type typ with its type parameters tparams replaced by
+// the corresponding type arguments targs, recursively.
+//
+// TODO(gri) tparams is only passed so we can verify that the type parameters
+// occuring in typ are the ones from typ's parameter list. We should be able
+// to prove that this is always the case and then we don't need this extra
+// argument anymore.
+func (check *Checker) subst(typ Type, tparams []*TypeName, targs []Type) Type {
+	assert(len(tparams) == len(targs))
+	if len(tparams) == 0 {
 		return typ
 	}
-	s := subster{check, targs, make(map[Type]Type)}
+	s := subster{check, make(map[Type]Type), tparams, targs}
 	return s.typ(typ)
 }
 
 type subster struct {
-	check *Checker
-	targs []Type
-	cache map[Type]Type
+	check   *Checker
+	cache   map[Type]Type
+	tparams []*TypeName
+	targs   []Type
 }
 
 func (s *subster) typ(typ Type) (res Type) {
-	// TODO(gri) this is not correct in the presence of cycles
-	if t, hit := s.cache[typ]; hit {
-		return t
+	// avoid repeating the same substitution for a given type
+	// TODO(gri) is this correct in the presence of cycles?
+	if typ, hit := s.cache[typ]; hit {
+		return typ
 	}
 	defer func() {
 		s.cache[typ] = res
@@ -95,9 +119,7 @@ func (s *subster) typ(typ Type) (res Type) {
 		}
 
 	case *Named:
-		underlying := s.typ(t.underlying)
-		//s.check.dump("  underlying = %s", underlying)
-		//s.check.dump("t.underlying = %s", t.underlying)
+		underlying := s.typ(t.underlying).Underlying()
 		if underlying != t.underlying {
 			// create a new named type - for now use printed type in name
 			// TODO(gri) consider type map to map types to indices (on the other hand, a type string seems just as good)
@@ -105,11 +127,12 @@ func (s *subster) typ(typ Type) (res Type) {
 				panic("cannot handle instantiation of types with methods yet")
 			}
 			// TODO(gri) review name creation and factor out
-			name := t.obj.name + typesString(s.targs)
+			name := t.obj.name + typeArgsString(s.targs)
 			tname, found := s.check.typMap[name]
 			if !found {
 				// TODO(gri) what is the correct position to use here?
 				tname = NewTypeName(t.obj.pos, s.check.pkg, name, nil)
+				//s.check.dump("name = %s", name)
 				NewNamed(tname, underlying, nil) // TODO(gri) provide correct method list
 				s.check.typMap[name] = tname
 			}
@@ -118,15 +141,21 @@ func (s *subster) typ(typ Type) (res Type) {
 
 	case *Parameterized:
 		// first, instantiate any arguments if necessary
+		// TODO(gri) should this be done in check.inst
+		// and thus for any caller of check.inst)?
 		targs := make([]Type, len(t.targs))
 		for i, a := range t.targs {
 			targs[i] = s.typ(a) // TODO(gri) fix this
 		}
 		// then instantiate t
-		return s.check.subst(t.tname.typ, targs)
+		return s.check.inst(t.tname, targs)
 
 	case *TypeParam:
-		// TODO(gri) do we need to check that we're using the correct targs list/index?
+		// verify that the type parameter t is from the correct
+		// parameterized type
+		assert(s.tparams[t.index] == t.obj)
+		// TODO(gri) targ may be nil in error messages from check.infer.
+		// Eliminate that possibility and then we don't need this check.
 		if targ := s.targs[t.index]; targ != nil {
 			return targ
 		}
@@ -179,7 +208,7 @@ func (s *subster) varList(in []*Var) (out []*Var, copied bool) {
 	return
 }
 
-func typesString(targs []Type) string {
+func typeArgsString(targs []Type) string {
 	var buf bytes.Buffer
 	buf.WriteByte('<')
 	for i, arg := range targs {

src/go/types/testdata/tmp.go2

@@ -4,10 +4,8 @@
 
 package p
 
-type A(type P, Q) = P
+type List(type P) []P
 
-var x A(int, string)
+type T(type P) List(P)
 
-type T(type P) struct {
-	f A(P, string)
-}
+var _ T(int) //= T(int)(List(int){1, 2, 3})

src/go/types/testdata/typeinst.go2

@@ -58,7 +58,7 @@ var _ List(List(List(int)))
 
 type T3(type P) List(P)
 
-//var _ T3(int) = (T3(int))(List(int){1, 2, 3}) // TODO(gri) cannot do this yet
+var _ T3(int) = T3(int)(List(int){1, 2, 3})
 
 
 // TODO

src/go/types/typexpr.go

@@ -147,15 +147,11 @@ func (check *Checker) definedType(e ast.Expr, def *Named) (T Type) {
 func (check *Checker) instantiatedType(e ast.Expr) Type {
 	typ := check.typ(e)
 	if ptyp, _ := typ.(*Parameterized); ptyp != nil {
-		tname := ptyp.tname
-		typ = check.subst(tname.typ, ptyp.targs)
+		typ = check.inst(ptyp.tname, ptyp.targs)
+		// TODO(gri) can this ever be nil? comment.
 		if typ == nil {
 			return Typ[Invalid] // error was reported by check.instatiate
 		}
-
-		if trace {
-			check.trace(e.Pos(), "instantiated %s -> %s", tname, typ)
-		}
 	}
 	return typ
 }