go.tools/go/types: *interface types have no methods

Fixes golang/go#5918.

R=adonovan
CC=golang-dev
https://golang.org/cl/12909043

go/types/call.go

@@ -298,17 +298,22 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 				// Verify that LookupFieldOrMethod and MethodSet.Lookup agree.
 				typ := x.typ
 				if x.mode == variable {
-					// If typ is not an (unnamed) pointer, use *typ instead,
-					// because the method set of *typ includes the methods
-					// of typ.
+					// If typ is not an (unnamed) pointer or an interface,
+					// use *typ instead, because the method set of *typ
+					// includes the methods of typ.
 					// Variables are addressable, so we can always take their
 					// address.
-					if _, isPtr := typ.(*Pointer); !isPtr {
-						typ = &Pointer{base: typ}
+					if _, ok := typ.(*Pointer); !ok {
+						if _, ok := typ.Underlying().(*Interface); !ok {
+							typ = &Pointer{base: typ}
+						}
 					}
 				}
 				// If we created a synthetic pointer type above, we will throw
 				// away the method set computed here after use.
+				// TODO(gri) Method set computation should probably always compute
+				// both, the value and the pointer receiver method set and represent
+				// them in a single structure.
 				// TODO(gri) Consider also using a method set cache for the lifetime
 				// of checker once we rely on MethodSet lookup instead of individual
 				// lookup.

go/types/lookup.go

@@ -12,9 +12,9 @@ package types
 //           indirectly via different packages.)
 
 // LookupFieldOrMethod looks up a field or method with given package and name
-// in typ and returns the corresponding *Var or *Func, an index sequence,
-// and a bool indicating if there were any pointer indirections on the path
-// to the field or method.
+// in T and returns the corresponding *Var or *Func, an index sequence, and a
+// bool indicating if there were any pointer indirections on the path to the
+// field or method.
 //
 // The last index entry is the field or method index in the (possibly embedded)
 // type where the entry was found, either:
@@ -29,8 +29,8 @@ package types
 // If no entry is found, a nil object is returned. In this case, the returned
 // index sequence points to an ambiguous entry if it exists, or it is nil.
 //
-func LookupFieldOrMethod(typ Type, pkg *Package, name string) (obj Object, index []int, indirect bool) {
-	obj, index, indirect = lookupFieldOrMethod(typ, pkg, name)
+func LookupFieldOrMethod(T Type, pkg *Package, name string) (obj Object, index []int, indirect bool) {
+	obj, index, indirect = lookupFieldOrMethod(T, pkg, name)
 	if obj != nil {
 		return
 	}
@@ -54,7 +54,7 @@ func LookupFieldOrMethod(typ Type, pkg *Package, name string) (obj Object, index
 	// TODO(gri) WTF? There isn't a more direct way? Perhaps we should
 	//           outlaw named types to pointer types - they are almost
 	//           never what one wants, anyway.
-	if t, _ := typ.(*Named); t != nil {
+	if t, _ := T.(*Named); t != nil {
 		u := t.underlying
 		if _, ok := u.(*Pointer); ok {
 			// typ is a named type with an underlying type of the form *T,
@@ -71,18 +71,31 @@ func LookupFieldOrMethod(typ Type, pkg *Package, name string) (obj Object, index
 	return
 }
 
-func lookupFieldOrMethod(typ Type, pkg *Package, name string) (obj Object, index []int, indirect bool) {
+func lookupFieldOrMethod(T Type, pkg *Package, name string) (obj Object, index []int, indirect bool) {
 	// WARNING: The code in this function is extremely subtle - do not modify casually!
+	//          This function and NewMethodSet should kept in sync.
 
 	if name == "_" {
 		return // blank fields/methods are never found
 	}
 
+	typ, isPtr := deref(T)
+	named, _ := typ.(*Named)
+
+	// *typ where typ is an interface has no methods.
+	if isPtr {
+		utyp := typ
+		if named != nil {
+			utyp = named.underlying
+		}
+		if _, ok := utyp.(*Interface); ok {
+			return
+		}
+	}
+
 	// Start with typ as single entry at shallowest depth.
 	// If typ is not a named type, insert a nil type instead.
-	typ, isPtr := deref(typ)
-	t, _ := typ.(*Named)
-	current := []embeddedType{{t, nil, isPtr, false}}
+	current := []embeddedType{{named, nil, isPtr, false}}
 
 	// named types that we have seen already, allocated lazily
 	var seen map[*Named]bool

go/types/methodset.go

@@ -28,7 +28,7 @@ func (s *MethodSet) String() string {
 	var buf bytes.Buffer
 	fmt.Fprintln(&buf, "MethodSet {")
 	for _, f := range s.list {
-		fmt.Fprintln(&buf, f)
+		fmt.Fprintf(&buf, "\t%s\n", f)
 	}
 	fmt.Fprintln(&buf, "}")
 	return buf.String()
@@ -90,15 +90,28 @@ func (c *cachedMethodSet) of(typ Type) *MethodSet {
 // It always returns a non-nil method set, even if it is empty.
 func NewMethodSet(T Type) *MethodSet {
 	// WARNING: The code in this function is extremely subtle - do not modify casually!
+	//          This function and lookupFieldOrMethod should be kept in sync.
 
 	// method set up to the current depth, allocated lazily
 	var base methodSet
 
+	typ, isPtr := deref(T)
+	named, _ := typ.(*Named)
+
+	// *typ where typ is an interface has no methods.
+	if isPtr {
+		utyp := typ
+		if named != nil {
+			utyp = named.underlying
+		}
+		if _, ok := utyp.(*Interface); ok {
+			return &emptyMethodSet
+		}
+	}
+
 	// Start with typ as single entry at shallowest depth.
 	// If typ is not a named type, insert a nil type instead.
-	typ, isPtr := deref(T)
-	t, _ := typ.(*Named)
-	current := []embeddedType{{t, nil, isPtr, false}}
+	current := []embeddedType{{named, nil, isPtr, false}}
 
 	// named types that we have seen already, allocated lazily
 	var seen map[*Named]bool

go/types/testdata/methodsets.src

@@ -188,3 +188,29 @@ func _() {
 	(&T3{}).v2()
 	(&T3{}).p2()
 }
+
+// *T has no methods if T is an interface type
+func issue5918() {
+	var (
+		err error
+		_ = err.Error()
+		_ func() string = err.Error
+		_ func(error) string = error.Error
+
+		perr = &err
+		_ = perr /* ERROR "no field or method" */ .Error()
+		_ func() string = perr /* ERROR "no field or method" */ .Error
+		_ func(*error) string = ( /* ERROR "no field or method" */ *error).Error
+	)
+
+	type T *interface{ m() int }
+	var (
+		x T
+		_ = (*x).m()
+		_ = (*x).m
+
+		_ = x /* ERROR "no field or method" */ .m()
+		_ = x /* ERROR "no field or method" */ .m
+		_ = T /* ERROR "no field or method" */ .m
+	)
+}

ssa/print.go

@@ -389,8 +389,8 @@ func (p *Package) DumpTo(w io.Writer) {
 			// Iterate over the keys of mset(*T) since they
 			// are a superset of mset(T)'s keys.
 			// The keys of a types.MethodSet are sorted (by Id).
-			mset := methodSetOf(mem.Type())
-			pmset := methodSetOf(types.NewPointer(mem.Type()))
+			mset := mem.Type().MethodSet()
+			pmset := types.NewPointer(mem.Type()).MethodSet()
 			for i, n := 0, pmset.Len(); i < n; i++ {
 				meth := pmset.At(i)
 				// If the method also exists in mset(T), show that instead.

ssa/promote.go

@@ -22,16 +22,6 @@ func recvType(obj *types.Func) types.Type {
 	return obj.Type().(*types.Signature).Recv().Type()
 }
 
-func methodSetOf(typ types.Type) *types.MethodSet {
-	// TODO(adonovan): temporary workaround.  Inline it away when fixed.
-	if _, ok := deref(typ).Underlying().(*types.Interface); ok && isPointer(typ) {
-		// TODO(gri): fix: go/types bug: pointer-to-interface
-		// has no methods---yet go/types says it has!
-		return new(types.MethodSet)
-	}
-	return typ.MethodSet()
-}
-
 // Method returns the Function implementing method meth, building
 // wrapper methods on demand.
 //

ssa/source.go

@@ -104,7 +104,7 @@ func findNamedFunc(pkg *Package, pos token.Pos) *Function {
 				return mem
 			}
 		case *Type:
-			mset := methodSetOf(types.NewPointer(mem.Type()))
+			mset := types.NewPointer(mem.Type()).MethodSet()
 			for i, n := 0, mset.Len(); i < n; i++ {
 				// Don't call Program.Method: avoid creating wrappers.
 				obj := mset.At(i).Obj().(*types.Func)
@@ -188,7 +188,7 @@ func (prog *Program) FuncValue(obj *types.Func) Value {
 		return v
 	}
 	// Interface method wrapper?
-	meth := methodSetOf(recvType(obj)).Lookup(obj.Pkg(), obj.Name())
+	meth := recvType(obj).MethodSet().Lookup(obj.Pkg(), obj.Name())
 	return prog.Method(meth)
 }
 

ssa/visit.go

@@ -45,7 +45,7 @@ func (visit *visitor) program() {
 }
 
 func (visit *visitor) methodSet(typ types.Type) {
-	mset := methodSetOf(typ)
+	mset := typ.MethodSet()
 	for i, n := 0, mset.Len(); i < n; i++ {
 		// Side-effect: creates all wrapper methods.
 		visit.function(visit.prog.Method(mset.At(i)))