From 62683a7752cc3b9532707da6412c2c777a342d91 Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Tue, 31 Mar 2020 23:04:38 -0700
Subject: [PATCH] go/types: fix lookup of methods of pointers to type
 parameters

Since the type bound of a type parameter is an interface, no method
is found if the receiver is a pointer to a type parameter (pointers
to interfaces have no methods).

Ignore the indirection in this case.

Change-Id: Ie2af78b4cfb2f70b5d4f1d2afc631716a10ff7d9
---
 src/go/types/NOTES               |  1 +
 src/go/types/lookup.go           | 18 +++++++++++++++---
 src/go/types/testdata/issues.go2 | 31 +++++++++++++++++++++++++++++++
 src/go/types/testdata/tmp.go2    |  3 +--
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/src/go/types/NOTES b/src/go/types/NOTES
index b68c2cc650..10d5ce55ef 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
 
+- 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?
diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go
index 3fa8565cb5..da5369c617 100644
--- a/src/go/types/lookup.go
+++ b/src/go/types/lookup.go
@@ -82,7 +82,17 @@ func (check *Checker) rawLookupFieldOrMethod(T Type, addressable bool, pkg *Pack
 		return // blank fields/methods are never found
 	}
 
-	typ, isPtr := derefUnpack(T)
+	typ, isPtr := deref(T)
+
+	// If we have a type parameter, ignore isPtr otherwise we would
+	// return immediately below since the type parameter bound is an
+	// interface. This is needed for methods on variables that are
+	// pointers to values of type parameter type.
+	// TODO(gri) Should this be done in derefUnpack?
+	if tpar, _ := typ.(*TypeParam); tpar != nil {
+		typ = tpar.bound
+		isPtr = false
+	}
 
 	// *typ where typ is an interface has no methods.
 	if isPtr && IsInterface(typ) {
@@ -166,6 +176,9 @@ func (check *Checker) rawLookupFieldOrMethod(T Type, addressable bool, pkg *Pack
 					// this depth, f.typ appears multiple times at the next
 					// depth.
 					if obj == nil && f.embedded {
+						// TODO(gri) investigate derefUnpack here (see
+						// comment in the beginning on unpacking type
+						// parameters).
 						typ, isPtr := derefUnpack(f.typ)
 						// TODO(gri) optimization: ignore types that can't
 						// have fields or methods (only Named, Struct, and
@@ -409,8 +422,7 @@ func deref(typ Type) (Type, bool) {
 	return typ, false
 }
 
-// derefUnpack is like deref but it also unpacks type parameters
-// and parameterized types.
+// derefUnpack is like deref but it also unpacks type parameters.
 func derefUnpack(typ Type) (Type, bool) {
 	typ, ptr := deref(typ)
 	if tpar, _ := typ.(*TypeParam); tpar != nil {
diff --git a/src/go/types/testdata/issues.go2 b/src/go/types/testdata/issues.go2
index fc21520a0f..5f7fd4fde8 100644
--- a/src/go/types/testdata/issues.go2
+++ b/src/go/types/testdata/issues.go2
@@ -47,3 +47,34 @@ func _() {
 	var m map[int]int
 	Equal(m, nil)
 }
+
+// If we have a receiver of pointer type (below: *T) we must ignore
+// the pointer in the implementation of the method lookup because
+// the type bound of T is an interface an pointer to interface types
+// have no methods and then the lookup would fail.
+contract C(T) {
+    T m()
+}
+
+// using contract C
+func _(type T C)(x *T) {
+	x.m()
+}
+
+// using an interface as bound
+func _(type T interface{ m() })(x *T) {
+	x.m()
+}
+
+// This is the original (simplified) program causing the same issue.
+type GraphP(type Node, Edge GP) struct {
+	nodes []*Node
+}
+
+contract GP(Node, Edge) {
+	Node Edges() []*Edge
+}
+
+func (g *GraphP(Node, Edge)) Edges(n *Node) []*Edge {
+	return n.Edges()
+}
diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2
index 3f2775edb0..7e3d137d2b 100644
--- a/src/go/types/testdata/tmp.go2
+++ b/src/go/types/testdata/tmp.go2
@@ -4,13 +4,12 @@
 
 package p
 
-// TODO(gri) this should work
 contract C(T) {
     T m()
 }
 
 func _(type T C)(x *T) {
-	x.m /* ERROR no field or method m */ ()
+	x.m()
 }
 
 /*