From d259ea48d140dfcd518f9c17c5ded1e6d5cebe9e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 31 Oct 2013 21:47:35 -0700 Subject: [PATCH] go.tools/go/types: first cut at checking variable initialization cycles Missing: - dependencies via functions (incl. closures and methods) - more tests (incl. API test) R=adonovan CC=golang-dev https://golang.org/cl/20510043 --- go/types/api.go | 6 ++++ go/types/check.go | 62 ++++++++++++++++++++++++++++++------- go/types/check_test.go | 1 + go/types/resolver.go | 54 ++++++++++++++++++++++++++------ go/types/testdata/init0.src | 42 +++++++++++++++++++++++++ go/types/typexpr.go | 5 +++ 6 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 go/types/testdata/init0.src diff --git a/go/types/api.go b/go/types/api.go index 3f4c9967..41ad35ec 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -161,6 +161,12 @@ type Info struct { // *ast.RangeStmt // Scopes map[ast.Node]*Scope + + // InitOrder is the list of package-level variables in the order in which + // they must be initialized. Variables related by an initialization dependency + // appear in topological order, and unrelated variables appear in source order. + // TODO(gri) find a better name + InitOrder []*Var } // Check type-checks a package and returns the resulting package object, diff --git a/go/types/check.go b/go/types/check.go index e471e06f..5aab71d0 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -34,18 +34,20 @@ type checker struct { fset *token.FileSet pkg *Package - 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 - lhsVarsList [][]*Var // type switch lhs variable sets, for 'declared but not used' errors - delayed []func() // delayed checks that require fully setup types + methods map[string][]*Func // maps type names to associated methods + conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls) + dependencies map[Object]map[Object]bool // set of object initialization dependencies (obj depends on {obj1, obj2, ...}) + untyped map[ast.Expr]exprInfo // map of expressions without final type + lhsVarsList [][]*Var // type switch lhs variable sets, for 'declared but not used' errors + 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) topScope *Scope // current topScope for lookups - iota exact.Value // value of iota in a constant declaration; nil otherwise + iota exact.Value // current value of iota in a constant declaration; nil otherwise + init Object // current variable or function for which init expression or body is type-checked // functions funcList []funcInfo // list of functions/methods with correct signatures and non-empty bodies @@ -58,15 +60,45 @@ type checker struct { 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), + conf: conf, + fset: fset, + pkg: pkg, + methods: make(map[string][]*Func), + conversions: make(map[*ast.CallExpr]bool), + dependencies: make(map[Object]map[Object]bool), + untyped: make(map[ast.Expr]exprInfo), } } +func isVarOrFunc(obj Object) bool { + switch obj.(type) { + case *Var, *Func: + return true + } + return false +} + +// addInitDep adds the dependency edge (to -> check.init) +// if check.init is a package-level variable or function. +func (check *checker) addInitDep(to Object) { + from := check.init + if from == nil || from.Parent() == nil { + return // not a package-level variable or function + } + + if debug { + assert(isVarOrFunc(from)) + assert(isVarOrFunc(to)) + } + + m := check.dependencies[from] // lazily allocated + if m == nil { + m = make(map[Object]bool) + check.dependencies[from] = m + } + m[to] = true +} + func (check *checker) delay(f func()) { check.delayed = append(check.delayed, f) } @@ -227,5 +259,11 @@ func (conf *Config) check(pkgPath string, fset *token.FileSet, files []*ast.File } } + // 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/check_test.go b/go/types/check_test.go index befeee21..bc95a48e 100644 --- a/go/types/check_test.go +++ b/go/types/check_test.go @@ -52,6 +52,7 @@ var tests = [][]string{ {"testdata/cycles2.src"}, {"testdata/cycles3.src"}, {"testdata/cycles4.src"}, + {"testdata/init0.src"}, {"testdata/decls0.src"}, {"testdata/decls1.src"}, {"testdata/decls2a.src", "testdata/decls2b.src"}, diff --git a/go/types/resolver.go b/go/types/resolver.go index 2e19c218..a260bd01 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -353,16 +353,13 @@ func (check *checker) resolveFiles(files []*ast.File) { // Phase 3: Typecheck all objects in objList, but not function bodies. - check.objMap = objMap // indicate that we are checking global declarations (objects may not have a type yet) + check.objMap = objMap // indicate that we are checking package-level declarations (objects may not have a type yet) for _, obj := range objList { if obj.Type() == nil { check.objDecl(obj, nil, false) } } - - // done with package-level declarations - check.objMap = nil - objList = nil + check.objMap = nil // done with package-level declarations // 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 @@ -401,7 +398,19 @@ func (check *checker) resolveFiles(files []*ast.File) { } } - // Phase 5: Check for declared but not used packages and variables. + // Phase 5: Check initialization dependencies. + // Note: must happen after checking all functions because function bodies + // may introduce dependencies + + state := make(map[Object]uint8) + for _, obj := range objList { + if isVarOrFunc(obj) && state[obj] == 0 { + check.objDependencies(obj, state) + } + } + objList = nil // not needed anymore + + // Phase 6: Check for declared but not used packages and variables. // Note: must happen after checking all functions because closures may affect outer scopes // spec: "It is illegal (...) to directly import a package without referring to @@ -464,6 +473,26 @@ func (check *checker) resolveFiles(files []*ast.File) { } } +func (check *checker) objDependencies(obj Object, state map[Object]uint8) { + const inProgress, done = 1, 2 + if state[obj] == inProgress { + check.errorf(obj.Pos(), "initialization cycle for %s", obj.Name()) + // TODO(gri) print the cycle + state[obj] = done // avoid further errors + return + } + + state[obj] = inProgress + for dep := range check.dependencies[obj] { + check.objDependencies(dep, state) + } + state[obj] = done + + if v, _ := obj.(*Var); v != nil { + check.Info.InitOrder = append(check.Info.InitOrder, v) + } +} + func (check *checker) usage(scope *Scope) { for _, obj := range scope.elems { if v, _ := obj.(*Var); v != nil && !v.used { @@ -557,14 +586,19 @@ func (check *checker) varDecl(obj *Var, typ, init ast.Expr) { return } + // save current init object + oldInit := check.init + check.init = obj + if m, _ := init.(*multiExpr); m != nil { check.initVars(m.lhs, m.rhs, token.NoPos) - return + } else { + var x operand + check.expr(&x, init) + check.initVar(obj, &x) } - var x operand - check.expr(&x, init) - check.initVar(obj, &x) + check.init = oldInit } func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk bool) { diff --git a/go/types/testdata/init0.src b/go/types/testdata/init0.src new file mode 100644 index 00000000..c72d30de --- /dev/null +++ b/go/types/testdata/init0.src @@ -0,0 +1,42 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// initialization cycles + +package init0 + +// type-checking, not initialization cycles (we don't know the types) +// (avoid duplicate errors) +var ( + x1 /* ERROR cycle */ = y1 + y1 = x1 + + a1 = b1 + b1 /* ERROR cycle */ = c1 + c1 = d1 + d1 = b1 +) + +// initialization cycles (we know the types) +var ( + x2 /* ERROR initialization cycle */ int = y2 + y2 = x2 + + a2 = b2 + b2 /* ERROR initialization cycle */ int = c2 + c2 = d2 + d2 = b2 +) + +// cycles via struct fields +type S1 struct { + f int +} +var x3 /* ERROR initialization cycle */ S1 = S1{x3.f} + +// cycles via functions +var x4 = f1() // TODO(gri) recognize cycle +func f1() int { + return x4 +} diff --git a/go/types/typexpr.go b/go/types/typexpr.go index b9d71c42..281fab0f 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -89,6 +89,11 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) case *Var: obj.used = true x.mode = variable + if obj.parent.parent == Universe && typ != Typ[Invalid] { + // obj is a package-level variable, and there are no other errors + // (such as a type-checking cycle) + check.addInitDep(obj) + } case *Func: obj.used = true