From ae874abc7a72a327ef87016e8c10edacbe4ab1d0 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 14 Aug 2013 11:02:10 -0700 Subject: [PATCH] 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 | 15 ++++++++++----- go/types/lookup.go | 33 ++++++++++++++++++++++---------- go/types/methodset.go | 21 ++++++++++++++++---- go/types/testdata/methodsets.src | 26 +++++++++++++++++++++++++ ssa/print.go | 4 ++-- ssa/promote.go | 10 ---------- ssa/source.go | 4 ++-- ssa/visit.go | 2 +- 8 files changed, 81 insertions(+), 34 deletions(-) diff --git a/go/types/call.go b/go/types/call.go index b863ebb308..4263034cc4 100644 --- a/go/types/call.go +++ b/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. diff --git a/go/types/lookup.go b/go/types/lookup.go index 048ac47704..db09858719 100644 --- a/go/types/lookup.go +++ b/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 diff --git a/go/types/methodset.go b/go/types/methodset.go index 11986207c6..806cd15a4b 100644 --- a/go/types/methodset.go +++ b/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 diff --git a/go/types/testdata/methodsets.src b/go/types/testdata/methodsets.src index c12e7cd1b9..60988bbcca 100644 --- a/go/types/testdata/methodsets.src +++ b/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 + ) +} diff --git a/ssa/print.go b/ssa/print.go index 6bdc1b5cfb..9c82b5d81c 100644 --- a/ssa/print.go +++ b/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. diff --git a/ssa/promote.go b/ssa/promote.go index 1d26b8b9df..b050177737 100644 --- a/ssa/promote.go +++ b/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. // diff --git a/ssa/source.go b/ssa/source.go index c74f7c756a..60ea25256d 100644 --- a/ssa/source.go +++ b/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) } diff --git a/ssa/visit.go b/ssa/visit.go index 3679ac70d6..5437c57eb2 100644 --- a/ssa/visit.go +++ b/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)))