From 4329a10ae7748848fb5aae36d3c660468fe81eec Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Wed, 9 Jul 2014 20:00:49 -0700 Subject: [PATCH] go.tools/go: separate interface construction from method set construction We introduce a method (*Interface).Complete(), which is intended to be called from clients after all embedded interfaces have been fully defined. For importers, this will definitely be the case after the import has finished, so each importer have been updated to do so, with the exception of the gcimporter, which does not use embedded interfaces, therefore Complete() can be called immediately after construction. Building the method set separately from the constructor type caused some problems with go/importer, which copies the types.Interface object, leading to there existing two almost-identical interface types referenced from interface method receivers, only one of which has been completed. To avoid this situation, the importer has been modified to construct the interface object only once. Fixes golang/go#8177. LGTM=gri R=gri, dave, gordon.klaus, adonovan CC=golang-codereviews https://golang.org/cl/105060044 --- go/gccgoimporter/parser.go | 5 ++++ go/gcimporter/gcimporter.go | 4 ++- go/importer/export.go | 21 +++++++++------ go/importer/import.go | 18 ++++++++++--- go/importer/import_test.go | 2 ++ go/pointer/gen.go | 2 +- go/types/type.go | 54 +++++++++++++++++++++++++------------ go/types/universe.go | 2 +- 8 files changed, 77 insertions(+), 31 deletions(-) diff --git a/go/gccgoimporter/parser.go b/go/gccgoimporter/parser.go index 6291e87a..6c4faf6c 100644 --- a/go/gccgoimporter/parser.go +++ b/go/gccgoimporter/parser.go @@ -839,6 +839,11 @@ func (p *parser) parsePackage() *types.Package { for p.tok != scanner.EOF { p.parseDirective() } + for _, typ := range p.typeMap { + if it, ok := typ.(*types.Interface); ok { + it.Complete() + } + } p.pkg.MarkComplete() return p.pkg } diff --git a/go/gcimporter/gcimporter.go b/go/gcimporter/gcimporter.go index 743baa9c..f0e0412e 100644 --- a/go/gcimporter/gcimporter.go +++ b/go/gcimporter/gcimporter.go @@ -598,7 +598,9 @@ func (p *parser) parseInterfaceType() types.Type { } p.expect('}') - return types.NewInterface(methods, nil) + // Complete requires the type's embedded interfaces to be fully defined, + // but we do not define any + return types.NewInterface(methods, nil).Complete() } // ChanType = ( "chan" [ "<-" ] | "<-" "chan" ) Type . diff --git a/go/importer/export.go b/go/importer/export.go index 7156bb7c..95799630 100644 --- a/go/importer/export.go +++ b/go/importer/export.go @@ -328,16 +328,21 @@ func (p *exporter) qualifiedName(pkg *types.Package, name string) { } func (p *exporter) signature(sig *types.Signature) { - // TODO(gri) We only need to record the receiver type - // for interface methods if we flatten them - // out. If we track embedded types instead, - // the information is already present. - // We do need the receiver information (T vs *T) + // We need the receiver information (T vs *T) // for methods associated with named types. + // We do not record interface receiver types in the + // export data because 1) the importer can derive them + // from the interface type and 2) they create cycles + // in the type graph. if recv := sig.Recv(); recv != nil { - // 1-element tuple - p.int(1) - p.param(recv) + if _, ok := recv.Type().Underlying().(*types.Interface); !ok { + // 1-element tuple + p.int(1) + p.param(recv) + } else { + // 0-element tuple + p.int(0) + } } else { // 0-element tuple p.int(0) diff --git a/go/importer/import.go b/go/importer/import.go index 7fce5a53..16c6eba3 100644 --- a/go/importer/import.go +++ b/go/importer/import.go @@ -72,6 +72,13 @@ func ImportData(imports map[string]*types.Package, data []byte) (int, *types.Pac p.obj(pkg) } + // complete interfaces + for _, typ := range p.typList { + if it, ok := typ.(*types.Interface); ok { + it.Complete() + } + } + // package was imported completely and without errors pkg.MarkComplete() @@ -254,8 +261,12 @@ func (p *importer) typ() types.Type { return t case interfaceTag: - t := new(types.Interface) - p.record(t) + // Create a dummy entry in the type list. This is safe because we + // 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. + n := len(p.typList) + p.record(nil) // read embedded interfaces embeddeds := make([]*types.Named, p.int()) @@ -270,7 +281,8 @@ func (p *importer) typ() types.Type { methods[i] = types.NewFunc(token.NoPos, pkg, name, p.typ().(*types.Signature)) } - *t = *types.NewInterface(methods, embeddeds) + t := types.NewInterface(methods, embeddeds) + p.typList[n] = t return t case mapTag: diff --git a/go/importer/import_test.go b/go/importer/import_test.go index 7950d7e4..14d218ae 100644 --- a/go/importer/import_test.go +++ b/go/importer/import_test.go @@ -61,6 +61,8 @@ var tests = []string{ `package p; type T chan int`, `package p; type T <-chan complex64`, `package p; type T chan<- map[int]string`, + // test case for issue 8177 + `package p; type T1 interface { F(T2) }; type T2 interface { T1 }`, // vars `package p; var X int`, diff --git a/go/pointer/gen.go b/go/pointer/gen.go index 12bbfb1f..d8f96a72 100644 --- a/go/pointer/gen.go +++ b/go/pointer/gen.go @@ -20,7 +20,7 @@ import ( ) var ( - tEface = types.NewInterface(nil, nil) + tEface = types.NewInterface(nil, nil).Complete() tInvalid = types.Typ[types.Invalid] tUnsafePtr = types.Typ[types.UnsafePointer] ) diff --git a/go/types/type.go b/go/types/type.go index 5aeb416e..a0c4e5ee 100644 --- a/go/types/type.go +++ b/go/types/type.go @@ -263,29 +263,12 @@ func NewInterface(methods []*Func, embeddeds []*Named) *Interface { } sort.Sort(byUniqueMethodName(methods)) - var allMethods []*Func if embeddeds == nil { - allMethods = methods - } else { - allMethods = append(allMethods, methods...) - for _, t := range embeddeds { - it := t.Underlying().(*Interface) - for _, tm := range it.allMethods { - // Make a copy of the method and adjust its receiver type. - newm := *tm - newmtyp := *tm.typ.(*Signature) - newm.typ = &newmtyp - newmtyp.recv = NewVar(newm.pos, newm.pkg, "", typ) - allMethods = append(allMethods, &newm) - } - } sort.Sort(byUniqueTypeName(embeddeds)) - sort.Sort(byUniqueMethodName(allMethods)) } typ.methods = methods typ.embeddeds = embeddeds - typ.allMethods = allMethods return typ } @@ -313,6 +296,43 @@ func (t *Interface) Method(i int) *Func { return t.allMethods[i] } // Empty returns true if t is the empty interface. func (t *Interface) Empty() bool { return len(t.allMethods) == 0 } +// Complete computes the interface's method set. It must be called by users of +// NewInterface after the interface's embedded types are fully defined and +// before using the interface type in any way other than to form other types. +// Complete returns the receiver. +func (t *Interface) Complete() *Interface { + if t.allMethods != nil { + return t + } + + var allMethods []*Func + if t.embeddeds == nil { + if t.methods == nil { + allMethods = make([]*Func, 0, 1) + } else { + allMethods = t.methods + } + } else { + allMethods = append(allMethods, t.methods...) + for _, et := range t.embeddeds { + it := et.Underlying().(*Interface) + it.Complete() + for _, tm := range it.allMethods { + // Make a copy of the method and adjust its receiver type. + newm := *tm + newmtyp := *tm.typ.(*Signature) + newm.typ = &newmtyp + newmtyp.recv = NewVar(newm.pos, newm.pkg, "", t) + allMethods = append(allMethods, &newm) + } + } + sort.Sort(byUniqueMethodName(allMethods)) + } + t.allMethods = allMethods + + return t +} + // A Map represents a map type. type Map struct { key, elem Type diff --git a/go/types/universe.go b/go/types/universe.go index 3d01e604..1494db0f 100644 --- a/go/types/universe.go +++ b/go/types/universe.go @@ -69,7 +69,7 @@ func defPredeclaredTypes() { res := NewVar(token.NoPos, nil, "", Typ[String]) sig := &Signature{results: NewTuple(res)} err := NewFunc(token.NoPos, nil, "Error", sig) - typ := &Named{underlying: NewInterface([]*Func{err}, nil)} + typ := &Named{underlying: NewInterface([]*Func{err}, nil).Complete()} sig.recv = NewVar(token.NoPos, nil, "", typ) def(NewTypeName(token.NoPos, nil, "error", typ)) }