From 3a75f78acd3bf2dd3abe9072d2de9410a77e339f Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 10 Jul 2013 20:10:24 -0700 Subject: [PATCH] go.tools/go/types: cleanup init expr checks - factored our arity checks - started more systematic tests for const/var decls - better error messages - fixed several corner case errors R=adonovan CC=golang-dev https://golang.org/cl/11137043 --- go/types/check_test.go | 2 + go/types/operand.go | 7 +- go/types/resolver.go | 145 +++++++++++++++----------------- go/types/testdata/const0.src | 8 +- go/types/testdata/constdecl.src | 85 +++++++++++++++++++ go/types/testdata/decls1.src | 8 +- go/types/testdata/vardecl.src | 44 ++++++++++ 7 files changed, 209 insertions(+), 90 deletions(-) create mode 100644 go/types/testdata/constdecl.src create mode 100644 go/types/testdata/vardecl.src diff --git a/go/types/check_test.go b/go/types/check_test.go index 21ad03c5..57696a8d 100644 --- a/go/types/check_test.go +++ b/go/types/check_test.go @@ -51,6 +51,8 @@ var tests = [][]string{ {"testdata/decls2a.src", "testdata/decls2b.src"}, {"testdata/decls3.src"}, {"testdata/const0.src"}, + {"testdata/constdecl.src"}, + {"testdata/vardecl.src"}, {"testdata/expr0.src"}, {"testdata/expr1.src"}, {"testdata/expr2.src"}, diff --git a/go/types/operand.go b/go/types/operand.go index cc74be64..f42fc7b0 100644 --- a/go/types/operand.go +++ b/go/types/operand.go @@ -19,14 +19,14 @@ import ( type operandMode int const ( - invalid operandMode = iota // operand is invalid (due to an earlier error) - ignore + invalid operandMode = iota // operand is invalid novalue // operand represents no value (result of a function call w/o result) typexpr // operand is a type typexprn // like typexpr; only used to communicate between checker.expr0 and checker.rawExpr constant // operand is a constant; the operand's typ is a Basic type variable // operand is an addressable variable value // operand is a computed value - valueok // like mode == value, but operand may be used in a comma,ok expression + valueok // like value, but operand may be used in a comma,ok expression ) var operandModeString = [...]string{ @@ -42,7 +42,8 @@ var operandModeString = [...]string{ // An operand represents an intermediate value during type checking. // Operands have an (addressing) mode, the expression evaluating to -// the operand, the operand's type, and for constants a constant value. +// the operand, the operand's type, and a value if mode == constant. +// The zero value of operand is a ready to use invalid operand. // type operand struct { mode operandMode diff --git a/go/types/resolver.go b/go/types/resolver.go index f25216dd..ec41215b 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -49,6 +49,38 @@ type projExpr struct { ast.Expr // rhs } +// arityMatch checks that the lhs and rhs of a const or var decl +// have the appropriate number of names and init exprs. For const +// decls, init is the value spec providing the init exprs; for +// var decls, init is nil (the init exprs are in s in this case). +func (check *checker) arityMatch(s, init *ast.ValueSpec) { + l := len(s.Names) + r := len(s.Values) + if init != nil { + r = len(init.Values) + } + + switch { + case init == nil && r == 0: + // var decl w/o init expr + if s.Type == nil { + check.errorf(s.Pos(), "missing type or init expr") + } + case l < r: + if l < len(s.Values) { + // init exprs from s + n := s.Values[l] + check.errorf(n.Pos(), "extra init expr %s", n) + } else { + // init exprs "inherited" + check.errorf(s.Pos(), "extra init expr at %s", init.Pos()) + } + case l > r && (init != nil || r != 1): + n := s.Names[r] + check.errorf(n.Pos(), "missing init expr for %s", n) + } +} + func (check *checker) resolveFiles(files []*ast.File, importer Importer) { pkg := check.pkg @@ -100,7 +132,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { // ignore case *ast.GenDecl: - var last *ast.ValueSpec // last list of const initializers seen + var last *ast.ValueSpec // last ValueSpec with type or init exprs seen for iota, spec := range d.Specs { switch s := spec.(type) { case *ast.ImportSpec: @@ -110,7 +142,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { path, _ := strconv.Unquote(s.Path.Value) imp, err := importer(pkg.imports, path) if imp == nil && err == nil { - err = errors.New("Context.Import returned niil") + err = errors.New("Context.Import returned nil") } if err != nil { check.errorf(s.Path.Pos(), "could not import %s (%s)", path, err) @@ -157,8 +189,11 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { switch d.Tok { case token.CONST: // determine which initialization expressions to use - if len(s.Values) > 0 { + switch { + case s.Type != nil || len(s.Values) > 0: last = s + case last == nil: + last = new(ast.ValueSpec) // make sure last exists } // declare all constants @@ -173,17 +208,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { declare(name, obj, last.Type, init) } - // arity of lhs and rhs must match - if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { - switch { - case lhs < rhs: - x := s.Values[lhs] - check.errorf(x.Pos(), "too many initialization expressions") - case lhs > rhs && rhs != 1: - n := s.Names[rhs] - check.errorf(n.Pos(), "missing initialization expression for %s", n) - } - } + check.arityMatch(s, last) case token.VAR: // declare all variables @@ -199,6 +224,9 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { 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 = &projExpr{lhs, s.Values[0]} default: if i < len(s.Values) { @@ -209,17 +237,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { declare(name, obj, s.Type, init) } - // report if there are too many initialization expressions - if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { - switch { - case lhs < rhs: - x := s.Values[lhs] - check.errorf(x.Pos(), "too many initialization expressions") - case lhs > rhs && rhs != 1: - n := s.Names[rhs] - check.errorf(n.Pos(), "missing initialization expression for %s", n) - } - } + check.arityMatch(s, nil) default: check.invalidAST(s.Pos(), "invalid token %s", d.Tok) @@ -377,27 +395,13 @@ func (check *checker) declareConst(obj *Const, typ, init ast.Expr) { obj.typ = check.typ(typ, nil, false) } + // check initialization var x operand - - if init == nil { - goto Error // error reported before + if init != nil { + check.expr(&x, init) } - - check.expr(&x, init) - if x.mode == invalid { - goto Error - } - check.initConst(obj, &x) - check.iota = nil - return -Error: - if obj.typ == nil { - obj.typ = Typ[Invalid] - } else { - obj.val = exact.MakeUnknown() - } check.iota = nil } @@ -602,56 +606,49 @@ func (check *checker) declStmt(decl ast.Decl) { // ignore case *ast.GenDecl: - var last []ast.Expr // last list of const initializers seen + var last *ast.ValueSpec // last ValueSpec with type or init exprs seen for iota, spec := range d.Specs { switch s := spec.(type) { case *ast.ValueSpec: switch d.Tok { case token.CONST: - // determine which initialization expressions to use - if len(s.Values) > 0 { - last = s.Values + // determine which init exprs to use + switch { + case s.Type != nil || len(s.Values) > 0: + last = s + case last == nil: + last = new(ast.ValueSpec) // make sure last exists } // declare all constants lhs := make([]*Const, len(s.Names)) for i, name := range s.Names { obj := NewConst(name.Pos(), pkg, name.Name, nil, exact.MakeInt64(int64(iota))) - check.callIdent(name, obj) lhs[i] = obj var init ast.Expr - if i < len(last) { - init = last[i] + if i < len(last.Values) { + init = last.Values[i] } - check.declareConst(obj, s.Type, init) + check.declareConst(obj, last.Type, init) } - // arity of lhs and rhs must match - switch lhs, rhs := len(s.Names), len(last); { - case lhs < rhs: - x := last[lhs] - check.errorf(x.Pos(), "too many initialization expressions") - case lhs > rhs: - n := s.Names[rhs] - check.errorf(n.Pos(), "missing initialization expression for %s", n) - } + check.arityMatch(s, last) - for _, obj := range lhs { - check.declare(check.topScope, nil, obj) + for i, name := range s.Names { + check.declare(check.topScope, name, lhs[i]) } case token.VAR: - // declare all variables + // For declareVar called with a projExpr we need the fully + // initialized lhs. Compute it in a separate pre-pass. lhs := make([]*Var, len(s.Names)) for i, name := range s.Names { - obj := NewVar(name.Pos(), pkg, name.Name, nil) - check.callIdent(name, obj) - lhs[i] = obj + lhs[i] = NewVar(name.Pos(), pkg, name.Name, nil) } - // iterate in 2 phases because declareVar requires fully initialized lhs! + // declare all variables for i, obj := range lhs { var init ast.Expr switch len(s.Values) { @@ -670,20 +667,10 @@ func (check *checker) declStmt(decl ast.Decl) { check.declareVar(obj, s.Type, init) } - // arity of lhs and rhs must match - if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { - switch { - case lhs < rhs: - x := s.Values[lhs] - check.errorf(x.Pos(), "too many initialization expressions") - case lhs > rhs && rhs != 1: - n := s.Names[rhs] - check.errorf(n.Pos(), "missing initialization expression for %s", n) - } - } + check.arityMatch(s, nil) - for _, obj := range lhs { - check.declare(check.topScope, nil, obj) + for i, name := range s.Names { + check.declare(check.topScope, name, lhs[i]) } default: diff --git a/go/types/testdata/const0.src b/go/types/testdata/const0.src index 71c6664c..6bf50c34 100644 --- a/go/types/testdata/const0.src +++ b/go/types/testdata/const0.src @@ -180,8 +180,8 @@ const ( const ( a1, a2, a3 = 7, 3.1415926, "foo" b1, b2, b3 = b3, b1, 42 - c1, c2, c3 /* ERROR "missing initialization" */ = 1, 2 - d1, d2, d3 = 1, 2, 3, 4 /* ERROR "too many initialization" */ + c1, c2, c3 /* ERROR "missing init expr for c3" */ = 1, 2 + d1, d2, d3 = 1, 2, 3, 4 /* ERROR "extra init expr 4" */ _p0 = assert(a1 == 7) _p1 = assert(a2 == 3.1415926) _p2 = assert(a3 == "foo") @@ -194,8 +194,8 @@ func _() { const ( a1, a2, a3 = 7, 3.1415926, "foo" b1, b2, b3 = b3, b1, 42 - c1, c2, c3 /* ERROR "missing initialization" */ = 1, 2 - d1, d2, d3 = 1, 2, 3, 4 /* ERROR "too many initialization" */ + c1, c2, c3 /* ERROR "missing init expr for c3" */ = 1, 2 + d1, d2, d3 = 1, 2, 3, 4 /* ERROR "extra init expr 4" */ _p0 = assert(a1 == 7) _p1 = assert(a2 == 3.1415926) _p2 = assert(a3 == "foo") diff --git a/go/types/testdata/constdecl.src b/go/types/testdata/constdecl.src new file mode 100644 index 00000000..0f71353b --- /dev/null +++ b/go/types/testdata/constdecl.src @@ -0,0 +1,85 @@ +// 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. + +package constdecl + +import "math" + +var v int + +// Const decls must be initialized by constants. +const _ = v /* ERROR "not constant" */ +const _ = math /* ERROR "not constant" */ .Sin(0) +const _ = int /* ERROR "not an expression" */ + +func _() { + const _ = v /* ERROR "not constant" */ + const _ = math /* ERROR "not constant" */ .Sin(0) + const _ = int /* ERROR "not an expression" */ +} + +// Identifier and expression arity must match. +const _ /* ERROR "missing init expr for _" */ +const _ = 1, 2 /* ERROR "extra init expr 2" */ + +const _ /* ERROR "missing init expr for _" */ int +const _ int = 1, 2 /* ERROR "extra init expr 2" */ + +const ( + _ /* ERROR "missing init expr for _" */ + _ = 1, 2 /* ERROR "extra init expr 2" */ + + _ /* ERROR "missing init expr for _" */ int + _ int = 1, 2 /* ERROR "extra init expr 2" */ +) + +const ( + _ = 1 + _ + _, _ /* ERROR "missing init expr for _" */ + _ +) + +const ( + _, _ = 1, 2 + _, _ + _ /* ERROR "extra init expr at" */ + _, _ + _, _, _ /* ERROR "missing init expr for _" */ + _, _ +) + +func _() { + const _ /* ERROR "missing init expr for _" */ + const _ = 1, 2 /* ERROR "extra init expr 2" */ + + const _ /* ERROR "missing init expr for _" */ int + const _ int = 1, 2 /* ERROR "extra init expr 2" */ + + const ( + _ /* ERROR "missing init expr for _" */ + _ = 1, 2 /* ERROR "extra init expr 2" */ + + _ /* ERROR "missing init expr for _" */ int + _ int = 1, 2 /* ERROR "extra init expr 2" */ + ) + + const ( + _ = 1 + _ + _, _ /* ERROR "missing init expr for _" */ + _ + ) + + const ( + _, _ = 1, 2 + _, _ + _ /* ERROR "extra init expr at" */ + _, _ + _, _, _ /* ERROR "missing init expr for _" */ + _, _ + ) +} + +// TODO(gri) move extra tests from testdata/const0.src into here \ No newline at end of file diff --git a/go/types/testdata/decls1.src b/go/types/testdata/decls1.src index a45c2ad7..d902b577 100644 --- a/go/types/testdata/decls1.src +++ b/go/types/testdata/decls1.src @@ -100,15 +100,15 @@ var ( // Multiple assignment expressions var ( m1a, m1b = 1, 2 - m2a, m2b, m2c /* ERROR "missing initialization" */ = 1, 2 - m3a, m3b = 1, 2, 3 /* ERROR "too many initialization" */ + m2a, m2b, m2c /* ERROR "missing init expr for m2c" */ = 1, 2 + m3a, m3b = 1, 2, 3 /* ERROR "extra init expr 3" */ ) func _() { var ( m1a, m1b = 1, 2 - m2a, m2b, m2c /* ERROR "missing initialization" */ = 1, 2 - m3a, m3b = 1, 2, 3 /* ERROR "too many initialization" */ + m2a, m2b, m2c /* ERROR "missing init expr for m2c" */ = 1, 2 + m3a, m3b = 1, 2, 3 /* ERROR "extra init expr 3" */ ) } diff --git a/go/types/testdata/vardecl.src b/go/types/testdata/vardecl.src new file mode 100644 index 00000000..4f90b091 --- /dev/null +++ b/go/types/testdata/vardecl.src @@ -0,0 +1,44 @@ +// 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. + +package vardecl + +func f() {} + +// Var decls must have a type or an initializer. +var _ int +var _, _ int + +var _ /* ERROR "missing type or init expr" */ +var _ /* ERROR "missing type or init expr" */, _ +var _ /* ERROR "missing type or init expr" */, _, _ + +// The initializer must be an expression. +var _ = int /* ERROR "not an expression" */ +var _ = f /* ERROR "used as value" */ () + +// Identifier and expression arity must match. +var _, _ = 1, 2 +var _ = 1, 2 /* ERROR "extra init expr 2" */ +var _ /* ERROR "assignment count mismatch" */ , _ = 1 // TODO(gri) better error message here +var _, _, _ /* ERROR "missing init expr for _" */ = 1, 2 + +var _, _ int = 1, 2 +var _ int = 1, 2 /* ERROR "extra init expr 2" */ +var _ /* ERROR "assignment count mismatch" */ , _ int = 1 // TODO(gri) better error message here +var _, _, _ /* ERROR "missing init expr for _" */ int = 1, 2 + +var ( + _, _ = 1, 2 + _ = 1, 2 /* ERROR "extra init expr 2" */ + _ /* ERROR "assignment count mismatch" */ , _ = 1 // TODO(gri) better error message here + _, _, _ /* ERROR "missing init expr for _" */ = 1, 2 + + _, _ int = 1, 2 + _ int = 1, 2 /* ERROR "extra init expr 2" */ + _ /* ERROR "assignment count mismatch" */ , _ int = 1 // TODO(gri) better error message here + _, _, _ /* ERROR "missing init expr for _" */ int = 1, 2 +) + +// TODO(gri) consolidate other var decl checks in this file \ No newline at end of file