From 4fa7fd828d3d4cccb84d200892a1e66c1981e99e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 2 Dec 2019 15:33:00 -0800 Subject: [PATCH] go/types: fix type inference and method calls with parameterized receivers This change expands type inference to parameterized named types and clarifies handling of type parameters during type inference (specifically, an inferred type may be a type parameter itself). The change also implements method selectors with parameterized receiver types by using type inference to deduce the receiver type parameters. Add more tests and enable many disabled tests that now work correctly. Change-Id: I8e10f9166fec9a8454f14b8fb089230c70422a1b --- src/go/types/call.go | 22 ++++++++++---- src/go/types/infer.go | 5 +++- src/go/types/predicates.go | 45 ++++++++++++++++++---------- src/go/types/testdata/contracts.go2 | 6 ++-- src/go/types/testdata/map.go2 | 6 ++-- src/go/types/testdata/map2.go2 | 26 ++++++++-------- src/go/types/testdata/tmp.go2 | 31 +------------------ src/go/types/testdata/typeparams.go2 | 35 ++++++++++++++++++++++ src/go/types/type.go | 2 +- 9 files changed, 106 insertions(+), 72 deletions(-) diff --git a/src/go/types/call.go b/src/go/types/call.go index 35a0a6c6c6..0f12e67d83 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -344,6 +344,7 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { obj Object index []int indirect bool + madeCopy bool // for debugging purposes only ) sel := e.Sel.Name @@ -471,10 +472,21 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { // methods may not have a fully set up signature yet if m, _ := obj.(*Func); m != nil { check.objDecl(m, nil) - // TODO(gri) fix this - if IsParameterized(x.typ) { - check.errorf(x.pos(), "method expressions/values/calls with parameterized receiver types not implemented yet") - goto Error + // 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 { + // 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("### 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) + obj = © + madeCopy = true // if we made a copy, the method set verification below can't work } } @@ -522,7 +534,7 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { // addressability, should we report the type &(x.typ) instead? check.recordSelection(e, MethodVal, x.typ, obj, index, indirect) - if debug { + if debug && !madeCopy { // Verify that LookupFieldOrMethod and MethodSet.Lookup agree. // TODO(gri) This only works because we call LookupFieldOrMethod // _before_ calling NewMethodSet: LookupFieldOrMethod completes diff --git a/src/go/types/infer.go b/src/go/types/infer.go index e5fa2524ec..1e3c0a5ea6 100644 --- a/src/go/types/infer.go +++ b/src/go/types/infer.go @@ -108,7 +108,7 @@ func isParameterized(typ Type, seen map[Type]bool) (res bool) { }() switch t := typ.(type) { - case nil, *Basic, *Named: // TODO(gri) should nil be handled here? + case nil, *Basic: // TODO(gri) should nil be handled here? break case *Array: @@ -159,6 +159,9 @@ func isParameterized(typ Type, seen map[Type]bool) (res bool) { case *Chan: return isParameterized(t.elem, seen) + case *Named: + return isParameterizedList(t.targs, seen) + case *TypeParam: return true diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go index c00a1d0ab9..8c6bf21435 100644 --- a/src/go/types/predicates.go +++ b/src/go/types/predicates.go @@ -118,18 +118,12 @@ func (p *ifacePair) identical(q *ifacePair) bool { return p.x == q.x && p.y == q.y || p.x == q.y && p.y == q.x } +// If a non-nil tparams is provided, type inference is done for type parameters in x. func (check *Checker) identical0(x, y Type, cmpTags bool, p *ifacePair, tparams []Type) bool { if x == y { return true } - // make sure type parameter is in x if we have one - // TODO(gri) this may not be needed: we should only be inferring in one direction, - // from (given) argument types, to possibly free parameter types - if _, ok := x.(*TypeParam); !ok { - x, y = y, x - } - switch x := x.(type) { case *Basic: // Basic types are singletons except for the rune and byte @@ -283,19 +277,40 @@ func (check *Checker) identical0(x, y Type, cmpTags bool, p *ifacePair, tparams case *Named: // Two named types are identical if their type names originate // in the same type declaration. + // if y, ok := y.(*Named); ok { + // return x.obj == y.obj + // } if y, ok := y.(*Named); ok { - return x.obj == y.obj + // Without type inference, type parameters (if any) must match + // exactly and thus the type names must match exactly as well. + if tparams == nil { + // TODO(gri) Why is x == y not sufficient? And if it is, + // we can just return false here because x == y + // is caught in the very beginning of this function. + return x.obj == y.obj + } + + // TODO(gri) This is not always correct: two types may have the same names + // in the same package if one of them is nested in a function. + // Extremely unlikely but we need an always correct solution. + if x.obj.pkg == y.obj.pkg && stripArgNames(x.obj.name) == stripArgNames(y.obj.name) { + assert(len(x.targs) == len(y.targs)) + for i, x := range x.targs { + if !check.identical0(x, y.targs[i], cmpTags, p, tparams) { + return false + } + } + return true + } } case *TypeParam: - if y, ok := y.(*TypeParam); ok { - // TODO(gri) do we need to look at type names here? - // - consider type-checking a generic function calling another generic function - // - what about self-recursive calls? - return x.index == y.index - } + // TODO(gri) do we need to look at type names here? + // - consider type-checking a generic function calling another generic function + // - what about self-recursive calls? + // (may need a map keyed by type parameters with the values the respective inferred types) if tparams == nil { - return false + return false // x and y being equal is caught in the very beginning of this function } if x := tparams[x.index]; x != nil { return check.identical0(x, y, cmpTags, p, tparams) diff --git a/src/go/types/testdata/contracts.go2 b/src/go/types/testdata/contracts.go2 index d79f8cc550..28af96227e 100644 --- a/src/go/types/testdata/contracts.go2 +++ b/src/go/types/testdata/contracts.go2 @@ -157,7 +157,7 @@ func _(type F, C FloatComplex)(c C) { _ = imag(c) re := real(c) im := imag(c) - var _ F = re // TODO(gri) why does this work? (no type conversion needed?) - var _ F = im // TODO(gri) why does this work? (no type conversion needed?) - c = C(complex(re, im)) + var fre F = F(re) + var fim F = F(im) + c = C(complex(fre, fim)) } diff --git a/src/go/types/testdata/map.go2 b/src/go/types/testdata/map.go2 index 0c5880286a..3d8b8d3a99 100644 --- a/src/go/types/testdata/map.go2 +++ b/src/go/types/testdata/map.go2 @@ -53,8 +53,7 @@ func (m *Map(K, V)) Insert(key K, val V) bool { (*pn).val = val return false } - // TODO(gri) investigate assignment - // *pn = &node(K, V){key: key, val: val} + *pn = &node(K, V){key: key, val: val} return true } @@ -86,8 +85,7 @@ func (m *Map(K, V)) InOrder() *Iterator(K, V) { // Stop sending values if sender.Send returns false, // meaning that nothing is listening at the receiver end. return f(n.left) && - // TODO - // sender.Send(keyValue(K, V){n.key, n.val}) && + sender.Send(keyValue(K, V){n.key, n.val}) && f(n.right) } go func() { diff --git a/src/go/types/testdata/map2.go2 b/src/go/types/testdata/map2.go2 index e8c5a90064..b88521427a 100644 --- a/src/go/types/testdata/map2.go2 +++ b/src/go/types/testdata/map2.go2 @@ -53,8 +53,7 @@ func (m *Map(K, V)) Insert(key K, val V) bool { (*pn).val = val return false } - // TODO(gri) look into this assignment - // *pn = &node(K, V){key: key, val: val} + *pn = &node(K, V){key: key, val: val} return true } @@ -86,8 +85,7 @@ func (m *Map(K, V)) InOrder() *Iterator(K, V) { // Stop sending values if sender.Send returns false, // meaning that nothing is listening at the receiver end. return f(n.left) && - // TODO - // sender.Send(keyValue(K, V){n.key, n.val}) && + sender.Send(keyValue(K, V){n.key, n.val}) && f(n.right) } go func() { @@ -96,9 +94,7 @@ func (m *Map(K, V)) InOrder() *Iterator(K, V) { }() // TODO(gri) The design draft doesn't require that we repeat // the type parameters here. Fix the implementation. - _ = receiver - panic(0) - //return &Iterator(K, V){receiver} + return &Iterator(K, V){receiver} // return &Iterator{receiver} } @@ -116,12 +112,7 @@ func (it *Iterator(K, V)) Next() (K, V, bool) { var zerov V return zerok, zerov, false } - // TODO(gri) investigate return - // return keyval.key, keyval.val, true - _ = keyval - var k K - var v V - return k, v, true + return keyval.key, keyval.val, true } // chans @@ -134,6 +125,15 @@ type chans_Sender(type T) struct { done <-chan bool } +func (s *chans_Sender(T)) Send(v T) bool { + select { + case s.values <- v: + return true + case <-s.done: + return false + } +} + func (s *chans_Sender(T)) Close() { close(s.values) } diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index bcba30a448..119b909d98 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -9,34 +9,5 @@ type T(type P) struct {} func (_ T(P)) m1() T(P) func (m T(P)) m2() { - // TODO(gri) this should be valid - var _ T(P) = m /* ERROR cannot use */ .m1() + var _ T(P) = m.m1() } - -/* -type Pair(type K) struct { - key K -} - -type Receiver(type T) struct { - values T -} - -type Iterator(type K) struct { - r Receiver(Pair(K)) -} - -func (r Receiver(T)) Values() T { - return r.values -} - -func (it Iterator(K)) Next() K { - //x := it.r.Values() - // it : Iterator(K) - // it.r : Receiver(Pair(K)) - // it.r.Values: Pair(K) - //var r Receiver(Pair(K)) - var x Pair(K) //= (r.Values)() - return x.key -} -*/ \ No newline at end of file diff --git a/src/go/types/testdata/typeparams.go2 b/src/go/types/testdata/typeparams.go2 index f5496ab1eb..62bfa25aae 100644 --- a/src/go/types/testdata/typeparams.go2 +++ b/src/go/types/testdata/typeparams.go2 @@ -129,3 +129,38 @@ type T struct {} func (T) m1() {} func (T) m2( /* ERROR method must have no type parameters */ type)() {} func (T) m3( /* ERROR method must have no type parameters */ type P)() {} + +// type inference across parameterized types + +type S1(type P) struct { f P } + +func f9(type P)(x S1(P)) + +func _() { + f9(int)(S1(int){42}) + f9(S1(int){42}) +} + +type S2(type A, B, C) struct{} + +func f10(type X, Y, Z)(a S2(X, int, Z), b S2(X, Y, bool)) + +func _(type P)() { + f10(int, float32, string)(S2(int, int, string){}, S2(int, float32, bool){}) + f10(S2(int, int, string){}, S2(int, float32, bool){}) + f10(S2(P, int, P){}, S2(P, float32, bool){}) +} + +// method expressions + +func (_ S1(P)) m() + +func _() { + m := S1(int).m + m(struct { f int }{42}) +} + +func _(type T) (x T) { + m := S1(T).m + m(S1(T){x}) +} diff --git a/src/go/types/type.go b/src/go/types/type.go index c0ab698353..67921b3b7d 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -537,7 +537,7 @@ func (c *Contract) ifaceAt(index int) *Interface { // A TypeParam represents a type parameter type. type TypeParam struct { - id uint64 // unique id + id uint64 // unique id (TODO should this be with the object? all objects?) obj *TypeName index int // parameter index bound Type // either an *Interface or a *Contract