From 20efc5ba73634062f13f434a862ad72f2ffcf552 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 3 Feb 2014 16:02:56 -0800 Subject: [PATCH] go.tools/go/types: use init cycle tracking for all const and var cycles Also rename types.Assertable -> types.AssertableTo per adonovan. LGTM=adonovan R=adonovan CC=golang-codereviews https://golang.org/cl/59680044 --- cmd/vet/types.go | 2 +- go/types/api.go | 4 ++-- go/types/decl.go | 16 +++++++------ go/types/resolver.go | 38 ++++++++++++++++++------------- go/types/testdata/const0.src | 4 ++-- go/types/testdata/init0.src | 44 ++++++++++++++++++++++++++++-------- go/types/typexpr.go | 11 ++++----- 7 files changed, 74 insertions(+), 45 deletions(-) diff --git a/cmd/vet/types.go b/cmd/vet/types.go index 8149a4ef..10fdaf47 100644 --- a/cmd/vet/types.go +++ b/cmd/vet/types.go @@ -103,7 +103,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp } // If we can use a string, might arg (dynamically) implement the Stringer or Error interface? if t&argString != 0 { - if types.Assertable(errorType, typ) || types.Assertable(stringerType, typ) { + if types.AssertableTo(errorType, typ) || types.AssertableTo(stringerType, typ) { return true } } diff --git a/go/types/api.go b/go/types/api.go index 3e516254..0dfc083a 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -226,8 +226,8 @@ func (conf *Config) Check(path string, fset *token.FileSet, files []*ast.File, i return pkg, err } -// Assertable reports whether a value of type V can be asserted to have type T. -func Assertable(V *Interface, T Type) bool { +// AssertableTo reports whether a value of type V can be asserted to have type T. +func AssertableTo(V *Interface, T Type) bool { f, _ := MissingMethod(T, V, false) return f == nil } diff --git a/go/types/decl.go b/go/types/decl.go index e78dea69..5b88f8cc 100644 --- a/go/types/decl.go +++ b/go/types/decl.go @@ -64,15 +64,23 @@ func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { oldDecl := check.decl check.decl = nil + // Const and var declarations must not have initialization + // cycles. We track them by remembering the current declaration + // in check.decl. Initialization expressions depending on other + // consts, vars, or functions, add dependencies to the current + // check.decl. switch obj := obj.(type) { case *Const: + check.decl = d // new package-level const decl check.constDecl(obj, d.typ, d.init) case *Var: check.decl = d // new package-level var decl check.varDecl(obj, d.lhs, d.typ, d.init) case *TypeName: + // invalid recursive types are detected via path check.typeDecl(obj, d.typ, def, path) case *Func: + // functions may be recursive - no need to track dependencies check.funcDecl(obj, d) default: unreachable() @@ -86,9 +94,7 @@ func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { assert(obj.typ == nil) - // TODO(gri) Instead of this code we should rely on initialization cycle detection. if obj.visited { - check.errorf(obj.Pos(), "illegal cycle in initialization of constant %s", obj.name) obj.typ = Typ[Invalid] return } @@ -97,6 +103,7 @@ func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { // use the correct value of iota assert(check.iota == nil) check.iota = obj.val + defer func() { check.iota = nil }() // determine type, if any if typ != nil { @@ -104,7 +111,6 @@ func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { if !isConstType(t) { check.errorf(typ.Pos(), "invalid constant type %s", t) obj.typ = Typ[Invalid] - check.iota = nil return } obj.typ = t @@ -116,16 +122,12 @@ func (check *checker) constDecl(obj *Const, typ, init ast.Expr) { check.expr(&x, init) } check.initConst(obj, &x) - - check.iota = nil } func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) { assert(obj.typ == nil) - // TODO(gri) Instead of this code we should rely on initialization cycle detection. if obj.visited { - check.errorf(obj.Pos(), "illegal cycle in initialization of variable %s", obj.name) obj.typ = Typ[Invalid] return } diff --git a/go/types/resolver.go b/go/types/resolver.go index ae6361c1..b05411a0 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -89,7 +89,7 @@ func (check *checker) resolveFiles(files []*ast.File) { var ( objMap = make(map[Object]*declInfo) // corresponding declaration info - initMap = make(map[Object]*declInfo) // declaration info for variables with initializers + initMap = make(map[Object]*declInfo) // declaration info for objects with initializers ) // declare declares obj in the package scope, records its ident -> obj mapping, @@ -239,7 +239,11 @@ func (check *checker) resolveFiles(files []*ast.File) { init = last.Values[i] } - declare(name, obj, &declInfo{file: fileScope, typ: last.Type, init: init}) + d := &declInfo{file: fileScope, typ: last.Type, init: init} + declare(name, obj, d) + + // all constants have an initializer + initMap[obj] = d } check.arityMatch(s, last) @@ -275,7 +279,7 @@ func (check *checker) resolveFiles(files []*ast.File) { declare(name, obj, d) - // remember obj if it has an initializer + // remember variable if it has an initializer if d1 != nil || init != nil { initMap[obj] = d } @@ -330,7 +334,7 @@ func (check *checker) resolveFiles(files []*ast.File) { } info := &declInfo{file: fileScope, fdecl: d} objMap[obj] = info - // remember obj if it has a body (= initializer) + // remember function if it has a body (= initializer) if d.Body != nil { initMap[obj] = info } @@ -355,12 +359,11 @@ func (check *checker) resolveFiles(files []*ast.File) { // Phase 3: Typecheck all objects in objMap, but not function bodies. check.initMap = initMap - check.objMap = objMap // indicate that we are checking package-level declarations (objects may not have a type yet) - emptyCycle := make([]*TypeName, 0, 8) // re-use the same underlying array for cycle detection + check.objMap = objMap // indicate that we are checking package-level declarations (objects may not have a type yet) + 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, emptyCycle) + check.objDecl(obj, nil, typePath) } - emptyCycle = nil // not needed anymore check.objMap = nil // not needed anymore // At this point we may have a non-empty check.methods map; this means that not all @@ -380,11 +383,12 @@ func (check *checker) resolveFiles(files []*ast.File) { // Note: must happen after checking all functions because function bodies // may introduce dependencies + initPath := make([]Object, 0, 8) // pre-allocate space for initialization paths so that the underlying array is reused for _, obj := range objectsOf(initMap) { - if obj, _ := obj.(*Var); obj != nil { + switch obj.(type) { + case *Const, *Var: if init := initMap[obj]; init != nil { - // provide non-nil path for re-use of underlying array - check.dependencies(obj, init, make([]Object, 0, 8)) + check.dependencies(obj, init, initPath) } } } @@ -488,16 +492,18 @@ func (check *checker) dependencies(obj Object, init *declInfo, path []Object) { } if init.mark > 0 { - // cycle detected - find index of first variable in cycle, if any + // cycle detected - find index of first constant or variable in cycle, if any first := -1 cycle := path[init.mark-1:] + L: for i, obj := range cycle { - if _, ok := obj.(*Var); ok { + switch obj.(type) { + case *Const, *Var: first = i - break + break L } } - // only report an error if there's at least one variable + // only report an error if there's at least one constant or variable if first >= 0 { obj := cycle[first] check.errorf(obj.Pos(), "initialization cycle for %s", obj.Name()) @@ -528,7 +534,7 @@ func (check *checker) dependencies(obj Object, init *declInfo, path []Object) { } init.mark = -1 // init.mark < 0 - // record the init order for variables + // record the init order for variables only if this, _ := obj.(*Var); this != nil { initLhs := init.lhs // possibly nil (see declInfo.lhs field comment) if initLhs == nil { diff --git a/go/types/testdata/const0.src b/go/types/testdata/const0.src index 1a0dd42d..c4419ab6 100644 --- a/go/types/testdata/const0.src +++ b/go/types/testdata/const0.src @@ -179,8 +179,8 @@ const ( // initialization cycles const ( - a /* ERROR "cycle" */ = a - b /* ERROR "cycle" */ , c /* ERROR "cycle" */, d, e = e, d, c, b // TODO(gri) should only have one cycle error + a /* ERROR "initialization cycle" */ = a + b /* ERROR "initialization cycle" */ , c /* ERROR "initialization cycle" */, d, e = e, d, c, b // TODO(gri) should only have one cycle error f float64 = d ) diff --git a/go/types/testdata/init0.src b/go/types/testdata/init0.src index 9fe0571e..9467daf8 100644 --- a/go/types/testdata/init0.src +++ b/go/types/testdata/init0.src @@ -6,23 +6,34 @@ package init0 -// type-checking, not initialization cycles (we don't know the types) -// (avoid duplicate errors) -var ( - x0 /* ERROR cycle */ = x0 +// initialization cycles (we don't know the types) +const ( + s0 /* ERROR initialization cycle */ = s0 - x1 /* ERROR cycle */ = y1 + x0 /* ERROR initialization cycle */ = y0 + y0 = x0 + + a0 = b0 + b0 /* ERROR initialization cycle */ = c0 + c0 = d0 + d0 = b0 +) + +var ( + s1 /* ERROR initialization cycle */ = s1 + + x1 /* ERROR initialization cycle */ = y1 y1 = x1 a1 = b1 - b1 /* ERROR cycle */ = c1 + b1 /* ERROR initialization cycle */ = c1 c1 = d1 d1 = b1 ) // initialization cycles (we know the types) -var ( - x00 /* ERROR initialization cycle */ int = x00 +const ( + s2 /* ERROR initialization cycle */ int = s2 x2 /* ERROR initialization cycle */ int = y2 y2 = x2 @@ -33,12 +44,25 @@ var ( d2 = b2 ) +var ( + s3 /* ERROR initialization cycle */ int = s3 + + x3 /* ERROR initialization cycle */ int = y3 + y3 = x3 + + a3 = b3 + b3 /* ERROR initialization cycle */ int = c3 + c3 = d3 + d3 = b3 +) + // cycles via struct fields type S1 struct { f int } -var x3 /* ERROR initialization cycle */ S1 = S1{x3.f} +const cx3 S1 /* ERROR invalid constant type */ = S1{cx3.f} +var vx3 /* ERROR initialization cycle */ S1 = S1{vx3.f} // cycles via functions @@ -53,7 +77,7 @@ func f3() int { return x8 } // cycles via closures -var x9 /* ERROR illegal cycle */ = func() int { return x9 }() +var x9 /* ERROR initialization cycle */ = func() int { return x9 }() var x10 /* ERROR initialization cycle */ = f4() diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 9a1d5572..6945d253 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -50,6 +50,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa // (This code is only used for dot-imports. Without them, we // would only have to mark Vars.) obj.used = true + check.addDeclDep(obj) if typ == Typ[Invalid] { return } @@ -87,22 +88,18 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa case *Var: obj.used = true + check.addDeclDep(obj) x.mode = variable - if typ != Typ[Invalid] { - check.addDeclDep(obj) - } case *Func: obj.used = true + check.addDeclDep(obj) x.mode = value - if typ != Typ[Invalid] { - check.addDeclDep(obj) - } case *Builtin: obj.used = true // for built-ins defined by package unsafe - x.mode = builtin x.id = obj.id + x.mode = builtin case *Nil: // no need to "use" the nil object