diff --git a/go/types/api_test.go b/go/types/api_test.go index fd29a6af..edd70f66 100644 --- a/go/types/api_test.go +++ b/go/types/api_test.go @@ -5,6 +5,7 @@ package types_test import ( + "fmt" "go/ast" "go/parser" "go/token" @@ -444,3 +445,27 @@ func TestInitOrderInfo(t *testing.T) { } } } + +func TestFiles(t *testing.T) { + var sources = []string{ + "package p; type T struct{}; func (T) m1() {}", + "package p; func (T) m2() {}; var _ interface{ m1(); m2() } = T{}", + "package p; func (T) m3() {}; var _ interface{ m1(); m2(); m3() } = T{}", + } + + var conf Config + fset := token.NewFileSet() + pkg := NewPackage("p", "p") + check := NewChecker(&conf, fset, pkg, nil) + + for i, src := range sources { + filename := fmt.Sprintf("sources%d", i) + f, err := parser.ParseFile(fset, filename, src, 0) + if err != nil { + t.Fatal(err) + } + if err := check.Files([]*ast.File{f}); err != nil { + t.Error(err) + } + } +} diff --git a/go/types/check.go b/go/types/check.go index 9b6b95d1..56fe49dc 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -151,6 +151,17 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *ch // initFiles initializes the files-specific portion of checker. // The provided files must all belong to the same package. func (check *checker) initFiles(files []*ast.File) { + // start with a clean slate (check.Files may be called multiple times) + check.files = nil + check.fileScopes = nil + check.dotImports = nil + + check.firstErr = nil + check.methods = nil + check.untyped = nil + check.funcs = nil + check.delayed = nil + // determine package name, files, and set up file scopes, dotImports maps pkg := check.pkg for _, file := range files { @@ -171,13 +182,6 @@ func (check *checker) initFiles(files []*ast.File) { // ignore this file } } - - // start with a clean slate (check.Files may be called multiple times) - check.firstErr = nil - check.methods = nil - check.untyped = nil - check.funcs = nil - check.delayed = nil } // A bailout panic is raised to indicate early termination. @@ -202,11 +206,12 @@ func (check *checker) Files(files []*ast.File) (err error) { check.collectObjects() - check.packageObjects() + objList := objectsOf(check.objMap) + check.packageObjects(objList) check.functionBodies() - check.initDependencies() + check.initDependencies(objList) check.unusedImports() diff --git a/go/types/decl.go b/go/types/decl.go index 6cd4f235..1bb2b5f8 100644 --- a/go/types/decl.go +++ b/go/types/decl.go @@ -163,7 +163,7 @@ func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) { } // underlying returns the underlying type of typ; possibly by following -// forward chains of named types. Such chains only exist while names types +// forward chains of named types. Such chains only exist while named types // are incomplete. func underlying(typ Type) Type { for { @@ -210,22 +210,29 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []* // any forward chain (they always end in an unnamed type). named.underlying = underlying(named.underlying) - // type-check signatures of associated methods + // check and add associated methods + // TODO(gri) It's easy to create pathological cases where the + // current approach is incorrect: In general we need to know + // and add all methods _before_ type-checking the type. + // See http://play.golang.org/p/WMpE0q2wK8 + check.addMethodDecls(obj) +} + +func (check *checker) addMethodDecls(obj *TypeName) { + // get associated methods methods := check.methods[obj.name] if len(methods) == 0 { return // no methods } + delete(check.methods, obj.name) - // spec: "For a base type, the non-blank names of methods bound - // to it must be unique." - // => use an objset to determine redeclarations + // use an objset to check for name conflicts var mset objset // spec: "If the base type is a struct type, the non-blank method // and field names must be distinct." - // => pre-populate the objset to find conflicts - // TODO(gri) consider keeping the objset with the struct instead - if t, _ := named.underlying.(*Struct); t != nil { + base := obj.typ.(*Named) + if t, _ := base.underlying.(*Struct); t != nil { for _, fld := range t.fields { if fld.name != "_" { assert(mset.insert(fld) == nil) @@ -233,15 +240,25 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []* } } - // check each method + // Checker.Files may be called multiple times; additional package files + // may add methods to already type-checked types. Add pre-existing methods + // so that we can detect redeclarations. + for _, m := range base.methods { + assert(m.name != "_") + assert(mset.insert(m) == nil) + } + + // type-check methods for _, m := range methods { + // spec: "For a base type, the non-blank names of methods bound + // to it must be unique." if m.name != "_" { if alt := mset.insert(m); alt != nil { switch alt.(type) { case *Var: check.errorf(m.pos, "field and method with the same name %s", m.name) case *Func: - check.errorf(m.pos, "method %s already declared for %s", m.name, named) + check.errorf(m.pos, "method %s already declared for %s", m.name, base) default: unreachable() } @@ -249,16 +266,12 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []* continue } } - check.recordDef(check.objMap[m].fdecl.Name, m) check.objDecl(m, nil, nil) - // Methods with blank _ names cannot be found. - // Don't add them to the method list. + // methods with blank _ names cannot be found - don't keep them if m.name != "_" { - named.methods = append(named.methods, m) + base.methods = append(base.methods, m) } } - - delete(check.methods, obj.name) // we don't need them anymore } func (check *checker) funcDecl(obj *Func, decl *declInfo) { diff --git a/go/types/resolver.go b/go/types/resolver.go index 678c0fa2..29ebd92c 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -305,6 +305,8 @@ func (check *checker) collectObjects() { check.declare(pkg.scope, d.Name, obj) } } else { + // method + check.recordDef(d.Name, obj) // Associate method with receiver base type name, if possible. // Ignore methods that have an invalid receiver, or a blank _ // receiver name. They will be type-checked later, with regular @@ -339,17 +341,24 @@ func (check *checker) collectObjects() { } // packageObjects typechecks all package objects in check.objMap, but not function bodies. -func (check *checker) packageObjects() { +func (check *checker) packageObjects(objList []Object) { + // add new methods to already type-checked types (from a prior Checker.Files call) + for _, obj := range objList { + if obj, _ := obj.(*TypeName); obj != nil && obj.typ != nil { + check.addMethodDecls(obj) + } + } + // pre-allocate space for type declaration paths so that the underlying array is reused typePath := make([]*TypeName, 0, 8) - for _, obj := range objectsOf(check.objMap) { + for _, obj := range objList { check.objDecl(obj, nil, typePath) } // At this point we may have a non-empty check.methods map; this means that not all // entries were deleted at the end of typeDecl because the respective receiver base - // types were not declared. In that case, an error was reported when declaring those + // types were not found. In that case, an error was reported when declaring those // methods. We can now safely discard this map. check.methods = nil } @@ -362,11 +371,11 @@ func (check *checker) functionBodies() { } // initDependencies computes initialization dependencies. -func (check *checker) initDependencies() { +func (check *checker) initDependencies(objList []Object) { // pre-allocate space for initialization paths so that the underlying array is reused initPath := make([]Object, 0, 8) - for _, obj := range objectsOf(check.objMap) { + for _, obj := range objList { switch obj.(type) { case *Const, *Var: if d := check.objMap[obj]; d.hasInitializer() {