[dev.typeparams] cmd/compile/internal/types2: untyped shift counts must fit into...

Updates #43697.

Change-Id: If94658cb798bb0434ac3ebbf9dff504dcd59a02a
Reviewed-on: https://go-review.googlesource.com/c/go/+/283872
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go
index a3778129ffbaca6604622a21bd0be9c713f70fb1..736d3bfacccd97dbf5d64f787aa5ad71cc6c5c18 100644 (file)
--- 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 {
+
+       // Provide a good error message for negative shift counts.
+       if y.mode == constant_ {
+               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
                }
-       default:
-               check.invalidOpf(y, "shift count %s must be integer", y)
-               x.mode = invalid
-               return
        }
 
-       var yval constant.Value
-       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 {
-                       check.invalidOpf(y, "negative shift count %s", y)
+       // 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 04a679f5bb4110ad0a68264b51476ac30e5f033a..60db731cf4e03d3d2ee9c02bc1fb623ab89cb6c4 100644 (file)
--- 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)
        )
 }