diff --git a/go/types/api.go b/go/types/api.go index 41ad35ec..dbf145ec 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -162,11 +162,20 @@ type Info struct { // 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 + // InitOrder is the list of package-level initializers in the order in which + // they must be executed. Initializers referring to variables related by an + // initialization dependency appear in topological order, the others appear + // in source order. Variables without an initialization expression do not + // appear in this list. + InitOrder []*Initializer +} + +// An Initializer describes a package-level variable, or a list of variables in case +// of a multi-valued initialization expression, and the corresponding initialization +// expression. +type Initializer struct { + Lhs []*Var // var Lhs = Rhs + Rhs ast.Expr } // Check type-checks a package and returns the resulting package object, diff --git a/go/types/api_test.go b/go/types/api_test.go index 8cb955c2..193438a4 100644 --- a/go/types/api_test.go +++ b/go/types/api_test.go @@ -209,3 +209,50 @@ func TestScopesInfo(t *testing.T) { } } } + +func TestInitOrder(t *testing.T) { + var tests = []struct { + src string + vars [][]string + // TODO(gri) check also init expression + }{ + {`package p; var (x = 1; y = x)`, [][]string{{"x"}, {"y"}}}, + {`package p; var (a = 1; b = 2; c = 3)`, [][]string{{"a"}, {"b"}, {"c"}}}, + {`package p; var (a, b, c = 1, 2, 3)`, [][]string{{"a"}, {"b"}, {"c"}}}, + {`package p; var _ = f(); func f() int { return 1 }`, [][]string{{"_"}}}, // blank var + {`package p; var (a = 0; x = y; y = z; z = 0)`, [][]string{{"a"}, {"z"}, {"y"}, {"x"}}}, + {`package p; var (a, _ = m[0]; m map[int]string)`, [][]string{{"a", "_"}}}, // blank var + {`package p; var a, b = f(); func f() (_, _ int) { return z, z }; var z = 0`, [][]string{{"z"}, {"a", "b"}}}, + // TODO(gri) add more tests (incl. methods, closures, init functions) + } + + for i, test := range tests { + path := fmt.Sprintf("InitOrder%d", i) + info := Info{} + mustTypecheck(t, path, test.src, &info) + + // number of initializers must match + if len(info.InitOrder) != len(test.vars) { + t.Errorf("%s: got %d initializers; want %d", path, len(info.InitOrder), len(test.vars)) + continue + } + + // initializer order and initialized variables must match + for i, got := range info.InitOrder { + want := test.vars[i] + // number of variables must match + if len(got.Lhs) != len(want) { + t.Errorf("%s, %d.th initializer: got %d variables; want %d", path, i, len(got.Lhs), len(want)) + continue + } + + // variable names must match + for j, got := range got.Lhs { + want := want[j] + if got.name != want { + t.Errorf("%s, %d.th initializer: got name %s; want %s", path, i, got.name, want) + } + } + } + } +} diff --git a/go/types/check.go b/go/types/check.go index 5aab71d0..9bc7a471 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -34,20 +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) - 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 + 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 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 // 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 + 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 @@ -60,43 +60,32 @@ 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), - dependencies: make(map[Object]map[Object]bool), - untyped: make(map[ast.Expr]exprInfo), + conf: conf, + fset: fset, + pkg: pkg, + methods: make(map[string][]*Func), + conversions: make(map[*ast.CallExpr]bool), + untyped: make(map[ast.Expr]exprInfo), } } -func isVarOrFunc(obj Object) bool { - switch obj.(type) { - case *Var, *Func: - return true +// addDeclDep adds the dependency edge (check.decl -> to) +// if check.decl exists and to has an init expression. +func (check *checker) addDeclDep(to Object) { + from := check.decl + if from == nil { + return // not in a package-level init expression } - 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 + init := check.initMap[to] + if init == nil { + return // to does not have a package-level init expression } - - if debug { - assert(isVarOrFunc(from)) - assert(isVarOrFunc(to)) - } - - m := check.dependencies[from] // lazily allocated + m := from.deps if m == nil { - m = make(map[Object]bool) - check.dependencies[from] = m + m = make(map[Object]*declInfo) + from.deps = m } - m[to] = true + m[to] = init } func (check *checker) delay(f func()) { diff --git a/go/types/expr.go b/go/types/expr.go index b08d6eca..066b633d 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -969,7 +969,8 @@ func (check *checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { if sig, ok := check.typ(e.Type, nil, false).(*Signature); ok { x.mode = value x.typ = sig - check.later(nil, sig, e.Body) + // TODO(gri) provide correct *declInfo here + check.later(nil, nil, sig, e.Body) } else { check.invalidAST(e.Pos(), "invalid function literal %s", e) goto Error diff --git a/go/types/resolver.go b/go/types/resolver.go index a260bd01..b6c2c7b2 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -37,31 +37,27 @@ func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) { // A declInfo describes a package-level const, type, var, or func declaration. type declInfo struct { file *Scope // scope of file containing this declaration + lhs []*Var // lhs of n:1 variable declarations, or nil typ ast.Expr // type, or nil - init ast.Expr // initialization expression, or nil - fdecl *ast.FuncDecl // function declaration, or nil -} + init ast.Expr // init expression, or nil + fdecl *ast.FuncDecl // func declaration, or nil -// A multiExpr describes the lhs variables and a single but -// (expected to be) multi-valued rhs init expr of a variable -// declaration. -type multiExpr struct { - lhs []*Var - rhs []ast.Expr // len(rhs) == 1 - ast.Expr // dummy to satisfy ast.Expr interface + deps map[Object]*declInfo // init dependencies; lazily allocated + mark byte // see check.dependencies } type funcInfo struct { - obj *Func // for debugging/tracing only + obj *Func // for debugging/tracing only + info *declInfo // for cycle detection sig *Signature body *ast.BlockStmt } // later appends a function with non-empty body to check.funcList. -func (check *checker) later(f *Func, sig *Signature, body *ast.BlockStmt) { +func (check *checker) later(f *Func, info *declInfo, sig *Signature, body *ast.BlockStmt) { // functions implemented elsewhere (say in assembly) have no body if !check.conf.IgnoreFuncBodies && body != nil { - check.funcList = append(check.funcList, funcInfo{f, sig, body}) + check.funcList = append(check.funcList, funcInfo{f, info, sig, body}) } } @@ -107,14 +103,14 @@ func (check *checker) resolveFiles(files []*ast.File) { // base type names. var ( - fileScope *Scope // current file scope - objList []Object // list of package-level objects to type-check - objMap = make(map[Object]declInfo) // declaration info for each package-level object + objList []Object // list of package-level objects, in source order + objMap = make(map[Object]*declInfo) // corresponding declaration info + initMap = make(map[Object]*declInfo) // declaration info for variables with initializers ) // declare declares obj in the package scope, records its ident -> obj mapping, // and updates objList and objMap. The object must not be a function or method. - declare := func(ident *ast.Ident, obj Object, typ, init ast.Expr) { + 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 @@ -126,7 +122,7 @@ func (check *checker) resolveFiles(files []*ast.File) { check.declare(pkg.scope, ident, obj) objList = append(objList, obj) - objMap[obj] = declInfo{fileScope, typ, init, nil} + objMap[obj] = d } importer := check.conf.Import @@ -145,7 +141,7 @@ func (check *checker) resolveFiles(files []*ast.File) { // but there is no corresponding package object. check.recordObject(file.Name, nil) - fileScope = NewScope(pkg.scope) + fileScope := NewScope(pkg.scope) check.recordScope(file, fileScope) fileScopes = append(fileScopes, fileScope) dotImports = append(dotImports, nil) // element (map) is lazily allocated @@ -253,36 +249,46 @@ func (check *checker) resolveFiles(files []*ast.File) { init = last.Values[i] } - declare(name, obj, last.Type, init) + declare(name, obj, &declInfo{file: fileScope, typ: last.Type, init: init}) } check.arityMatch(s, last) case token.VAR: - // declare all variables lhs := make([]*Var, len(s.Names)) + // If there's exactly one rhs initializer, use + // the same declInfo d1 for all lhs variables + // so that each lhs variable depends on the same + // rhs initializer (n:1 var declaration). + var d1 *declInfo + if len(s.Values) == 1 { + // The lhs elements are only set up after the foor loop below, + // but that's ok because declareVar only collects the declInfo + // for a later phase. + d1 = &declInfo{file: fileScope, lhs: lhs, typ: s.Type, init: s.Values[0]} + } + + // declare all variables for i, name := range s.Names { obj := NewVar(name.Pos(), pkg, name.Name, nil) lhs[i] = obj + d := d1 var init ast.Expr - switch len(s.Values) { - case len(s.Names): - // lhs and rhs match - init = s.Values[i] - case 1: - // rhs must be a multi-valued expression - // (lhs may not be fully set up yet, but - // that's fine because declare simply collects - // the information for later processing.) - init = &multiExpr{lhs, s.Values, nil} - default: + if d == nil { + // individual assignments if i < len(s.Values) { init = s.Values[i] } + d = &declInfo{file: fileScope, typ: s.Type, init: init} } - declare(name, obj, s.Type, init) + declare(name, obj, d) + + // remember obj if it has an initializer + if d1 != nil || init != nil { + initMap[obj] = d + } } check.arityMatch(s, nil) @@ -293,7 +299,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, s.Type, nil) + declare(s.Name, obj, &declInfo{file: fileScope, typ: s.Type}) default: check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) @@ -333,7 +339,12 @@ func (check *checker) resolveFiles(files []*ast.File) { } } objList = append(objList, obj) - objMap[obj] = declInfo{fileScope, nil, nil, d} + info := &declInfo{file: fileScope, fdecl: d} + objMap[obj] = info + // remember obj if it has a body (= initializer) + if d.Body != nil { + initMap[obj] = info + } default: check.invalidAST(d.Pos(), "unknown ast.Decl node %T", d) @@ -354,6 +365,7 @@ 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 package-level declarations (objects may not have a type yet) + check.initMap = initMap for _, obj := range objList { if obj.Type() == nil { check.objDecl(obj, nil, false) @@ -387,6 +399,7 @@ func (check *checker) resolveFiles(files []*ast.File) { check.topScope = f.sig.scope // open function scope check.funcSig = f.sig check.hasLabel = false + check.decl = f.info check.stmtList(0, f.body.List) if check.hasLabel { @@ -402,13 +415,17 @@ func (check *checker) resolveFiles(files []*ast.File) { // Note: must happen after checking all functions because function bodies // may introduce dependencies - state := make(map[Object]uint8) + // For source order and reproducible error messages, + // iterate through objList rather than initMap. for _, obj := range objList { - if isVarOrFunc(obj) && state[obj] == 0 { - check.objDependencies(obj, state) + if obj, _ := obj.(*Var); obj != nil { + if init := initMap[obj]; init != nil { + check.dependencies(obj, init) + } } } - objList = nil // not needed anymore + objList = nil // not needed anymore + check.initMap = 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 @@ -473,23 +490,36 @@ func (check *checker) resolveFiles(files []*ast.File) { } } -func (check *checker) objDependencies(obj Object, state map[Object]uint8) { +func (check *checker) dependencies(obj Object, init *declInfo) { 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 + + if init.mark == done { return } - state[obj] = inProgress - for dep := range check.dependencies[obj] { - check.objDependencies(dep, state) + if init.mark == inProgress { + // cycle detected: end recursion and report error for variables + if obj, _ := obj.(*Var); obj != nil { + check.errorf(obj.pos, "initialization cycle for %s", obj.name) + // TODO(gri) print the cycle + init.mark = done // avoid further errors + } + return } - state[obj] = done - if v, _ := obj.(*Var); v != nil { - check.Info.InitOrder = append(check.Info.InitOrder, v) + init.mark = inProgress + for obj, dep := range init.deps { + check.dependencies(obj, dep) + } + init.mark = done + + // record the init order for variables + if lhs, _ := obj.(*Var); lhs != nil { + initLhs := init.lhs // possibly nil (see declInfo.lhs field comment) + if initLhs == nil { + initLhs = []*Var{lhs} + } + check.Info.InitOrder = append(check.Info.InitOrder, &Initializer{initLhs, init.init}) } } @@ -517,19 +547,25 @@ func (check *checker) objDecl(obj Object, def *Named, cycleOk bool) { oldIota := check.iota check.iota = nil + // save current decl + oldDecl := check.decl + check.decl = nil + switch obj := obj.(type) { case *Const: check.constDecl(obj, d.typ, d.init) case *Var: - check.varDecl(obj, d.typ, d.init) + check.decl = d // new package-level var decl + check.varDecl(obj, d.lhs, d.typ, d.init) case *TypeName: check.typeDecl(obj, d.typ, def, cycleOk) case *Func: - check.funcDecl(obj, d.fdecl) + check.funcDecl(obj, d) default: unreachable() } + check.decl = oldDecl check.iota = oldIota check.topScope = oldScope } @@ -561,7 +597,8 @@ func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { check.iota = nil } -func (check *checker) varDecl(obj *Var, typ, init ast.Expr) { +// TODO(gri) document arguments +func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) { if obj.visited { check.errorf(obj.Pos(), "illegal cycle in initialization of variable %s", obj.name) obj.typ = Typ[Invalid] @@ -586,19 +623,28 @@ 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) - } else { + if lhs == nil || len(lhs) == 1 { + assert(lhs == nil || lhs[0] == obj) var x operand check.expr(&x, init) check.initVar(obj, &x) + return } - check.init = oldInit + if debug { + // obj must be one of lhs + found := false + for _, lhs := range lhs { + if obj == lhs { + found = true + break + } + } + if !found { + panic("inconsistent lhs") + } + } + check.initVars(lhs, []ast.Expr{init}, token.NoPos) } func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk bool) { @@ -696,11 +742,12 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk delete(check.methods, obj.name) // we don't need them anymore } -func (check *checker) funcDecl(obj *Func, fdecl *ast.FuncDecl) { +func (check *checker) funcDecl(obj *Func, info *declInfo) { // func declarations cannot use iota assert(check.iota == nil) obj.typ = Typ[Invalid] // guard against cycles + fdecl := info.fdecl sig := check.funcType(fdecl.Recv, fdecl.Type, nil) 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") @@ -708,7 +755,7 @@ func (check *checker) funcDecl(obj *Func, fdecl *ast.FuncDecl) { } obj.typ = sig - check.later(obj, sig, fdecl.Body) + check.later(obj, info, sig, fdecl.Body) } func (check *checker) declStmt(decl ast.Decl) { @@ -754,15 +801,14 @@ func (check *checker) declStmt(decl ast.Decl) { } case token.VAR: - // For varDecl called with a multiExpr we need the fully - // initialized lhs. Compute it in a separate pre-pass. - lhs := make([]*Var, len(s.Names)) + lhs0 := make([]*Var, len(s.Names)) for i, name := range s.Names { - lhs[i] = NewVar(name.Pos(), pkg, name.Name, nil) + lhs0[i] = NewVar(name.Pos(), pkg, name.Name, nil) } - // declare all variables - for i, obj := range lhs { + // initialize all variables + for i, obj := range lhs0 { + var lhs []*Var var init ast.Expr switch len(s.Values) { case len(s.Names): @@ -770,20 +816,22 @@ func (check *checker) declStmt(decl ast.Decl) { init = s.Values[i] case 1: // rhs is expected to be a multi-valued expression - init = &multiExpr{lhs, s.Values, nil} + lhs = lhs0 + init = s.Values[0] default: if i < len(s.Values) { init = s.Values[i] } } - - check.varDecl(obj, s.Type, init) + check.varDecl(obj, lhs, s.Type, init) } check.arityMatch(s, nil) + // declare all variables + // (only at this point are the variable scopes (parents) set) for i, name := range s.Names { - check.declare(check.topScope, name, lhs[i]) + check.declare(check.topScope, name, lhs0[i]) } default: diff --git a/go/types/testdata/init0.src b/go/types/testdata/init0.src index c72d30de..c4d9bfe5 100644 --- a/go/types/testdata/init0.src +++ b/go/types/testdata/init0.src @@ -36,7 +36,11 @@ type S1 struct { var x3 /* ERROR initialization cycle */ S1 = S1{x3.f} // cycles via functions -var x4 = f1() // TODO(gri) recognize cycle -func f1() int { - return x4 -} +var x4 = x5 +var x5 /* ERROR initialization cycle */ = f1() +func f1() int { return x5*10 } + +var x6, x7 /* ERROR initialization cycle */ = f2() +var x8 = x7 +func f2() (int, int) { return f3() + f3(), 0 } +func f3() int { return x8 } diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 281fab0f..20b86f9f 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -89,15 +89,16 @@ 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) + if typ != Typ[Invalid] { + check.addDeclDep(obj) } case *Func: obj.used = true x.mode = value + if typ != Typ[Invalid] { + check.addDeclDep(obj) + } case *Builtin: obj.used = true // for built-ins defined by package unsafe