From 5504084423489752f11d65d45100ae90c4eb7e96 Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Mon, 9 Dec 2019 17:21:02 -0800
Subject: [PATCH] go/types: implement type bound checking for type parameter
 type args

If the type argument is itself a type parameter, we must use that
type parameter's bound to check against the bound of the type
parameter matching the type argument.

Change-Id: If5128115a9fc10af8163c37b1f75645447ead766
---
 src/go/types/NOTES                  | 12 ------
 src/go/types/subst.go               | 63 ++++++++++++++++-------------
 src/go/types/testdata/contracts.go2 | 12 ++++++
 src/go/types/type.go                |  5 ++-
 4 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/src/go/types/NOTES b/src/go/types/NOTES
index 24de144410..3be20066a1 100644
--- a/src/go/types/NOTES
+++ b/src/go/types/NOTES
@@ -1,5 +1,4 @@
 TODO
-- satisfyBounds is not working correctly
 - allow recursive type parameterization without need to repeat type parameters
 - if type parameters are repeated in recursive instantiation, they must be the same order
 - implement contract embedding
@@ -8,17 +7,6 @@ TODO
 
 
 OPEN ISSUES
-- incorrect constraint errors:
-
-package p
-
-contract B(T) {
-	T int
-}
-
-type T(type P B) P
-
-func f(type P B)(x T(P)) // this should be valid
 
 
 DESIGN DECISIONS
diff --git a/src/go/types/subst.go b/src/go/types/subst.go
index aa7ec979d6..546d18c9a9 100644
--- a/src/go/types/subst.go
+++ b/src/go/types/subst.go
@@ -52,9 +52,17 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist
 
 	// check bounds
 	for i, tname := range tparams {
+		targ := targs[i]
+		if _, ok := targ.Underlying().(*Contract); ok {
+			panic("contract provided as type argument")
+		}
+
 		tpar := tname.typ.(*TypeParam)
+		// TODO(gri) decide if we want to keep this or standardize on the empty interface
+		//           (keeping it may make sense because it's a common scenario and it may
+		//           be more efficient to check)
 		if tpar.bound == nil {
-			continue // no bound
+			continue // no bound (optimization)
 		}
 
 		// best position for error reporting
@@ -64,45 +72,36 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist
 		}
 
 		// determine type parameter bound
-		var iface *Interface
-		switch bound := tpar.bound.Underlying().(type) {
-		case *Interface:
-			iface = bound
-		case *Contract:
-			iface = bound.ifaceAt(i)
-		default:
-			unreachable()
-		}
+		iface := tpar.Interface()
 
-		// use interface type of type parameter, if any
-		// targ must implement iface
-		targ := targs[i]
+		// targ must implement iface (methods)
 		if m, _ := check.missingMethod(targ, iface, true); m != nil {
 			check.softErrorf(pos, "%s does not satisfy %s (missing method %s)", targ, tpar.bound, m)
 			break
 		}
 
 		// targ's underlying type must also be one of the interface types listed, if any
-		if len(iface.types) > 0 {
-			utyp := targ.Underlying()
+		if len(iface.types) == 0 {
+			break // nothing to do
+		}
 
-			// TODO(gri) Cannot handle a type argument that is itself parameterized for now
-			switch utyp.(type) {
-			case *Interface, *Contract:
-				panic("unimplemented")
-			}
-
-			// TODO(gri) factor this out as sep. function?
-			ok := false
+		// if targ is a type parameter, the list of iface types must be a subset of the
+		// list of targ types
+		if t, _ := targ.Underlying().(*TypeParam); t != nil {
+			targFace := t.Interface()
 			for _, t := range iface.types {
-				// if we find one matching type, we're ok
-				if Identical(utyp, t) {
-					ok = true
+				if !includesType(t, targFace) {
+					check.softErrorf(pos, "%s does not satisfy %s (missing type %s)", targ, tpar.bound, t)
 					break
 				}
 			}
+			break
+		}
 
-			if !ok {
+		// otherwise, targ's underlying type must also be one of the interface types listed, if any
+		if len(iface.types) > 0 {
+			// TODO(gri) must it be the underlying type, or should it just be the type? (spec question)
+			if !includesType(targ.Underlying(), iface) {
 				check.softErrorf(pos, "%s does not satisfy %s (%s not found in %s)", targ, tpar.bound, targ, iface)
 				break
 			}
@@ -112,6 +111,16 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist
 	return check.subst(pos, typ, tparams, targs)
 }
 
+// includesType reports whether iface includes typ
+func includesType(typ Type, iface *Interface) bool {
+	for _, t := range iface.types {
+		if Identical(typ.Underlying(), t) {
+			return true
+		}
+	}
+	return false
+}
+
 // subst returns the type typ with its type parameters tparams replaced by
 // the corresponding type arguments targs, recursively.
 func (check *Checker) subst(pos token.Pos, typ Type, tpars []*TypeName, targs []Type) Type {
diff --git a/src/go/types/testdata/contracts.go2 b/src/go/types/testdata/contracts.go2
index 08348a7d6b..6ea2a45040 100644
--- a/src/go/types/testdata/contracts.go2
+++ b/src/go/types/testdata/contracts.go2
@@ -197,3 +197,15 @@ func _() {
         f1 /* ERROR float64 not found */ (1.2)
         f1(42)
 }
+
+type T3(type P B1) P
+
+func _(type P B1)(x T3(P))
+
+contract B2(T) {
+	T int
+}
+
+type T4(type P B2) P
+
+func _(type P B2)(x T4(P))
diff --git a/src/go/types/type.go b/src/go/types/type.go
index c8b91028a0..e8aa697b9e 100644
--- a/src/go/types/type.go
+++ b/src/go/types/type.go
@@ -558,9 +558,10 @@ func (check *Checker) NewTypeParam(obj *TypeName, index int, bound Type) *TypePa
 // the result is the empty interface.
 // TODO(gri) should this be Underlying instead?
 func (t *TypeParam) Interface() *Interface {
-	switch b := t.bound.Underlying().(type) {
-	case nil:
+	if t.bound == nil {
 		return &emptyInterface
+	}
+	switch b := t.bound.Underlying().(type) {
 	case *Interface:
 		return b
 	case *Contract: