go/types: de-parameterize instantiated contracts when embedding them

More precisely, de-parameterize the corresponding interfaces when
embedding them in the corresponding type bounds (interfaces) for
the embedding contract. This prevents the embedded (and already
instantiated) interfaces from being instantiated again when the
outer interface is instantiated and avoids breaking an assertion.

See also the test case in testdata/issues.go2 for more details.

Change-Id: I70c9354849eda0c8a36905d0b80f4d3031542f30

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)

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

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

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<K> rather than anInt<K>.
+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)
+}

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)()
 }