From 2a7660310166d1a78b68347419e791ec6fe344cf Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Sat, 13 Jul 2019 14:32:00 -0700 Subject: [PATCH] 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 | 6 +-- src/go/types/infer.go | 4 +- src/go/types/subst.go | 61 ++++++++++++++++++++++-------- src/go/types/testdata/tmp.go2 | 8 ++-- src/go/types/testdata/typeinst.go2 | 2 +- src/go/types/typexpr.go | 8 +--- 6 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/go/types/call.go b/src/go/types/call.go index 4a4a292c4a..da7dae8165 100644 --- a/src/go/types/call.go +++ b/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. } diff --git a/src/go/types/infer.go b/src/go/types/infer.go index 51d07f620a..2fa0b3979b 100644 --- a/src/go/types/infer.go +++ b/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 } } diff --git a/src/go/types/subst.go b/src/go/types/subst.go index bc1d9e035c..c0224fe691 100644 --- a/src/go/types/subst.go +++ b/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 { diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index b84d6eb72f..820a4f7e9a 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/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}) \ No newline at end of file diff --git a/src/go/types/testdata/typeinst.go2 b/src/go/types/testdata/typeinst.go2 index 18815d5e84..ad9dad05e1 100644 --- a/src/go/types/testdata/typeinst.go2 +++ b/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 diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 54db0fc3fa..5823e3c674 100644 --- a/src/go/types/typexpr.go +++ b/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 }