From a30aa6c43eb3c56d387fbe11cb483d819fa43887 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 11 Mar 2020 22:15:39 -0700 Subject: [PATCH] go/types: fix contract embedding Contract embedding is reduced to interface embedding. When embedding interfaces: 1) use the correct interface to embed; and 2) don't leave type bounds residue on the incoming type arguments of the embedded interface/contract as those are also the type parameters of the embedding (outer) interface/contract. Also: - For now, print "any" instead of "interface{}" (empty interface) when printing type parameter lists to reduce clutter. Eventually we should not print anything for empty interface bounds, but we first must also group type parameters with the same type bounds. - Print the type parameter subscript in type parameter lists to make it clearer which type parameter we are referring to. Change-Id: Ic83516096387d0f512c4c76a8a8fe849d51e033a --- src/go/types/NOTES | 2 +- src/go/types/api_test.go | 4 ++-- src/go/types/contracts.go | 28 ++++++++++++------------ src/go/types/examples/contracts.go2 | 8 +++---- src/go/types/testdata/tmp.go2 | 33 +++++++---------------------- src/go/types/typestring.go | 17 +++++++++++++-- 6 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 65715fe13e..08b982b6e4 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -4,6 +4,7 @@ so we have a better track record. I only switched to this file in Nov 2019, henc ---------------------------------------------------------------------------------------------------- TODO +- review use of Contract.TParams field - it seems like it's only needed for length checks? - review handling of fields of instantiated generic types (do we need to make them non-parameterized, similar to what we did for the embedded interfaces created by contract embedding?) - debug (and error msg) printing of generic instantiated types needs some work @@ -15,7 +16,6 @@ TODO ---------------------------------------------------------------------------------------------------- KNOWN ISSUES -- parameterized contract embedding is not working correctly - a non-comparable type used as type argument for a comparable contract leads to sub-optimal error message (referring to missing method ==) - iteration over generic variables doesn't report certain channel errors (see TODOs in code) diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go index 170f819522..bf5034cf9e 100644 --- a/src/go/types/api_test.go +++ b/src/go/types/api_test.go @@ -276,9 +276,9 @@ func TestTypesInfo(t *testing.T) { {`package x5; func _() { var x map[string][...]int; x = map[string][...]int{"": {1,2,3}} }`, `x`, `map[string][-1]int`}, // parameterized functions - {`package p0; func f(type T)(T); var _ = f(int)`, `f`, `func(type T interface{})(T₁)`}, + {`package p0; func f(type T)(T); var _ = f(int)`, `f`, `func(type T₁ any)(T₁)`}, {`package p1; func f(type T)(T); var _ = f(int)`, `f(int)`, `func(int)`}, - {`package p2; func f(type T)(T); var _ = f(42)`, `f`, `func(type T interface{})(T₁)`}, + {`package p2; func f(type T)(T); var _ = f(42)`, `f`, `func(type T₁ any)(T₁)`}, {`package p2; func f(type T)(T); var _ = f(42)`, `f(42)`, `()`}, } diff --git a/src/go/types/contracts.go b/src/go/types/contracts.go index d99c87407a..1dae945e76 100644 --- a/src/go/types/contracts.go +++ b/src/go/types/contracts.go @@ -135,22 +135,24 @@ func (check *Checker) contractDecl(obj *Contract, cdecl *ast.ContractSpec) { continue } - // Add the instantiated bounds (ebound) as embedded interfaces to the respective - // embedding (outer) contract bound. Because the ebounds are already instantiated + // Add the instantiated bounds (tpar.bound) as embedded interfaces (embed) to the + // respective embedding (outer) contract bound. Because embeds are already instantiated // with the outer bound's type parameters, and because they are embedded, there // is no need to keep them in instantiated form; in fact it will lead to problems - // if the outer bound is instantiated again later. We can just keep the ebound's - // underlying interface instead. - // (Alternatively one could set ebound.tparam to nil, thus marking it as not - // parameterized. But because ebound may be shared one would have to make a - // copy first. Since interface embedding doesn't care whether the embedded - // interface is named or not - one may embed an interface alias, after all - - // it is simpler to just use the underlying unnamed interface.) - for i, ebound := range eobj.Bounds { - index := targs[i].(*TypeParam).index - iface := bounds[index].underlying.(*Interface) - iface.embeddeds = append(iface.embeddeds, ebound.underlying) // don't use Named form of ebound + // if the outer bound is instantiated again later. We can just keep the embeds + // instead. + for _, targ := range targs { + tpar := targ.(*TypeParam) + iface := bounds[tpar.index].underlying.(*Interface) + embed := tpar.Interface() // don't use Named form of tpar.bound + iface.embeddeds = append(iface.embeddeds, embed) check.posMap[iface] = append(check.posMap[iface], econtr.Pos()) // satisfy completeInterface requirements + // check.contractExpr assigned a type bound to its incoming type arguments, + // but these are also the type parameters of the embedding (outer) contract. + // Strip those bounds again (now that we have embedded them) otherwise the + // embedding contract's incoming parameters have type bounds associated + // with them. + tpar.bound = &emptyInterface } continue // success } diff --git a/src/go/types/examples/contracts.go2 b/src/go/types/examples/contracts.go2 index a3cd4769a1..f82b4a0074 100644 --- a/src/go/types/examples/contracts.go2 +++ b/src/go/types/examples/contracts.go2 @@ -106,8 +106,6 @@ contract compareTwo(A, B) { comparable(B) } -// This does not work at the moment due to a contract embedding bug. -// TODO(gri) fix this -// func cmp3(type T1, T2 compareTwo)(x1, y1 T1, x2, y2 T2) bool { -// return x1 == y1 || x2 != y2 -// } +func cmp3(type T1, T2 compareTwo)(x1, y1 T1, x2, y2 T2) bool { + return x1 == y1 || x2 != y2 +} diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 2123f15822..19f25850d1 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -4,48 +4,31 @@ package main -type C(type T) interface { - m() -} - -type W(type T) interface { - C(T) -} - -func _(type T W(T))(x T) { - // x.m() // TODO(gri) this doesn't work at the moment - embedding bug -} - -/* contract C(T) { - T m() + T m() T } +// contract C(T₁) {T₁ C₀(type T₁ any) = interface{m() T₁}} + contract W(T) { C(T) } -func _(type T W)(x T) { - x.m() -} -*/ +// contract W(T₂) {T₂ W₀(type T₂ main.C₀) = interface{interface{m() T₂}}} -/* -contract comparable(T) { - T foo() bool +func _(type T W)(x T) { + var _ T = x.m() } contract compareTwo(A, B) { comparable(A) - //comparable(B) + comparable(B) } func _(type T1, T2 compareTwo)(x1, y1 T1) bool { - // return x1 == y1 - return x1.foo() + return x1 == y1 } func _(type T comparable)(x, y T) bool { return x == y || x != y } -*/ \ No newline at end of file diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go index e6fbacd064..ef3c07cfbe 100644 --- a/src/go/types/typestring.go +++ b/src/go/types/typestring.go @@ -296,12 +296,25 @@ func writeTParamList(buf *bytes.Buffer, list []*TypeName, qf Qualifier, visited if i > 0 { buf.WriteString(", ") } - buf.WriteString(p.name) + + ptyp, _ := p.typ.(*TypeParam) + if ptyp != nil { + writeType(buf, ptyp, qf, visited) + } else { + 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) + if ptyp.Interface().Empty() { + // quick hack to declutter output + // TODO(gri) fix this per the TODO above + buf.WriteString("any") + } else { + writeType(buf, ptyp.bound, qf, visited) + } // TODO(gri) if this is a generic type bound, we should print // the type parameters }