diff --git a/src/go/internal/gcimporter/bimport.go b/src/go/internal/gcimporter/bimport.go index 23c1d2f76a..b8d9e318ed 100644 --- a/src/go/internal/gcimporter/bimport.go +++ b/src/go/internal/gcimporter/bimport.go @@ -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) diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go index 56870a1412..63abf97e7e 100644 --- a/src/go/internal/gcimporter/gcimporter_test.go +++ b/src/go/internal/gcimporter/gcimporter_test.go @@ -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) } } diff --git a/src/go/internal/srcimporter/srcimporter_test.go b/src/go/internal/srcimporter/srcimporter_test.go index 356e71d128..7310aa7d3f 100644 --- a/src/go/internal/srcimporter/srcimporter_test.go +++ b/src/go/internal/srcimporter/srcimporter_test.go @@ -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) } } diff --git a/src/go/types/type.go b/src/go/types/type.go index a58684a535..374966c4ed 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -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))