diff --git a/go/types/api.go b/go/types/api.go index 76643dcd..25f8e14b 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -233,7 +233,7 @@ func (init *Initializer) String() string { // The clean path must not be empty or dot ("."). func (conf *Config) Check(path string, fset *token.FileSet, files []*ast.File, info *Info) (*Package, error) { pkg := NewPackage(path, "") - return pkg, newChecker(conf, fset, pkg, info).files(files) + return pkg, NewChecker(conf, fset, pkg, info).Files(files) } // AssertableTo reports whether a value of type V can be asserted to have type T. diff --git a/go/types/api_test.go b/go/types/api_test.go index 209fc6b1..fd29a6af 100644 --- a/go/types/api_test.go +++ b/go/types/api_test.go @@ -404,6 +404,9 @@ func TestInitOrderInfo(t *testing.T) { {`package p9; type T struct{}; func (T) m() int { _ = y; return 0 }; var x, y = T.m, 1`, []string{ "y = 1", "x = T.m", }}, + {`package p10; var (d = c + b; a = 0; b = 0; c = 0)`, []string{ + "b = 0", "c = 0", "d = c + b", "a = 0", + }}, // test case for issue 7131 {`package main diff --git a/go/types/check.go b/go/types/check.go index d413fe01..9b6b95d1 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -48,21 +48,25 @@ type context struct { // It must be created with newChecker. type checker struct { // package information - // (set by newChecker) + // (initialized by NewChecker, valid for the life-time of checker) conf *Config fset *token.FileSet pkg *Package *Info + objMap map[Object]*declInfo // maps package-level object to declaration info + + // information collected during type-checking of a set of package files + // (initialized by Files, valid only for the duration of check.Files; + // maps and lists are allocated on demand) + files []*ast.File // package files + fileScopes []*Scope // file scope for each file + dotImports []map[*Package]token.Pos // positions of dot-imports for each file - // information collected during type-checking of an entire package - // (maps are allocated lazily) firstErr error // first error encountered methods map[string][]*Func // maps type names to associated methods untyped map[ast.Expr]exprInfo // map of expressions without final type - funcs []funcInfo // list of functions/methods with correct signatures and non-empty bodies - delayed []func() // delayed checks that require fully setup types - - objMap map[Object]*declInfo // map of package-level objects to declaration info + funcs []funcInfo // list of functions to type-check + delayed []func() // delayed checks requiring fully setup types // context within which the current object is type-checked // (valid only for the duration of type-checking a specific object) @@ -72,6 +76,25 @@ type checker struct { indent int // indentation for tracing } +// addDeclDep adds the dependency edge (check.decl -> to) +// if check.decl exists and to has an initializer. +func (check *checker) addDeclDep(to Object) { + from := check.decl + if from == nil { + return // not in a package-level init expression + } + decl := check.objMap[to] + if decl == nil || !decl.hasInitializer() { + return // to is not a package-level object or has no initializer + } + m := from.deps + if m == nil { + m = make(map[Object]*declInfo) + from.deps = m + } + m[to] = decl +} + func (check *checker) assocMethod(tname string, meth *Func) { m := check.methods if m == nil { @@ -98,8 +121,9 @@ func (check *checker) delay(f func()) { check.delayed = append(check.delayed, f) } -// newChecker returns a new Checker instance. -func newChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *checker { +// NewChecker returns a new Checker instance for a given package. +// Package files may be incrementally added via checker.Files. +func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *checker { // make sure we have a configuration if conf == nil { conf = new(Config) @@ -116,13 +140,46 @@ func newChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *ch } return &checker{ - conf: conf, - fset: fset, - pkg: pkg, - Info: info, + conf: conf, + fset: fset, + pkg: pkg, + Info: info, + objMap: make(map[Object]*declInfo), } } +// 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) { + // determine package name, files, and set up file scopes, dotImports maps + pkg := check.pkg + for _, file := range files { + switch name := file.Name.Name; pkg.name { + case "": + pkg.name = name + fallthrough + + case name: + check.files = append(check.files, file) + fileScope := NewScope(pkg.scope) + check.recordScope(file, fileScope) + check.fileScopes = append(check.fileScopes, fileScope) + check.dotImports = append(check.dotImports, nil) // element (map) is lazily allocated + + default: + check.errorf(file.Package, "package %s; expected %s", name, pkg.name) + // 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. type bailout struct{} @@ -137,76 +194,45 @@ func (check *checker) handleBailout(err *error) { } } -func (check *checker) files(files []*ast.File) (err error) { +// Files checks the provided files as part of the checker's package. +func (check *checker) Files(files []*ast.File) (err error) { defer check.handleBailout(&err) - pkg := check.pkg + check.initFiles(files) - // determine package name and files - i := 0 - for _, file := range files { - switch name := file.Name.Name; pkg.name { - case "": - pkg.name = name - fallthrough - case name: - files[i] = file - i++ - default: - check.errorf(file.Package, "package %s; expected %s", name, pkg.name) - // ignore this file - } - } + check.collectObjects() - check.resolveFiles(files[:i]) + check.packageObjects() + + check.functionBodies() + + check.initDependencies() + + check.unusedImports() // perform delayed checks for _, f := range check.delayed { f() } - check.delayed = nil // not needed anymore - // remaining untyped expressions must indeed be untyped - if debug { - for x, info := range check.untyped { - if isTyped(info.typ) { - check.dump("%s: %s (type %s) is typed", x.Pos(), x, info.typ) - panic(0) - } - } - } + check.recordUntyped() - // notify client of any untyped types left - // TODO(gri) Consider doing this before and - // after function body checking for smaller - // map size and more immediate feedback. - if check.Types != nil { - for x, info := range check.untyped { - check.recordTypeAndValue(x, info.typ, info.val) - } - } - - pkg.complete = true + check.pkg.complete = true return } -// addDeclDep adds the dependency edge (check.decl -> to) -// if check.decl exists and to has an initializer. -func (check *checker) addDeclDep(to Object) { - from := check.decl - if from == nil { - return // not in a package-level init expression +func (check *checker) recordUntyped() { + if !debug && check.Types == nil { + return // nothing to do } - decl := check.objMap[to] - if decl == nil || !decl.hasInitializer() { - return // to is not a package-level object or has no initializer + + for x, info := range check.untyped { + if debug && isTyped(info.typ) { + check.dump("%s: %s (type %s) is typed", x.Pos(), x, info.typ) + unreachable() + } + check.recordTypeAndValue(x, info.typ, info.val) } - m := from.deps - if m == nil { - m = make(map[Object]*declInfo) - from.deps = m - } - m[to] = decl } func (check *checker) recordTypeAndValue(x ast.Expr, typ Type, val exact.Value) { diff --git a/go/types/eval.go b/go/types/eval.go index 2ee62cec..549aadbf 100644 --- a/go/types/eval.go +++ b/go/types/eval.go @@ -86,7 +86,7 @@ func EvalNode(fset *token.FileSet, node ast.Expr, pkg *Package, scope *Scope) (t } // initialize checker - check := newChecker(nil, fset, pkg, nil) + check := NewChecker(nil, fset, pkg, nil) check.scope = scope defer check.handleBailout(&err) diff --git a/go/types/resolver.go b/go/types/resolver.go index 07fbf774..678c0fa2 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -84,33 +84,28 @@ func validatedImportPath(path string) (string, error) { return s, nil } -// TODO(gri) Split resolveFiles into smaller components. +// declarePkgObj declares obj in the package scope, records its ident -> obj mapping, +// and updates check.objMap. The object must not be a function or method. +func (check *checker) declarePkgObj(ident *ast.Ident, obj Object, d *declInfo) { + assert(ident.Name == obj.Name()) -func (check *checker) resolveFiles(files []*ast.File) { - pkg := check.pkg - - // Phase 1: Pre-declare all package-level objects so that they can be found - // independent of source order. Associate methods with receiver - // base type names. - - var objMap = make(map[Object]*declInfo) // corresponding declaration info - - // declare declares obj in the package scope, records its ident -> obj mapping, - // and updates objMap. The object must not be a function or method. - declare := func(ident *ast.Ident, obj Object, d *declInfo) { - assert(ident.Name == obj.Name()) - - // spec: "A package-scope or file-scope identifier with name init - // may only be declared to be a function with this (func()) signature." - if ident.Name == "init" { - check.errorf(ident.Pos(), "cannot declare init - must be func") - return - } - - check.declare(pkg.scope, ident, obj) - objMap[obj] = d + // spec: "A package-scope or file-scope identifier with name init + // may only be declared to be a function with this (func()) signature." + if ident.Name == "init" { + check.errorf(ident.Pos(), "cannot declare init - must be func") + return } + check.declare(check.pkg.scope, ident, obj) + check.objMap[obj] = d +} + +// collectObjects collects all file and package objects and inserts them +// into their respective scopes. It also performs imports and associates +// methods with receiver base type names. +func (check *checker) collectObjects() { + pkg := check.pkg + importer := check.conf.Import if importer == nil { if DefaultImport == nil { @@ -127,20 +122,11 @@ func (check *checker) resolveFiles(files []*ast.File) { pkgImports[imp] = true } - var ( - fileScopes []*Scope // file scope for each file - dotImports []map[*Package]token.Pos // positions of dot-imports for each file - ) - - for _, file := range files { + for fileNo, file := range check.files { // The package identifier denotes the current package, // but there is no corresponding package object. check.recordDef(file.Name, nil) - - fileScope := NewScope(pkg.scope) - check.recordScope(file, fileScope) - fileScopes = append(fileScopes, fileScope) - dotImports = append(dotImports, nil) // element (map) is lazily allocated + fileScope := check.fileScopes[fileNo] for _, decl := range file.Decls { switch d := decl.(type) { @@ -218,10 +204,10 @@ func (check *checker) resolveFiles(files []*ast.File) { } // add position to set of dot-import positions for this file // (this is only needed for "imported but not used" errors) - posSet := dotImports[len(dotImports)-1] + posSet := check.dotImports[fileNo] if posSet == nil { posSet = make(map[*Package]token.Pos) - dotImports[len(dotImports)-1] = posSet + check.dotImports[fileNo] = posSet } posSet[imp] = s.Pos() } else { @@ -250,7 +236,7 @@ func (check *checker) resolveFiles(files []*ast.File) { } d := &declInfo{file: fileScope, typ: last.Type, init: init} - declare(name, obj, d) + check.declarePkgObj(name, obj, d) } check.arityMatch(s, last) @@ -284,7 +270,7 @@ func (check *checker) resolveFiles(files []*ast.File) { d = &declInfo{file: fileScope, typ: s.Type, init: init} } - declare(name, obj, d) + check.declarePkgObj(name, obj, d) } check.arityMatch(s, nil) @@ -295,7 +281,7 @@ func (check *checker) resolveFiles(files []*ast.File) { case *ast.TypeSpec: obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Name, nil) - declare(s.Name, obj, &declInfo{file: fileScope, typ: s.Type}) + check.declarePkgObj(s.Name, obj, &declInfo{file: fileScope, typ: s.Type}) default: check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) @@ -334,29 +320,29 @@ func (check *checker) resolveFiles(files []*ast.File) { } } info := &declInfo{file: fileScope, fdecl: d} - objMap[obj] = info + check.objMap[obj] = info default: check.invalidAST(d.Pos(), "unknown ast.Decl node %T", d) } } } - pkgImports = nil // not needed anymore - // Phase 2: Verify that objects in package and file scopes have different names. - - for _, scope := range fileScopes { + // verify that objects in package and file scopes have different names + for _, scope := range check.fileScopes { for _, obj := range scope.elems { if alt := pkg.scope.Lookup(obj.Name()); alt != nil { check.errorf(alt.Pos(), "%s already declared in this file through import of package %s", obj.Name(), obj.Pkg().Name()) } } } +} - // Phase 3: Typecheck all objects in objMap, but not function bodies. +// packageObjects typechecks all package objects in check.objMap, but not function bodies. +func (check *checker) packageObjects() { + // pre-allocate space for type declaration paths so that the underlying array is reused + typePath := make([]*TypeName, 0, 8) - check.objMap = objMap - typePath := make([]*TypeName, 0, 8) // pre-allocate space for type declaration paths so that the underlying array is reused for _, obj := range objectsOf(check.objMap) { check.objDecl(obj, nil, typePath) } @@ -366,33 +352,33 @@ func (check *checker) resolveFiles(files []*ast.File) { // types were not declared. In that case, an error was reported when declaring those // methods. We can now safely discard this map. check.methods = nil +} - // Phase 4: Typecheck all functions bodies. - +// functionBodies typechecks all function bodies. +func (check *checker) functionBodies() { for _, f := range check.funcs { check.funcBody(f.decl, f.name, f.sig, f.body) } +} - // Phase 5: Check initialization dependencies. - // Note: must happen after checking all functions because function bodies - // may introduce dependencies +// initDependencies computes initialization dependencies. +func (check *checker) initDependencies() { + // pre-allocate space for initialization paths so that the underlying array is reused + initPath := make([]Object, 0, 8) - initPath := make([]Object, 0, 8) // pre-allocate space for initialization paths so that the underlying array is reused - for _, obj := range objectsOf(objMap) { + for _, obj := range objectsOf(check.objMap) { switch obj.(type) { case *Const, *Var: - if d := objMap[obj]; d.hasInitializer() { + if d := check.objMap[obj]; d.hasInitializer() { check.dependencies(obj, d, initPath) } } } +} - // Phase 6: Check for declared but not used packages and function variables. - // Note: must happen after checking all functions because function bodies - // may introduce package uses - - // If function bodies are not checked, packages' uses are likely missing, - // and there are no unused function variables. Nothing left to do. +// unusedImports checks for unused imports. +func (check *checker) unusedImports() { + // if function bodies are not checked, packages' uses are likely missing - don't check if check.conf.IgnoreFuncBodies { return } @@ -400,7 +386,7 @@ func (check *checker) resolveFiles(files []*ast.File) { // spec: "It is illegal (...) to directly import a package without referring to // any of its exported identifiers. To import a package solely for its side-effects // (initialization), use the blank identifier as explicit package name." - for i, scope := range fileScopes { + for i, scope := range check.fileScopes { var usedDotImports map[*Package]bool // lazily allocated for _, obj := range scope.elems { switch obj := obj.(type) { @@ -424,7 +410,7 @@ func (check *checker) resolveFiles(files []*ast.File) { } // Iterate through all dot-imports for this file and // check if the corresponding package was used. - for pkg, pos := range dotImports[i] { + for pkg, pos := range check.dotImports[i] { if !usedDotImports[pkg] { check.softErrorf(pos, "%q imported but not used", pkg.path) }