From 82c3f0a358ed449ffcdd5b419728721b314d7a91 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 14 Jan 2021 11:50:05 -0800 Subject: [PATCH] [dev.typeparams] cmd/compile/internal/types2: untyped shift counts must fit into uint Updates #43697. Change-Id: If94658cb798bb0434ac3ebbf9dff504dcd59a02a Reviewed-on: https://go-review.googlesource.com/c/go/+/283872 Trust: Robert Griesemer Run-TryBot: Robert Griesemer Reviewed-by: Robert Findley TryBot-Result: Go Bot --- src/cmd/compile/internal/types2/expr.go | 46 +++++++++---------- .../internal/types2/testdata/shifts.src | 12 +++-- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index a3778129ff..736d3bfacc 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -770,14 +770,14 @@ func (check *Checker) comparison(x, y *operand, op syntax.Operator) { } func (check *Checker) shift(x, y *operand, e *syntax.Operation, op syntax.Operator) { - untypedx := isUntyped(x.typ) + // TODO(gri) This function seems overly complex. Revisit. var xval constant.Value if x.mode == constant_ { xval = constant.ToInt(x.val) } - if isInteger(x.typ) || untypedx && xval != nil && xval.Kind() == constant.Int { + if isInteger(x.typ) || isUntyped(x.typ) && xval != nil && xval.Kind() == constant.Int { // The lhs is of integer type or an untyped constant representable // as an integer. Nothing to do. } else { @@ -789,40 +789,36 @@ func (check *Checker) shift(x, y *operand, e *syntax.Operation, op syntax.Operat // spec: "The right operand in a shift expression must have integer type // or be an untyped constant representable by a value of type uint." - switch { - case isInteger(y.typ): - // nothing to do - case isUntyped(y.typ): - check.convertUntyped(y, Typ[Uint]) - if y.mode == invalid { - x.mode = invalid - return - } - default: - check.invalidOpf(y, "shift count %s must be integer", y) - x.mode = invalid - return - } - var yval constant.Value + // Provide a good error message for negative shift counts. if y.mode == constant_ { - // rhs must be an integer value - // (Either it was of an integer type already, or it was - // untyped and successfully converted to a uint above.) - yval = constant.ToInt(y.val) - assert(yval.Kind() == constant.Int) - if constant.Sign(yval) < 0 { + yval := constant.ToInt(y.val) // consider -1, 1.0, but not -1.1 + if yval.Kind() == constant.Int && constant.Sign(yval) < 0 { check.invalidOpf(y, "negative shift count %s", y) x.mode = invalid return } } + // Caution: Check for isUntyped first because isInteger includes untyped + // integers (was bug #43697). + if isUntyped(y.typ) { + check.convertUntyped(y, Typ[Uint]) + if y.mode == invalid { + x.mode = invalid + return + } + } else if !isInteger(y.typ) { + check.invalidOpf(y, "shift count %s must be integer", y) + x.mode = invalid + return + } + if x.mode == constant_ { if y.mode == constant_ { // rhs must be within reasonable bounds in constant shifts const shiftBound = 1023 - 1 + 52 // so we can express smallestFloat64 - s, ok := constant.Uint64Val(yval) + s, ok := constant.Uint64Val(y.val) if !ok || s > shiftBound { check.invalidOpf(y, "invalid shift count %s", y) x.mode = invalid @@ -849,7 +845,7 @@ func (check *Checker) shift(x, y *operand, e *syntax.Operation, op syntax.Operat } // non-constant shift with constant lhs - if untypedx { + if isUntyped(x.typ) { // spec: "If the left operand of a non-constant shift // expression is an untyped constant, the type of the // constant is what it would be if the shift expression diff --git a/src/cmd/compile/internal/types2/testdata/shifts.src b/src/cmd/compile/internal/types2/testdata/shifts.src index 04a679f5bb..60db731cf4 100644 --- a/src/cmd/compile/internal/types2/testdata/shifts.src +++ b/src/cmd/compile/internal/types2/testdata/shifts.src @@ -20,7 +20,7 @@ func shifts0() { // This depends on the exact spec wording which is not // done yet. // TODO(gri) revisit and adjust when spec change is done - _ = 1<<- /* ERROR "truncated to uint" */ 1.0 + _ = 1<<- /* ERROR "negative shift count" */ 1.0 _ = 1<<1075 /* ERROR "invalid shift" */ _ = 2.0<<1 _ = 1<<1.0 @@ -60,11 +60,13 @@ func shifts1() { _ uint = 1 << u _ float32 = 1 /* ERROR "must be integer" */ << u - // for issue 14822 + // issue #14822 + _ = 1<<( /* ERROR "overflows uint" */ 1<<64) _ = 1<<( /* ERROR "invalid shift count" */ 1<<64-1) - _ = 1<<( /* ERROR "invalid shift count" */ 1<<64) - _ = u<<(1<<63) // valid - _ = u<<(1<<64) // valid + + // issue #43697 + _ = u<<( /* ERROR "overflows uint" */ 1<<64) + _ = u<<(1<<64-1) ) }