From 71588bc2bceddb95795bc5a306c835e5d8f58fdc Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 16 Oct 2012 10:20:03 -0700 Subject: [PATCH] exp/types/staging: index and slice type checks Also: handle assignments to the blank identifier. R=rsc CC=golang-dev https://golang.org/cl/6658050 --- src/pkg/exp/types/staging/const.go | 16 ++ src/pkg/exp/types/staging/expr.go | 156 +++++++++++++----- src/pkg/exp/types/staging/stmt.go | 12 +- src/pkg/exp/types/staging/testdata/decls1.src | 4 +- src/pkg/exp/types/staging/testdata/expr3.src | 74 ++++++++- 5 files changed, 216 insertions(+), 46 deletions(-) diff --git a/src/pkg/exp/types/staging/const.go b/src/pkg/exp/types/staging/const.go index 79b89e19153..e817f1fd858 100644 --- a/src/pkg/exp/types/staging/const.go +++ b/src/pkg/exp/types/staging/const.go @@ -165,6 +165,22 @@ func isZeroConst(x interface{}) bool { return ok && i == 0 } +// isNegConst reports whether the value of constant x is < 0. +// x must be a non-complex numeric value. +// +func isNegConst(x interface{}) bool { + switch x := x.(type) { + case int64: + return x < 0 + case *big.Int: + return x.Sign() < 0 + case *big.Rat: + return x.Sign() < 0 + } + unreachable() + return false +} + // isRepresentableConst reports whether the value of constant x can // be represented as a value of the basic type Typ[as] without loss // of precision. diff --git a/src/pkg/exp/types/staging/expr.go b/src/pkg/exp/types/staging/expr.go index 7c7445b2365..560b16c16f3 100644 --- a/src/pkg/exp/types/staging/expr.go +++ b/src/pkg/exp/types/staging/expr.go @@ -397,11 +397,39 @@ func (check *checker) binary(x, y *operand, op token.Token, hint Type) { // x.typ is unchanged } -func (check *checker) index(x *operand, e ast.Expr, iota int) { - check.expr(x, e, nil, iota) - if !isInteger(x.typ) { - check.errorf(x.pos(), "array index %s must be integer", x) +// index checks an index expression for validity. If length >= 0, it is the upper +// bound for the index. The result is a valid constant index >= 0, or a negative +// value. +// +func (check *checker) index(index ast.Expr, length int64, iota int) int64 { + var x operand + var i int64 // index value, valid if >= 0 + + check.expr(&x, index, nil, iota) + if !x.isInteger() { + check.errorf(x.pos(), "index %s must be integer", &x) + return -1 } + if x.mode != constant { + return -1 // we cannot check more + } + // x.mode == constant and the index value must be >= 0 + if isNegConst(x.val) { + check.errorf(x.pos(), "index %s must not be negative", &x) + return -1 + } + var ok bool + if i, ok = x.val.(int64); !ok { + // index value doesn't fit into an int64 + i = length // trigger out of bounds check below if we know length (>= 0) + } + + if length >= 0 && i >= length { + check.errorf(x.pos(), "index %s is out of bounds (>= %d)", &x, length) + return -1 + } + + return i } func (check *checker) callRecord(x *operand) { @@ -553,20 +581,34 @@ func (check *checker) exprOrType(x *operand, e ast.Expr, hint Type, iota int, cy goto Error case *ast.IndexExpr: - var index operand check.expr(x, e.X, hint, iota) + + valid := false + length := int64(-1) // valid if >= 0 switch typ := underlying(x.typ).(type) { - case *Array: - check.index(&index, e.Index, iota) - if x.mode == constant { - // TODO(gri) range check + case *Basic: + if isString(typ) { + valid = true + if x.mode == constant { + length = int64(len(x.val.(string))) + } + // an indexed string always yields a byte value + // (not a constant) even if the string and the + // index are constant + x.mode = value + x.typ = Typ[Byte] + } + + case *Array: + valid = true + length = typ.Len + if x.mode != variable { + x.mode = value } - // TODO(gri) only variable if array is variable - x.mode = variable x.typ = typ.Elt case *Slice: - check.index(&index, e.Index, iota) + valid = true x.mode = variable x.typ = typ.Elt @@ -574,45 +616,77 @@ func (check *checker) exprOrType(x *operand, e ast.Expr, hint Type, iota int, cy // TODO(gri) check index type x.mode = variable x.typ = typ.Elt + return + } - default: - check.invalidOp(e.Pos(), "cannot index %s", x.typ) + if !valid { + check.invalidOp(x.pos(), "cannot index %s", x) goto Error } + if e.Index == nil { + check.invalidAST(e.Pos(), "missing index expression for %s", x) + return + } + + check.index(e.Index, length, iota) + // ok to continue + case *ast.SliceExpr: - var lo, hi operand check.expr(x, e.X, hint, iota) - if e.Low != nil { - check.index(&lo, e.Low, iota) - } else { - lo.mode = constant - lo.expr = nil // TODO(gri) should not use nil here - lo.typ = Typ[UntypedInt] - lo.val = zeroConst - } - if e.High != nil { - check.index(&hi, e.High, iota) - } else { - unimplemented() - } - switch typ := x.typ.(type) { - case *Array: - unimplemented() - case *Slice: - assert(x.mode == variable) - // x.typ does not change - case *Pointer: - if typ, ok := underlying(typ.Base).(*Array); ok { - // TODO(gri) array slice - _ = typ + + valid := false + length := int64(-1) // valid if >= 0 + switch typ := underlying(x.typ).(type) { + case *Basic: + if isString(typ) { + valid = true + if x.mode == constant { + length = int64(len(x.val.(string))) + 1 // +1 for slice + } + // a sliced string always yields a string value + // of the same type as the original string (not + // a constant) even if the string and the indexes + // are constant + x.mode = value + // x.typ doesn't change } - unimplemented() - default: - check.invalidOp(e.Pos(), "cannot slice %s", x.typ) + + case *Array: + valid = true + length = typ.Len + 1 // +1 for slice + if x.mode != variable { + check.invalidOp(x.pos(), "cannot slice %s (value not addressable)", x) + goto Error + } + x.typ = &Slice{Elt: typ.Elt} + + case *Slice: + valid = true + x.mode = variable + // x.typ doesn't change + } + + if !valid { + check.invalidOp(x.pos(), "cannot slice %s", x) goto Error } + var lo int64 + if e.Low != nil { + lo = check.index(e.Low, length, iota) + } + + var hi int64 = length + if e.High != nil { + hi = check.index(e.High, length, iota) + } + + if hi >= 0 && lo > hi { + check.errorf(e.Low.Pos(), "inverted slice range: %d > %d", lo, hi) + // ok to continue + } + case *ast.TypeAssertExpr: check.expr(x, e.X, hint, iota) if _, ok := x.typ.(*Interface); !ok { diff --git a/src/pkg/exp/types/staging/stmt.go b/src/pkg/exp/types/staging/stmt.go index 9a2ea74895c..be5caa1e364 100644 --- a/src/pkg/exp/types/staging/stmt.go +++ b/src/pkg/exp/types/staging/stmt.go @@ -108,6 +108,15 @@ func (check *checker) assignment(lhs ast.Expr, x *operand, decl bool) { } func (check *checker) assign1to1(lhs, rhs ast.Expr, decl bool, iota int) { + ident, _ := lhs.(*ast.Ident) + + if ident != nil && ident.Name == "_" { + // anything can be assigned to a blank identifier - check rhs only + var x operand + check.expr(&x, rhs, nil, iota) + return + } + if !decl { // regular assignment - start with lhs[0] to obtain a type hint var z operand @@ -127,8 +136,7 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, decl bool, iota int) { } // declaration - rhs may or may not be typed yet - ident, ok := lhs.(*ast.Ident) - if !ok { + if ident == nil { check.errorf(lhs.Pos(), "cannot declare %s", lhs) return } diff --git a/src/pkg/exp/types/staging/testdata/decls1.src b/src/pkg/exp/types/staging/testdata/decls1.src index 6accee61ed5..32859fc0ee5 100644 --- a/src/pkg/exp/types/staging/testdata/decls1.src +++ b/src/pkg/exp/types/staging/testdata/decls1.src @@ -37,7 +37,7 @@ var ( s4 = s + t s5 = s /* ERROR "invalid operation" */ / t s6 = array[t1] - s7 = array[x /* ERROR "array index" */] + s7 = array[x /* ERROR "index" */] s8 = &a s10 = &42 /* ERROR "cannot take address" */ s11 = &v @@ -56,7 +56,7 @@ var ( t4 string = s + t t5 string = s /* ERROR "invalid operation" */ / t t6 byte = array[t1] - t7 byte = array[x /* ERROR "array index" */] + t7 byte = array[x /* ERROR "index" */] t8 *int = & /* ERROR "cannot assign" */ a t10 *int = &42 /* ERROR "cannot take address" */ t11 *complex64 = &v diff --git a/src/pkg/exp/types/staging/testdata/expr3.src b/src/pkg/exp/types/staging/testdata/expr3.src index ecdb54f4bc9..5635e12eeb0 100644 --- a/src/pkg/exp/types/staging/testdata/expr3.src +++ b/src/pkg/exp/types/staging/testdata/expr3.src @@ -2,10 +2,12 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// shifts +// various expressions package expr3 +// TODO(gri) Move the code below into function "shifts" once we check +// declarations with initilizations inside functions. var ( i0 int u0 uint @@ -41,3 +43,73 @@ var ( v float32 = 1 /* ERROR "must be integer" */ <