go/types: fix satisfaction check of argument type against type bound

Per the draft design:

In a generic function body all method calls will be pointer method calls.
If necessary, the function body will insert temporary variables, not seen
by the user, in order to get an addressable variable to use to call the method.

When checking if a concrete type argument satisfies its type bound, assume
that concrete type is the type of an addressable value.

Change-Id: I9d40138755c38330448c010994b95a82f38e60e0

src/go/types/NOTES

@@ -4,9 +4,8 @@ so we have a better track record. I only switched to this file in Nov 2019, henc
 ----------------------------------------------------------------------------------------------------
 TODO
 
+- revisit uses of (raw)lookupFieldOrMethod with respect to the addressable flag (can we get rid of it?)
 - revisit all uses of derefUnpack in lookup.go (see local comments); create test cases
-- implement type-checking for type parameters with pointer designation in contracts
-- figure out how to translate methods with pointer designation into interface bounds
 - 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?)
@@ -27,6 +26,10 @@ KNOWN ISSUES
   type parameter list or enclosing contract
 - A type parameter that is constrained by multiple contracts will not get the correct type bound.
   We may disallow this for  now.
+- Type-checking for type parameters with pointer designation in contracts is not implemented.
+  Do we actually need it? (Should the draft be updated?)
+- If we do need type-checking for type parameters with pointer designation in contracts, figure
+  out how to model this with interfaces. Do we need two interfaces, one for a T and one for a *T?
 
 ----------------------------------------------------------------------------------------------------
 OBSERVATIONS

src/go/types/README

@@ -77,13 +77,13 @@ idea how to implement that but we can easily type-check it).
 
 MAJOR KNOWN ISSUES
 
