From 9111cead205b1d04568d7312fe5001b733c46e4a Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 20 Feb 2014 14:52:21 -0800 Subject: [PATCH] go.tools/go/types: internal cleanups - clearer separation between package-level and object-level state - lazy allocation of various package-level maps - NewPackage automatically allocates a package scope - steps towards enabling incremental checking of extra (test) files after the (non-test) package is type-checked (not yet exposed) LGTM=adonovan R=adonovan CC=golang-codereviews https://golang.org/cl/66230044 --- go/gcimporter/gcimporter.go | 4 +- go/types/api.go | 7 +- go/types/assignments.go | 4 +- go/types/call.go | 4 +- go/types/check.go | 263 ++++++++++++++++++++---------------- go/types/decl.go | 40 +++--- go/types/eval.go | 5 +- go/types/expr.go | 6 +- go/types/package.go | 9 +- go/types/resolver.go | 9 +- go/types/return.go | 2 +- go/types/stmt.go | 39 +++--- go/types/typexpr.go | 4 +- go/types/universe.go | 2 +- 14 files changed, 209 insertions(+), 189 deletions(-) diff --git a/go/gcimporter/gcimporter.go b/go/gcimporter/gcimporter.go index fcff9fa1..743baa9c 100644 --- a/go/gcimporter/gcimporter.go +++ b/go/gcimporter/gcimporter.go @@ -347,7 +347,7 @@ func (p *parser) getPkg(id, name string) *types.Package { } pkg := p.imports[id] if pkg == nil && name != "" { - pkg = types.NewPackage(id, name, types.NewScope(nil)) + pkg = types.NewPackage(id, name) p.imports[id] = pkg } return pkg @@ -434,7 +434,7 @@ func (p *parser) parseName(materializePkg bool) (pkg *types.Package, name string // doesn't exist yet, create a fake package instead pkg = p.getPkg(id, "") if pkg == nil { - pkg = types.NewPackage(id, "", nil) + pkg = types.NewPackage(id, "") } } default: diff --git a/go/types/api.go b/go/types/api.go index 0dfc083a..43db3ed6 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -219,11 +219,8 @@ func (init *Initializer) String() string { // file set, and the package path the package is identified with. // 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, err := conf.check(path, fset, files, info) - if err == nil { - pkg.complete = true - } - return pkg, err + pkg := NewPackage(path, "") + 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/assignments.go b/go/types/assignments.go index d7f876bc..8961f0f9 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -158,7 +158,7 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type { var v *Var var v_used bool if ident != nil { - if obj := check.topScope.LookupParent(ident.Name); obj != nil { + if obj := check.scope.LookupParent(ident.Name); obj != nil { v, _ = obj.(*Var) if v != nil { v_used = v.used @@ -261,7 +261,7 @@ func (check *checker) assignVars(lhs, rhs []ast.Expr) { } func (check *checker) shortVarDecl(pos token.Pos, lhs, rhs []ast.Expr) { - scope := check.topScope + scope := check.scope // collect lhs variables var newVars []*Var diff --git a/go/types/call.go b/go/types/call.go index 2773e90a..21aa7872 100644 --- a/go/types/call.go +++ b/go/types/call.go @@ -33,7 +33,7 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind { if x.mode != invalid { check.conversion(x, T) if x.mode != invalid { - check.conversions[e] = true // for cap/len checking + check.markAsConversion(e) // for cap/len checking } } default: @@ -251,7 +251,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) { // can only appear in qualified identifiers which are mapped to // selector expressions. if ident, ok := e.X.(*ast.Ident); ok { - if pkg, _ := check.topScope.LookupParent(ident.Name).(*PkgName); pkg != nil { + if pkg, _ := check.scope.LookupParent(ident.Name).(*PkgName); pkg != nil { check.recordObject(ident, pkg) pkg.used = true exp := pkg.pkg.scope.Lookup(sel) diff --git a/go/types/check.go b/go/types/check.go index c4694bec..ee3a02c6 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -9,7 +9,6 @@ package types import ( "go/ast" "go/token" - "path" "code.google.com/p/go.tools/go/exact" ) @@ -27,44 +26,172 @@ type exprInfo struct { val exact.Value // constant value; or nil (if not a constant) } -// A checker is an instance of the type-checker. +// A context represents the context within which an object is type-checked. +type context struct { + decl *declInfo // package-level declaration whose init expression/function body is checked + scope *Scope // top-most scope for lookups + iota exact.Value // value of iota in a constant declaration; nil otherwise + sig *Signature // function signature if inside a function; nil otherwise + hasLabel bool // set if a function makes use of labels (only ~1% of functions); unused outside functions +} + +// A checker maintains the state of the type checker. +// It must be created with newChecker. type checker struct { + // package information + // (set by newChecker) conf *Config fset *token.FileSet pkg *Package + *Info + // 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 conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls) 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 - firstErr error // first error encountered - Info // collected type info + objMap map[Object]*declInfo // if set we are in the package-level declaration phase (otherwise all objects seen must be declared) + initMap map[Object]*declInfo // map of variables/functions with init expressions/bodies - objMap map[Object]*declInfo // if set we are in the package-level declaration phase (otherwise all objects seen must be declared) - initMap map[Object]*declInfo // map of variables/functions with init expressions/bodies - topScope *Scope // current topScope for lookups - iota exact.Value // current value of iota in a constant declaration; nil otherwise - decl *declInfo // current package-level declaration whose init expression/body is type-checked - - // functions - funcList []funcInfo // list of functions/methods with correct signatures and non-empty bodies - funcSig *Signature // signature of currently type-checked function - hasLabel bool // set if a function makes use of labels (only ~1% of functions) + // context within which the current object is type-checked + // (valid only for the duration of type-checking a specific object) + context // debugging indent int // indentation for tracing } -func newChecker(conf *Config, fset *token.FileSet, pkg *Package) *checker { - return &checker{ - conf: conf, - fset: fset, - pkg: pkg, - methods: make(map[string][]*Func), - conversions: make(map[*ast.CallExpr]bool), - untyped: make(map[ast.Expr]exprInfo), +func (check *checker) assocMethod(tname string, meth *Func) { + m := check.methods + if m == nil { + m = make(map[string][]*Func) + check.methods = m } + m[tname] = append(m[tname], meth) +} + +func (check *checker) markAsConversion(e *ast.CallExpr) { + m := check.conversions + if m == nil { + m = make(map[*ast.CallExpr]bool) + check.conversions = m + } + m[e] = true +} + +func (check *checker) rememberUntyped(e ast.Expr, lhs bool, typ *Basic, val exact.Value) { + m := check.untyped + if m == nil { + m = make(map[ast.Expr]exprInfo) + check.untyped = m + } + m[e] = exprInfo{lhs, typ, val} +} + +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 { + // make sure we have a configuration + if conf == nil { + conf = new(Config) + } + + // make sure we have a package canonicalization map + if conf.Packages == nil { + conf.Packages = make(map[string]*Package) + } + + // make sure we have an info struct + if info == nil { + info = new(Info) + } + + return &checker{ + conf: conf, + fset: fset, + pkg: pkg, + Info: info, + } +} + +// A bailout panic is raised to indicate early termination. +type bailout struct{} + +func (check *checker) handleBailout(err *error) { + switch p := recover().(type) { + case nil, bailout: + // normal return or early exit + *err = check.firstErr + default: + // re-panic + panic(p) + } +} + +func (check *checker) files(files []*ast.File) (err error) { + defer check.handleBailout(&err) + + pkg := check.pkg + + // 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.resolveFiles(files[:i]) + + // 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) + } + } + } + + // 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) + } + } + + // copy check.InitOrder back to incoming *info if necessary + // (In case of early (error) bailout, this is not done, but we don't care in that case.) + // if info != nil { + // info.InitOrder = check.InitOrder + // } + + pkg.complete = true + return } // addDeclDep adds the dependency edge (check.decl -> to) @@ -86,10 +213,6 @@ func (check *checker) addDeclDep(to Object) { m[to] = init } -func (check *checker) delay(f func()) { - check.delayed = append(check.delayed, f) -} - func (check *checker) recordTypeAndValue(x ast.Expr, typ Type, val exact.Value) { assert(x != nil && typ != nil) if val != nil { @@ -173,91 +296,3 @@ func (check *checker) recordScope(node ast.Node, scope *Scope) { m[node] = scope } } - -// A bailout panic is raised to indicate early termination. -type bailout struct{} - -func (check *checker) handleBailout(err *error) { - switch p := recover().(type) { - case nil, bailout: - // normal return or early exit - *err = check.firstErr - default: - // re-panic - panic(p) - } -} - -func (conf *Config) check(pkgPath string, fset *token.FileSet, files []*ast.File, info *Info) (pkg *Package, err error) { - // make sure we have a package canonicalization map - if conf.Packages == nil { - conf.Packages = make(map[string]*Package) - } - - pkg = NewPackage(pkgPath, "", NewScope(Universe)) // package name is set below - check := newChecker(conf, fset, pkg) - defer check.handleBailout(&err) - - // we need a reasonable package path to continue - if path.Clean(pkgPath) == "." { - check.errorf(token.NoPos, "invalid package path provided: %q", pkgPath) - return - } - - // 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 - } - } - - // install optional info - if info != nil { - check.Info = *info - } - - check.resolveFiles(files[:i]) - - // 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) - } - } - } - - // 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) - } - } - - // copy check.InitOrder back to incoming *info if necessary - // (In case of early (error) bailout, this is not done, but we don't care in that case.) - if info != nil { - info.InitOrder = check.InitOrder - } - - return -} diff --git a/go/types/decl.go b/go/types/decl.go index b5793ce2..326818bf 100644 --- a/go/types/decl.go +++ b/go/types/decl.go @@ -31,7 +31,7 @@ func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) { } } -// objDecl type-checks the declaration of obj in its respective file scope. +// objDecl type-checks the declaration of obj in its respective (file) context. // See check.typ for the details on def and path. func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { if obj.Type() != nil { @@ -52,17 +52,13 @@ func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { unreachable() } - // adjust file scope for current object - oldScope := check.topScope - check.topScope = d.file // for lookup - - // save current iota - oldIota := check.iota - check.iota = nil - - // save current decl - oldDecl := check.decl - check.decl = nil + // save/restore current context and setup object context + defer func(ctxt context) { + check.context = ctxt + }(check.context) + check.context = context{ + scope: d.file, + } // Const and var declarations must not have initialization // cycles. We track them by remembering the current declaration @@ -85,10 +81,6 @@ func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { default: unreachable() } - - check.decl = oldDecl - check.iota = oldIota - check.topScope = oldScope } func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { @@ -274,13 +266,13 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []* } type funcInfo struct { - name string // for tracing only - info *declInfo // for cycle detection + name string // for debugging/tracing only + decl *declInfo // for cycle detection sig *Signature body *ast.BlockStmt } -func (check *checker) funcDecl(obj *Func, info *declInfo) { +func (check *checker) funcDecl(obj *Func, decl *declInfo) { assert(obj.typ == nil) // func declarations cannot use iota @@ -288,7 +280,7 @@ func (check *checker) funcDecl(obj *Func, info *declInfo) { sig := new(Signature) obj.typ = sig // guard against cycles - fdecl := info.fdecl + fdecl := decl.fdecl check.funcType(sig, fdecl.Recv, fdecl.Type) if sig.recv == nil && obj.name == "init" && (sig.params.Len() > 0 || sig.results.Len() > 0) { check.errorf(fdecl.Pos(), "func init must have no arguments and no return values") @@ -298,7 +290,7 @@ func (check *checker) funcDecl(obj *Func, info *declInfo) { // function body must be type-checked after global declarations // (functions implemented elsewhere have no body) if !check.conf.IgnoreFuncBodies && fdecl.Body != nil { - check.funcList = append(check.funcList, funcInfo{obj.name, info, sig, fdecl.Body}) + check.funcs = append(check.funcs, funcInfo{obj.name, decl, sig, fdecl.Body}) } } @@ -341,7 +333,7 @@ func (check *checker) declStmt(decl ast.Decl) { check.arityMatch(s, last) for i, name := range s.Names { - check.declare(check.topScope, name, lhs[i]) + check.declare(check.scope, name, lhs[i]) } case token.VAR: @@ -388,7 +380,7 @@ func (check *checker) declStmt(decl ast.Decl) { // declare all variables // (only at this point are the variable scopes (parents) set) for i, name := range s.Names { - check.declare(check.topScope, name, lhs0[i]) + check.declare(check.scope, name, lhs0[i]) } default: @@ -397,7 +389,7 @@ func (check *checker) declStmt(decl ast.Decl) { case *ast.TypeSpec: obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Name, nil) - check.declare(check.topScope, s.Name, obj) + check.declare(check.scope, s.Name, obj) check.typeDecl(obj, s.Type, nil, nil) default: diff --git a/go/types/eval.go b/go/types/eval.go index dd4f0269..2ee62cec 100644 --- a/go/types/eval.go +++ b/go/types/eval.go @@ -86,9 +86,8 @@ func EvalNode(fset *token.FileSet, node ast.Expr, pkg *Package, scope *Scope) (t } // initialize checker - var conf Config - check := newChecker(&conf, fset, pkg) - check.topScope = scope + check := newChecker(nil, fset, pkg, nil) + check.scope = scope defer check.handleBailout(&err) // evaluate node diff --git a/go/types/expr.go b/go/types/expr.go index 57424c76..2f6e9d61 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -919,9 +919,9 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) exprKind { assert(x.expr != nil && typ != nil) if isUntyped(typ) { - // delay notification until it becomes typed + // delay type and value recording until we know the type // or until the end of type checking - check.untyped[x.expr] = exprInfo{false, typ.(*Basic), val} + check.rememberUntyped(x.expr, false, typ.(*Basic), val) } else { check.recordTypeAndValue(e, typ, val) } @@ -963,7 +963,7 @@ func (check *checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { // Anonymous functions are considered part of the // init expression/func declaration which contains // them: use existing package-level declaration info. - check.funcBody("", sig, e.Body) + check.funcBody(check.decl, "", sig, e.Body) x.mode = value x.typ = sig } else { diff --git a/go/types/package.go b/go/types/package.go index f09acc7f..039cd962 100644 --- a/go/types/package.go +++ b/go/types/package.go @@ -16,11 +16,10 @@ type Package struct { fake bool // scope lookup errors are silently dropped if package is fake (internal use only) } -// NewPackage returns a new Package for the given package path, -// name, and scope. The package is not complete and contains no -// explicit imports. -func NewPackage(path, name string, scope *Scope) *Package { - return &Package{path: path, name: name, scope: scope} +// NewPackage returns a new Package for the given package path and name. +// The package is not complete and contains no explicit imports. +func NewPackage(path, name string) *Package { + return &Package{path: path, name: name, scope: NewScope(Universe)} } // Path returns the package path. diff --git a/go/types/resolver.go b/go/types/resolver.go index b05411a0..4b9dcfdf 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -151,7 +151,7 @@ func (check *checker) resolveFiles(files []*ast.File) { } if path == "C" && check.conf.FakeImportC { // TODO(gri) shouldn't create a new one each time - imp = NewPackage("C", "C", NewScope(nil)) + imp = NewPackage("C", "C") imp.fake = true } else { var err error @@ -328,7 +328,7 @@ func (check *checker) resolveFiles(files []*ast.File) { typ = ptr.X } if base, _ := typ.(*ast.Ident); base != nil && base.Name != "_" { - check.methods[base.Name] = append(check.methods[base.Name], obj) + check.assocMethod(base.Name, obj) } } } @@ -374,9 +374,8 @@ func (check *checker) resolveFiles(files []*ast.File) { // Phase 4: Typecheck all functions bodies. - for _, f := range check.funcList { - check.decl = f.info - check.funcBody(f.name, f.sig, f.body) + for _, f := range check.funcs { + check.funcBody(f.decl, f.name, f.sig, f.body) } // Phase 5: Check initialization dependencies. diff --git a/go/types/return.go b/go/types/return.go index fb8ab6dc..f16a318b 100644 --- a/go/types/return.go +++ b/go/types/return.go @@ -31,7 +31,7 @@ func (check *checker) isTerminating(s ast.Stmt, label string) bool { // the predeclared (possibly parenthesized) panic() function is terminating if call, _ := unparen(s.X).(*ast.CallExpr); call != nil { if id, _ := call.Fun.(*ast.Ident); id != nil { - if obj := check.topScope.LookupParent(id.Name); obj != nil { + if obj := check.scope.LookupParent(id.Name); obj != nil { if b, _ := obj.(*Builtin); b != nil && b.id == _Panic { return true } diff --git a/go/types/stmt.go b/go/types/stmt.go index 6ca4c6cf..f0faa823 100644 --- a/go/types/stmt.go +++ b/go/types/stmt.go @@ -12,7 +12,7 @@ import ( "go/token" ) -func (check *checker) funcBody(name string, sig *Signature, body *ast.BlockStmt) { +func (check *checker) funcBody(decl *declInfo, name string, sig *Signature, body *ast.BlockStmt) { if trace { if name == "" { name = "" @@ -21,18 +21,17 @@ func (check *checker) funcBody(name string, sig *Signature, body *ast.BlockStmt) defer fmt.Println("--- ") } - // save/restore outer function state - defer func(scope *Scope, sig *Signature, label bool, indent int) { - check.topScope = scope - check.funcSig = sig - check.hasLabel = label + // save/restore current context and setup function context + // (and use 0 indentation at function start) + defer func(ctxt context, indent int) { + check.context = ctxt check.indent = indent - }(check.topScope, check.funcSig, check.hasLabel, check.indent) - - // setup inner function state - check.topScope = sig.scope - check.funcSig = sig - check.hasLabel = false + }(check.context, check.indent) + check.context = context{ + decl: decl, + scope: sig.scope, + sig: sig, + } check.indent = 0 check.stmtList(0, body.List) @@ -118,13 +117,13 @@ func (check *checker) multipleDefaults(list []ast.Stmt) { } func (check *checker) openScope(s ast.Stmt) { - scope := NewScope(check.topScope) + scope := NewScope(check.scope) check.recordScope(s, scope) - check.topScope = scope + check.scope = scope } func (check *checker) closeScope() { - check.topScope = check.topScope.Parent() + check.scope = check.scope.Parent() } func assignOp(op token.Token) token.Token { @@ -211,8 +210,8 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { if p := recover(); p != nil { panic(p) } - assert(scope == check.topScope) - }(check.topScope) + assert(scope == check.scope) + }(check.scope) } inner := ctxt &^ fallthroughOk @@ -319,7 +318,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { check.suspendedCall("defer", s.Call) case *ast.ReturnStmt: - sig := check.funcSig + sig := check.sig if n := sig.results.Len(); n > 0 { // determine if the function has named results named := false @@ -499,7 +498,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { T = x.typ } obj := NewVar(lhs.Pos(), check.pkg, lhs.Name, T) - check.declare(check.topScope, nil, obj) + check.declare(check.scope, nil, obj) check.recordImplicit(clause, obj) // For the "declared but not used" error, all lhs variables act as // one; i.e., if any one of them is 'used', all of them are 'used'. @@ -694,7 +693,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { // declare variables if len(vars) > 0 { for _, obj := range vars { - check.declare(check.topScope, nil, obj) // recordObject already called + check.declare(check.scope, nil, obj) // recordObject already called } } else { check.errorf(s.TokPos, "no new variables on left side of :=") diff --git a/go/types/typexpr.go b/go/types/typexpr.go index a89a217c..3f881429 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -23,7 +23,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa x.mode = invalid x.expr = e - obj := check.topScope.LookupParent(e.Name) + obj := check.scope.LookupParent(e.Name) if obj == nil { if e.Name == "_" { check.errorf(e.Pos(), "cannot use _ as value or type") @@ -141,7 +141,7 @@ func (check *checker) typ(e ast.Expr) Type { // funcType type-checks a function or method type and returns its signature. func (check *checker) funcType(sig *Signature, recv *ast.FieldList, ftyp *ast.FuncType) *Signature { - scope := NewScope(check.topScope) + scope := NewScope(check.scope) check.recordScope(ftyp, scope) recv_, _ := check.collectParams(scope, recv, false) diff --git a/go/types/universe.go b/go/types/universe.go index a371e2a5..255a715b 100644 --- a/go/types/universe.go +++ b/go/types/universe.go @@ -178,7 +178,7 @@ func DefPredeclaredTestFuncs() { func init() { Universe = NewScope(nil) - Unsafe = NewPackage("unsafe", "unsafe", NewScope(Universe)) + Unsafe = NewPackage("unsafe", "unsafe") Unsafe.complete = true defPredeclaredTypes()