diff --git a/src/go/types/NOTES b/src/go/types/NOTES index ecb80d62e5..387550846b 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -4,7 +4,8 @@ so we have a better track record. I only switched to this file in Nov 2019, henc ---------------------------------------------------------------------------------------------------- TODO -- review embedding of contracts with multiple and different numbers of arguments - likely incorrect +- 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 - improve error messages! - use []*TypeParam for tparams in subst? (unclear) diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index eb894aa234..f8045e4615 100644 --- a/src/go/types/check_test.go +++ b/src/go/types/check_test.go @@ -105,6 +105,7 @@ var tests = [][]string{ {"testdata/typeinst.go2"}, {"testdata/typeinst2.go2"}, {"testdata/contracts.go2"}, + {"testdata/issues.go2"}, {"testdata/todos.go2"}, // Go 2 examples from design doc diff --git a/src/go/types/contracts.go b/src/go/types/contracts.go index 818ca05484..d99c87407a 100644 --- a/src/go/types/contracts.go +++ b/src/go/types/contracts.go @@ -135,15 +135,21 @@ func (check *Checker) contractDecl(obj *Contract, cdecl *ast.ContractSpec) { continue } - // add the instantiated bounds as embedded interfaces to the respective - // embedding (outer) contract bound - // TODO(gri) This seems incorrect. We must accomodate the situation where - // the embedded interfaces have different type parameters than - // the embedding interfaces. + // Add the instantiated bounds (ebound) as embedded interfaces to the respective + // embedding (outer) contract bound. Because the ebounds 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) + iface.embeddeds = append(iface.embeddeds, ebound.underlying) // don't use Named form of ebound check.posMap[iface] = append(check.posMap[iface], econtr.Pos()) // satisfy completeInterface requirements } continue // success diff --git a/src/go/types/testdata/issues.go2 b/src/go/types/testdata/issues.go2 new file mode 100644 index 0000000000..fc21520a0f --- /dev/null +++ b/src/go/types/testdata/issues.go2 @@ -0,0 +1,49 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains regression tests for bugs found. + +package p + +// This used to crash with an assertion failure when +// instantiating f(int, int). The assertion was checking +// that the number of type parameters and arguments was +// matching for the embedded contract anInt (rather, its +// corresponding interface bound) but it really compared +// the numbers for anInt and twoInt (which embedds anInt). +// The fix simply uses the instantiated non-parameterized +// underlying interface of atInt rather than anInt. +contract anInt(T) { + T int +} + +contract twoInt(K, _) { + anInt(K) +} + +func f(type K, V twoInt)() + +func _ () { + f(int, int)() +} + +// This is the original (simplified) program causing the same issue. +contract comparable(T) { + T int, int8, int16, int32, int64, + uint, uint8, uint16, uint32, uint64, uintptr, + float32, float64, + string +} + +contract twocomparable(K, V) { + comparable(K) + comparable(V) +} + +func Equal(type K, V twocomparable)(m1, m2 map[K]V) + +func _() { + var m map[int]int + Equal(m, nil) +} diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 261cf2cb52..4d9fe75062 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -15,5 +15,5 @@ contract twoInt(K, _) { func f(type K, V twoInt)() func _ () { - // f(int, int)() // this fails at the moment + f(int, int)() } \ No newline at end of file