]> Cypherpunks repositories - gostls13.git/commitdiff
go/importer: use named receiver types for methods of named interfaces
authorRobert Griesemer <gri@golang.org>
Fri, 22 Dec 2017 23:04:20 +0000 (15:04 -0800)
committerRobert Griesemer <gri@golang.org>
Mon, 12 Feb 2018 21:43:04 +0000 (21:43 +0000)
R=go1.11

Once approved, this change must be ported to golang.org/x/tools/go/gcimporter15.

Fixes #13829.

Change-Id: I26a0094d2bfd38b97f2b64bae84b9f428fc9cdf1
Reviewed-on: https://go-review.googlesource.com/85318
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/internal/gcimporter/bimport.go
src/go/internal/gcimporter/gcimporter_test.go
src/go/internal/srcimporter/srcimporter_test.go
src/go/types/type.go

index 23c1d2f76a9a2db41897186cafd8ed9a2800fe69..b8d9e318ed165bb2bda5103f900eba49e1cb6b05 100644 (file)
@@ -257,7 +257,7 @@ func (p *importer) obj(tag int) {
        case constTag:
                pos := p.pos()
                pkg, name := p.qualifiedName()
-               typ := p.typ(nil)
+               typ := p.typ(nil, nil)
                val := p.value()
                p.declare(types.NewConst(pos, pkg, name, typ, val))
 
@@ -265,16 +265,16 @@ func (p *importer) obj(tag int) {
                // TODO(gri) verify type alias hookup is correct
                pos := p.pos()
                pkg, name := p.qualifiedName()
-               typ := p.typ(nil)
+               typ := p.typ(nil, nil)
                p.declare(types.NewTypeName(pos, pkg, name, typ))
 
        case typeTag:
-               p.typ(nil)
+               p.typ(nil, nil)
 
        case varTag:
                pos := p.pos()
                pkg, name := p.qualifiedName()
-               typ := p.typ(nil)
+               typ := p.typ(nil, nil)
                p.declare(types.NewVar(pos, pkg, name, typ))
 
        case funcTag:
@@ -379,7 +379,11 @@ func (t *dddSlice) String() string         { return "..." + t.elem.String() }
 // the package currently imported. The parent package is needed for
 // exported struct fields and interface methods which don't contain
 // explicit package information in the export data.
-func (p *importer) typ(parent *types.Package) types.Type {
+//
+// A non-nil tname is used as the "owner" of the result type; i.e.,
+// the result type is the underlying type of tname. tname is used
+// to give interface methods a named receiver type where possible.
+func (p *importer) typ(parent *types.Package, tname *types.Named) types.Type {
        // if the type was seen before, i is its index (>= 0)
        i := p.tagOrIndex()
        if i >= 0 {
@@ -409,15 +413,15 @@ func (p *importer) typ(parent *types.Package) types.Type {
                t0 := types.NewNamed(obj.(*types.TypeName), nil, nil)
 
                // but record the existing type, if any
-               t := obj.Type().(*types.Named)
-               p.record(t)
+               tname := obj.Type().(*types.Named) // tname is either t0 or the existing type
+               p.record(tname)
 
                // read underlying type
-               t0.SetUnderlying(p.typ(parent))
+               t0.SetUnderlying(p.typ(parent, t0))
 
                // interfaces don't have associated methods
                if types.IsInterface(t0) {
-                       return t
+                       return tname
                }
 
                // read associated methods
@@ -438,7 +442,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                        t0.AddMethod(types.NewFunc(pos, parent, name, sig))
                }
 
-               return t
+               return tname
 
        case arrayTag:
                t := new(types.Array)
@@ -447,7 +451,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                }
 
                n := p.int64()
-               *t = *types.NewArray(p.typ(parent), n)
+               *t = *types.NewArray(p.typ(parent, nil), n)
                return t
 
        case sliceTag:
@@ -456,7 +460,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                        p.record(t)
                }
 
-               *t = *types.NewSlice(p.typ(parent))
+               *t = *types.NewSlice(p.typ(parent, nil))
                return t
 
        case dddTag:
@@ -465,7 +469,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                        p.record(t)
                }
 
-               t.elem = p.typ(parent)
+               t.elem = p.typ(parent, nil)
                return t
 
        case structTag:
@@ -483,7 +487,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                        p.record(t)
                }
 
-               *t = *types.NewPointer(p.typ(parent))
+               *t = *types.NewPointer(p.typ(parent, nil))
                return t
 
        case signatureTag:
@@ -502,6 +506,8 @@ func (p *importer) typ(parent *types.Package) types.Type {
                // cannot expect the interface type to appear in a cycle, as any
                // such cycle must contain a named type which would have been
                // first defined earlier.
+               // TODO(gri) Is this still true now that we have type aliases?
+               // See issue #23225.
                n := len(p.typList)
                if p.trackAllTypes {
                        p.record(nil)
@@ -510,10 +516,10 @@ func (p *importer) typ(parent *types.Package) types.Type {
                var embeddeds []*types.Named
                for n := p.int(); n > 0; n-- {
                        p.pos()
-                       embeddeds = append(embeddeds, p.typ(parent).(*types.Named))
+                       embeddeds = append(embeddeds, p.typ(parent, nil).(*types.Named))
                }
 
-               t := types.NewInterface(p.methodList(parent), embeddeds)
+               t := types.NewInterface(p.methodList(parent, tname), embeddeds)
                p.interfaceList = append(p.interfaceList, t)
                if p.trackAllTypes {
                        p.typList[n] = t
@@ -526,8 +532,8 @@ func (p *importer) typ(parent *types.Package) types.Type {
                        p.record(t)
                }
 
-               key := p.typ(parent)
-               val := p.typ(parent)
+               key := p.typ(parent, nil)
+               val := p.typ(parent, nil)
                *t = *types.NewMap(key, val)
                return t
 
@@ -549,7 +555,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                default:
                        errorf("unexpected channel dir %d", d)
                }
-               val := p.typ(parent)
+               val := p.typ(parent, nil)
                *t = *types.NewChan(dir, val)
                return t
 
@@ -573,7 +579,7 @@ func (p *importer) fieldList(parent *types.Package) (fields []*types.Var, tags [
 func (p *importer) field(parent *types.Package) (*types.Var, string) {
        pos := p.pos()
        pkg, name, alias := p.fieldName(parent)
-       typ := p.typ(parent)
+       typ := p.typ(parent, nil)
        tag := p.string()
 
        anonymous := false
@@ -597,22 +603,30 @@ func (p *importer) field(parent *types.Package) (*types.Var, string) {
        return types.NewField(pos, pkg, name, typ, anonymous), tag
 }
 
-func (p *importer) methodList(parent *types.Package) (methods []*types.Func) {
+func (p *importer) methodList(parent *types.Package, baseType *types.Named) (methods []*types.Func) {
        if n := p.int(); n > 0 {
                methods = make([]*types.Func, n)
                for i := range methods {
-                       methods[i] = p.method(parent)
+                       methods[i] = p.method(parent, baseType)
                }
        }
        return
 }
 
-func (p *importer) method(parent *types.Package) *types.Func {
+func (p *importer) method(parent *types.Package, baseType *types.Named) *types.Func {
        pos := p.pos()
        pkg, name, _ := p.fieldName(parent)
+       // If we don't have a baseType, use a nil receiver.
+       // A receiver using the actual interface type (which
+       // we don't know yet) will be filled in when we call
+       // types.Interface.Complete.
+       var recv *types.Var
+       if baseType != nil {
+               recv = types.NewVar(token.NoPos, parent, "", baseType)
+       }
        params, isddd := p.paramList()
        result, _ := p.paramList()
-       sig := types.NewSignature(nil, params, result, isddd)
+       sig := types.NewSignature(recv, params, result, isddd)
        return types.NewFunc(pos, pkg, name, sig)
 }
 
@@ -668,7 +682,7 @@ func (p *importer) paramList() (*types.Tuple, bool) {
 }
 
 func (p *importer) param(named bool) (*types.Var, bool) {
-       t := p.typ(nil)
+       t := p.typ(nil, nil)
        td, isddd := t.(*dddSlice)
        if isddd {
                t = types.NewSlice(td.elem)
index 56870a14121a16e1c10cd3c3351c3db4565d4d15..63abf97e7e834e1f5211b2851f841cb10d1acaaa 100644 (file)
@@ -200,11 +200,22 @@ var importedObjectTests = []struct {
        name string
        want string
 }{
+       // non-interfaces
+       {"crypto.Hash", "type Hash uint"},
+       {"go/ast.ObjKind", "type ObjKind int"},
+       {"go/types.Qualifier", "type Qualifier func(*Package) string"},
+       {"go/types.Comparable", "func Comparable(T Type) bool"},
        {"math.Pi", "const Pi untyped float"},
+       {"math.Sin", "func Sin(x float64) float64"},
+
+       // interfaces
+       {"context.Context", "type Context interface{Deadline() (deadline time.Time, ok bool); Done() <-chan struct{}; Err() error; Value(key interface{}) interface{}}"},
+       {"crypto.Decrypter", "type Decrypter interface{Decrypt(rand io.Reader, msg []byte, opts DecrypterOpts) (plaintext []byte, err error); Public() PublicKey}"},
+       {"encoding.BinaryMarshaler", "type BinaryMarshaler interface{MarshalBinary() (data []byte, err error)}"},
        {"io.Reader", "type Reader interface{Read(p []byte) (n int, err error)}"},
        {"io.ReadWriter", "type ReadWriter interface{Reader; Writer}"},
-       {"math.Sin", "func Sin(x float64) float64"},
-       // TODO(gri) add more tests
+       {"go/ast.Node", "type Node interface{End() go/token.Pos; Pos() go/token.Pos}"},
+       {"go/types.Type", "type Type interface{String() string; Underlying() Type}"},
 }
 
 func TestImportedTypes(t *testing.T) {
@@ -239,6 +250,44 @@ func TestImportedTypes(t *testing.T) {
                if got != test.want {
                        t.Errorf("%s: got %q; want %q", test.name, got, test.want)
                }
+
+               if named, _ := obj.Type().(*types.Named); named != nil {
+                       verifyInterfaceMethodRecvs(t, named, 0)
+               }
+       }
+}
+
+// verifyInterfaceMethodRecvs verifies that method receiver types
+// are named if the methods belong to a named interface type.
+func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
+       // avoid endless recursion in case of an embedding bug that lead to a cycle
+       if level > 10 {
+               t.Errorf("%s: embeds itself", named)
+               return
+       }
+
+       iface, _ := named.Underlying().(*types.Interface)
+       if iface == nil {
+               return // not an interface
+       }
+
+       // check explicitly declared methods
+       for i := 0; i < iface.NumExplicitMethods(); i++ {
+               m := iface.ExplicitMethod(i)
+               recv := m.Type().(*types.Signature).Recv()
+               if recv == nil {
+                       t.Errorf("%s: missing receiver type", m)
+                       continue
+               }
+               if recv.Type() != named {
+                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), named)
+               }
+       }
+
+       // check embedded interfaces (they are named, too)
+       for i := 0; i < iface.NumEmbeddeds(); i++ {
+               // embedding of interfaces cannot have cycles; recursion will terminate
+               verifyInterfaceMethodRecvs(t, iface.Embedded(i), level+1)
        }
 }
 
index 356e71d12873a78ce3ce06ccfa016e7c05aca994..7310aa7d3f0b9abb107ad63d4bf908090411d345 100644 (file)
@@ -130,6 +130,44 @@ func TestImportedTypes(t *testing.T) {
                if got != test.want {
                        t.Errorf("%s: got %q; want %q", test.name, got, test.want)
                }
+
+               if named, _ := obj.Type().(*types.Named); named != nil {
+                       verifyInterfaceMethodRecvs(t, named, 0)
+               }
+       }
+}
+
+// verifyInterfaceMethodRecvs verifies that method receiver types
+// are named if the methods belong to a named interface type.
+func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
+       // avoid endless recursion in case of an embedding bug that lead to a cycle
+       if level > 10 {
+               t.Errorf("%s: embeds itself", named)
+               return
+       }
+
+       iface, _ := named.Underlying().(*types.Interface)
+       if iface == nil {
+               return // not an interface
+       }
+
+       // check explicitly declared methods
+       for i := 0; i < iface.NumExplicitMethods(); i++ {
+               m := iface.ExplicitMethod(i)
+               recv := m.Type().(*types.Signature).Recv()
+               if recv == nil {
+                       t.Errorf("%s: missing receiver type", m)
+                       continue
+               }
+               if recv.Type() != named {
+                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), named)
+               }
+       }
+
+       // check embedded interfaces (they are named, too)
+       for i := 0; i < iface.NumEmbeddeds(); i++ {
+               // embedding of interfaces cannot have cycles; recursion will terminate
+               verifyInterfaceMethodRecvs(t, iface.Embedded(i), level+1)
        }
 }
 
index a58684a535fd4604d4dc4be0040d40452799ef4a..374966c4ed96e5a7dba2d45f5aa238b2d836122f 100644 (file)
@@ -254,7 +254,8 @@ var emptyInterface = Interface{allMethods: markComplete}
 var markComplete = make([]*Func, 0)
 
 // NewInterface returns a new (incomplete) interface for the given methods and embedded types.
-// To compute the method set of the interface, Complete must be called.
+// NewInterface takes ownership of the provided methods and may modify their types by setting
+// missing receivers. To compute the method set of the interface, Complete must be called.
 func NewInterface(methods []*Func, embeddeds []*Named) *Interface {
        typ := new(Interface)
 
@@ -267,10 +268,10 @@ func NewInterface(methods []*Func, embeddeds []*Named) *Interface {
                if mset.insert(m) != nil {
                        panic("multiple methods with the same name")
                }
-               // set receiver
-               // TODO(gri) Ideally, we should use a named type here instead of
-               // typ, for less verbose printing of interface method signatures.
-               m.typ.(*Signature).recv = NewVar(m.pos, m.pkg, "", typ)
+               // set receiver if we don't have one
+               if sig := m.typ.(*Signature); sig.recv == nil {
+                       sig.recv = NewVar(m.pos, m.pkg, "", typ)
+               }
        }
        sort.Sort(byUniqueMethodName(methods))