-- various type-specific operations (such as sending a message, type
+- Various type-specific operations (such as sending a message, type
   assertions, etc.) on expressions of a generic type don't work yet
   (but are relatively easy to implement going forward)
-- error messages are reasonable but expect them to be significantly
-  better in a real implementation (the subscript numbers on type
-  parameters are there to visually identify different parameters
-  with the same name)
+- Error messages are reasonable but expect them to be significantly
+  better in a real implementation.
+- Type parameters with pointer designation in contracts are not yet
+  supported (e.g.: contract C(T) { *T m() } ).
 
 See also the NOTES file for a more up-to-date documentation of the
 current state and issues.

src/go/types/lookup.go

@@ -278,19 +278,21 @@ func (check *Checker) lookupType(m map[Type]int, typ Type) (int, bool) {
 // x is of interface type V).
 //
 func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType bool) {
-	m, typ := (*Checker)(nil).missingMethod(V, T, static)
+	m, typ := (*Checker)(nil).missingMethod(V, false, T, static)
 	return m, typ != nil
 }
 
-// missingMethod is like MissingMethod but accepts a receiver.
+// missingMethod is like MissingMethod but accepts a *Checker as
+// receiver and an addressable flag.
 // The receiver may be nil if missingMethod is invoked through
 // an exported API call (such as MissingMethod), i.e., when all
 // methods have been type-checked.
+// If addressable is set, V is the type of an addressable variable.
 // If the type has the correctly named method, but with the wrong
 // signature, the existing method is returned as well.
 // To improve error messages, also report the wrong signature
 // when the method exists on *V instead of V.
-func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method, wrongType *Func) {
+func (check *Checker) missingMethod(V Type, addressable bool, T *Interface, static bool) (method, wrongType *Func) {
 	check.completeInterface(token.NoPos, T)
 
 	// fast path for common case
@@ -334,10 +336,11 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool) (method,
 	}
 
 	// A concrete type implements T if it implements all methods of T.
-	Vd, _ := deref(V)
+	Vd, _ := deref(V) // TODO(gri) shouldn't "pointer-ness" flow into rawLookupFieldOrMethod below?
 	Vn, _ := Vd.(*Named)
 	for _, m := range T.allMethods {
-		obj, _, _ := check.rawLookupFieldOrMethod(V, false, m.pkg, m.name)
+		// TODO(gri) should this be calling lookupFieldOrMethod instead (and why not)?
+		obj, _, _ := check.rawLookupFieldOrMethod(V, addressable, m.pkg, m.name)
 
 		// Check if *V implements this method of T.
 		if obj == nil {
@@ -410,7 +413,7 @@ func (check *Checker) assertableTo(V *Interface, T Type, strict bool) (method, w
 	if _, ok := T.Underlying().(*Interface); ok && !(strict || forceStrict) {
 		return
 	}
-	return check.missingMethod(T, V, false)
+	return check.missingMethod(T, false, V, false)
 }
 
 // deref dereferences typ if it is a *Pointer and returns its base and true.

src/go/types/operand.go

@@ -267,7 +267,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) bool {
 
 	// T is an interface type and x implements T
 	if Ti, ok := Tu.(*Interface); ok {
-		if m, wrongType := check.missingMethod(V, Ti, true); m != nil /* Implements(V, Ti) */ {
+		if m, wrongType := check.missingMethod(V, false, Ti, true); m != nil /* Implements(V, Ti) */ {
 			if reason != nil {
 				if wrongType != nil {
 					if check.identical(m.typ, wrongType.typ) {

src/go/types/subst.go

@@ -132,10 +132,20 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist
 		iface = check.subst(pos, iface, smap).(*Interface)
 
 		// targ must implement iface (methods)
-		if m, _ := check.missingMethod(targ, iface, true); m != nil {
+		//
+		// Assume targ is addressable, per the draft design: "In a generic function
+		// body all method calls will be pointer method calls. If necessary, the
+		// function body will insert temporary variables, not seen by the user, in
+		// order to get an addressable variable to use to call the method."
+		//
+		// TODO(gri) Instead of the addressable (= true) flag, could we encode the
+		// same information by making targ a pointer type (and then get rid of the
+		// need for that extra flag)?
+		if m, _ := check.missingMethod(targ, true, iface, true); m != nil {
 			// TODO(gri) needs to print updated name to avoid major confusion in error message!
 			//           (print warning for now)
-			check.softErrorf(pos, "%s does not satisfy %s (warning: name not updated) = %s (missing method %s)", targ, tpar.bound, iface, m)
+			// check.softErrorf(pos, "%s does not satisfy %s (warning: name not updated) = %s (missing method %s)", targ, tpar.bound, iface, m)
+			check.softErrorf(pos, "%s does not satisfy %s (missing method %s)", targ, tpar.bound, m.name)
 			break
 		}
 

src/go/types/testdata/issues.go2

@@ -78,3 +78,32 @@ contract GP(Node, Edge) {
 func (g *GraphP(Node, Edge)) Edges(n *Node) []*Edge {
 	return n.Edges()
 }
+
+// In a generic function body all method calls will be pointer method calls.
+// If necessary, the function body will insert temporary variables, not seen
+// by the user, in order to get an addressable variable to use to call the method.
+// Thus, assume an argument type for a generic function to be the type of addressable
+// values in the generic function when checking if the argument type satisfies the
+// generic function's type bound.
+func f2(type _ interface{ m1(); m2() })()
+
+type T struct{}
+func (T) m1()
+func (*T) m2()
+
+func _() {
+	f2(T)()
+	f2(*T)()
+}
+
+// This is the original (simplified) program causing the same issue.
+func NewP(type Node, Edge GP)(nodes []*Node) *GraphP(Node, Edge) {
+	return &GraphP(Node, Edge){nodes: nodes}
+}
+
+type N struct{}
+func (n *N) Edges() []*E { return nil }
+type E struct{}
+func F() {
+	_ = NewP(N, E)(nil)
+}

src/go/types/testdata/tmp.go2

@@ -4,46 +4,11 @@
 
 package p
 
-contract C(T) {
-    T m()
-}
+func f(type _ interface{ m() })()
 
-func _(type T C)(x *T) {
-	x.m()
-}
-
-/*
-func f(type T)(T)
+type T struct{}
+func (*T) m()
 
 func _() {
-    var x int
-    f(x)
+	f(T)()
 }
-*/
-
-/*
-func f(func(int))
-func g(type T)(T)
-
-func _() {
-	f(g)
-}
-*/
-
-/*
-func f(type T)(T, func(type T)(T))
-func g(type T)(T)
-func _() {
-    var x int
-    f(x, g)
-}
-*/
-
-/*
-func f(type T)(T, func(T))
-func g(type T)(T)
-func _() {
-    var x int
-    f(x, g)
-}
-*/