From 7ca228a514cc98bf6e9a4a6e84a8aaca97e19145 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 10 Oct 2013 09:02:54 -0700 Subject: [PATCH] go.tools/go.types: more missing assigment checks implemented Assignments to "comma, ok" expressions on the lhs of an assignment are not permitted unless we have map index "comma, ok" expression. Created new operand mode 'mapindex' to distinguish this case. Renamed mode 'valueok' to the more commonly used 'commaok' term, which also makes it easier to distinguish from simply 'value'. Added corresponding tests. Fixes a TODO. R=adonovan CC=golang-dev https://golang.org/cl/14526049 --- go/types/assignments.go | 27 +++++++++++++-------------- go/types/call.go | 2 +- go/types/eval.go | 2 +- go/types/expr.go | 6 +++--- go/types/operand.go | 13 +++++++++---- go/types/testdata/errors.src | 2 +- go/types/testdata/stmt0.src | 28 ++++++++++++++++++++++++++++ 7 files changed, 56 insertions(+), 24 deletions(-) diff --git a/go/types/assignments.go b/go/types/assignments.go index 6a9f3eab..88544f9e 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -26,7 +26,7 @@ func (check *checker) assignment(x *operand, T Type) bool { switch x.mode { case invalid: return true // error reported before - case constant, variable, value, valueok: + case constant, variable, mapindex, value, commaok: // ok default: unreachable() @@ -99,12 +99,7 @@ func (check *checker) initConst(lhs *Const, x *operand) { return } - // rhs type must be a valid constant type - if !isConstType(x.typ) { - check.errorf(x.pos(), "%s has invalid constant type", x) - return - } - + assert(isConstType(x.typ)) lhs.val = x.val } @@ -193,14 +188,18 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type { return nil } - if z.mode == constant || z.mode == value { - check.errorf(z.pos(), "cannot assign to non-variable %s", &z) + // spec: Each left-hand side operand must be addressable, a map index + // expression, or the blank identifier. Operands may be parenthesized. + switch z.mode { + case invalid: + return nil + case variable, mapindex: + // ok + default: + check.errorf(z.pos(), "cannot assign to %s", &z) return nil } - // TODO(gri) z.mode can also be valueok which in some cases is ok (maps) - // but in others isn't (channels). Complete the checks here. - if !check.assignment(x, z.typ) { if x.mode != invalid { check.errorf(x.pos(), "cannot assign %s to %s", x, &z) @@ -258,7 +257,7 @@ func (check *checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos) } } - if !returnPos.IsValid() && x.mode == valueok && l == 2 { + if !returnPos.IsValid() && (x.mode == mapindex || x.mode == commaok) && l == 2 { // comma-ok expression (not permitted with return statements) x.mode = value t1 := check.initVar(lhs[0], &x) @@ -328,7 +327,7 @@ func (check *checker) assignVars(lhs, rhs []ast.Expr) { } } - if x.mode == valueok && l == 2 { + if (x.mode == mapindex || x.mode == commaok) && l == 2 { // comma-ok expression x.mode = value t1 := check.assignVar(lhs[0], &x) diff --git a/go/types/call.go b/go/types/call.go index d8b499be..7dd7bd4d 100644 --- a/go/types/call.go +++ b/go/types/call.go @@ -130,7 +130,7 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int) { }, t.Len() } - if x0.mode == valueok { + if x0.mode == mapindex || x0.mode == commaok { // comma-ok value if allowCommaOk { return func(x *operand, i int) { diff --git a/go/types/eval.go b/go/types/eval.go index 8f49b84b..dd4f0269 100644 --- a/go/types/eval.go +++ b/go/types/eval.go @@ -102,7 +102,7 @@ func EvalNode(fset *token.FileSet, node ast.Expr, pkg *Package, scope *Scope) (t case constant: val = x.val fallthrough - case typexpr, variable, value, valueok: + case typexpr, variable, mapindex, value, commaok: typ = x.typ } diff --git a/go/types/expr.go b/go/types/expr.go index 761e7090..2035e398 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -107,7 +107,7 @@ func (check *checker) unary(x *operand, op token.Token) { x.mode = invalid return } - x.mode = valueok + x.mode = commaok x.typ = typ.elt return } @@ -1168,7 +1168,7 @@ func (check *checker) expr0(x *operand, e ast.Expr, hint Type) exprKind { } goto Error } - x.mode = valueok + x.mode = mapindex x.typ = typ.elt x.expr = e return expression @@ -1308,7 +1308,7 @@ func (check *checker) expr0(x *operand, e ast.Expr, hint Type) exprKind { goto Error } check.typeAssertion(x.pos(), x, xtyp, T) - x.mode = valueok + x.mode = commaok x.typ = T case *ast.CallExpr: diff --git a/go/types/operand.go b/go/types/operand.go index b73f87ad..de518218 100644 --- a/go/types/operand.go +++ b/go/types/operand.go @@ -24,8 +24,9 @@ const ( typexpr // operand is a type constant // operand is a constant; the operand's typ is a Basic type variable // operand is an addressable variable + mapindex // operand is a map index expression (acts like a variable on lhs, commaok on rhs of an assignment) value // operand is a computed value - valueok // like value, but operand may be used in a comma,ok expression + commaok // like value, but operand may be used in a comma,ok expression ) var operandModeString = [...]string{ @@ -35,8 +36,9 @@ var operandModeString = [...]string{ typexpr: "type", constant: "constant", variable: "variable", + mapindex: "map index expression", value: "value", - valueok: "value, ok", + commaok: "comma, ok expression", } // An operand represents an intermediate value during type checking. @@ -83,11 +85,14 @@ func (x *operand) pos() token.Pos { // variable ( ) // variable ( of type ) // +// mapindex ( ) +// mapindex ( of type ) +// // value ( ) // value ( of type ) // -// valueok ( ) -// valueok ( of type ) +// commaok ( ) +// commaok ( of type ) // func (x *operand) String() string { var buf bytes.Buffer diff --git a/go/types/testdata/errors.src b/go/types/testdata/errors.src index 5f39bde4..11380078 100644 --- a/go/types/testdata/errors.src +++ b/go/types/testdata/errors.src @@ -34,7 +34,7 @@ func f(x int, m map[string]int) { // value, ok's const s = "foo" - m /* ERROR "m\[s\] \(value, ok of type int\) is not used" */ [s] + m /* ERROR "m\[s\] \(map index expression of type int\) is not used" */ [s] } // Valid ERROR comments can have a variety of forms. diff --git a/go/types/testdata/stmt0.src b/go/types/testdata/stmt0.src index aecdc4c7..752f33f6 100644 --- a/go/types/testdata/stmt0.src +++ b/go/types/testdata/stmt0.src @@ -92,6 +92,34 @@ func assignments1() { (_) = 0 } +func assignments2() { + var m map[string][]bool + var s []bool + var b bool + _ = s + _ = b + + // assignments to map index expressions are ok + s, b = m["foo"] + m["foo"] = nil + m["foo"] = nil /* ERROR assignment count mismatch */ , false + _ = append(m["foo"]) + _ = append(m["foo"], true) + + var c chan int + _, b = <-c + <- /* ERROR cannot assign */ c = 0 + <-c = 0 /* ERROR assignment count mismatch */ , false + + var x interface{} + _, b = x.(int) + x /* ERROR cannot assign */ .(int) = 0 + x.(int) = 0 /* ERROR assignment count mismatch */ , false + + assignments2 /* ERROR used as value */ () = nil + int /* ERROR not an expression */ = 0 +} + func issue6487() { type S struct{x int} _ = &S /* ERROR "cannot take address" */ {}.x