go/types: permit signed integer shift count

Permit shifts by non-constant signed integer shift counts.
Share logic for constant shift counts in non-constant
shifts and improve error messages a little bit.

R=Go1.13

Updates #19113.

Change-Id: Ia01d83ca8aa60a6a3f4c49f026e0c46396f852be
Reviewed-on: https://go-review.googlesource.com/c/159317
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index 0dc007069fadfff623587568d486f35b3b7a9780..66d62d68856d5f3a7a1718b7ef32b1bfc36ebef0 100644 (file)
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -655,11 +655,10 @@ func (check *Checker) shift(x, y *operand, e *ast.BinaryExpr, op token.Token) {
                return
        }
 
-       // spec: "The right operand in a shift expression must have unsigned
-       // integer type or be an untyped constant representable by a value of
-       // type uint."
+       // 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 isUnsigned(y.typ):
+       case isInteger(y.typ):
                // nothing to do
        case isUntyped(y.typ):
                check.convertUntyped(y, Typ[Uint])
@@ -668,21 +667,28 @@ func (check *Checker) shift(x, y *operand, e *ast.BinaryExpr, op token.Token) {
                        return
                }
        default:
-               check.invalidOp(y.pos(), "shift count %s must be unsigned integer", y)
+               check.invalidOp(y.pos(), "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.invalidOp(y.pos(), "negative shift count %s", y)
+                       x.mode = invalid
+                       return
+               }
+       }
+
        if x.mode == constant_ {
                if y.mode == constant_ {
-                       // rhs must be an integer value
-                       yval := constant.ToInt(y.val)
-                       if yval.Kind() != constant.Int {
-                               check.invalidOp(y.pos(), "shift count %s must be unsigned integer", y)
-                               x.mode = invalid
-                               return
-                       }
-                       // rhs must be within reasonable bounds
+                       // rhs must be within reasonable bounds in constant shifts
                        const shiftBound = 1023 - 1 + 52 // so we can express smallestFloat64
                        s, ok := constant.Uint64Val(yval)
                        if !ok || s > shiftBound {
@@ -741,11 +747,6 @@ func (check *Checker) shift(x, y *operand, e *ast.BinaryExpr, op token.Token) {
                }
        }
 
-       // constant rhs must be >= 0
-       if y.mode == constant_ && constant.Sign(y.val) < 0 {
-               check.invalidOp(y.pos(), "shift count %s must not be negative", y)
-       }
-
        // non-constant shift - lhs must be an integer
        if !isInteger(x.typ) {
                check.invalidOp(x.pos(), "shifted operand %s must be integer", x)

diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go
index 84908fd190c4eedb6a709423e59ae3339ac8b8ab..b254b29bdf21faf156f955ffe56b1be189314613 100644 (file)
--- a/src/go/types/stdlib_test.go
+++ b/src/go/types/stdlib_test.go
@@ -167,6 +167,7 @@ func TestStdFixed(t *testing.T) {
        }
 
        testTestDir(t, filepath.Join(runtime.GOROOT(), "test", "fixedbugs"),
+               "bug073.go",                           // checks for unsigned integer shift - disabled for now
                "bug248.go", "bug302.go", "bug369.go", // complex test instructions - ignore
                "issue6889.go",   // gc-specific test
                "issue7746.go",   // large constants - consumes too much memory

diff --git a/src/go/types/testdata/decls1.src b/src/go/types/testdata/decls1.src
index 07405469a445b0583719776bc6ebafe0704ad6d7..e6beb7835897bc12ae90eed4c1007b956ac33a21 100644 (file)
--- a/src/go/types/testdata/decls1.src
+++ b/src/go/types/testdata/decls1.src
@@ -43,7 +43,7 @@ var (
        s11 = &v
        s12 = -(u + *t11) / *&v
        s13 = a /* ERROR "shifted operand" */ << d
-       s14 = i << j /* ERROR "must be unsigned" */
+       s14 = i << j
        s18 = math.Pi * 10.0
        s19 = s1 /* ERROR "cannot call" */ ()
        s20 = f0 /* ERROR "no value" */ ()
@@ -62,7 +62,7 @@ var (
        t11 *complex64 = &v
        t12 complex64 = -(u + *t11) / *&v
        t13 int = a /* ERROR "shifted operand" */ << d
-       t14 int = i << j /* ERROR "must be unsigned" */
+       t14 int = i << j
        t15 math /* ERROR "not in selector" */
        t16 math.xxx /* ERROR "not declared" */
        t17 math /* ERROR "not a type" */ .Pi

diff --git a/src/go/types/testdata/expr1.src b/src/go/types/testdata/expr1.src
index eaaf610b03b54332469dd5524c4fdd7285146801..4ead815158f61e2dbbb91dec8e7feabef4a3923c 100644 (file)
--- a/src/go/types/testdata/expr1.src
+++ b/src/go/types/testdata/expr1.src
@@ -35,8 +35,8 @@ func _(x, y int, z myint) {
        x = x * y
        x = x / y
        x = x % y
-       x = x << y // ERROR must be unsigned integer
-       x = x >> y // ERROR must be unsigned integer
+       x = x << y
+       x = x >> y
 
        z = z + 1
        z = z + 1.0
@@ -46,8 +46,8 @@ func _(x, y int, z myint) {
        z = z /* ERROR mismatched types */ * y
        z = z /* ERROR mismatched types */ / y
        z = z /* ERROR mismatched types */ % y
-       z = z << y // ERROR must be unsigned integer
-       z = z >> y // ERROR must be unsigned integer
+       z = z << y
+       z = z >> y
 }
 
 type myuint uint

diff --git a/src/go/types/testdata/shifts.src b/src/go/types/testdata/shifts.src
index 52e340ec65cb3659b4aafe6911f336525eaa3de8..ebc95ba4d7948d216c2bb9d026a0104fd768bc50 100644 (file)
--- a/src/go/types/testdata/shifts.src
+++ b/src/go/types/testdata/shifts.src
@@ -10,9 +10,21 @@ func shifts0() {
                s = 10
                _ = 0<<0
                _ = 1<<s
-               _ = 1<<- /* ERROR "overflows uint" */ 1
+               _ = 1<<- /* ERROR "negative shift count" */ 1
+               // For the test below we may decide to convert to int
+               // rather than uint and then report a negative shift
+               // count instead, which might be a better error. The
+               // (minor) difference is that this would restrict the
+               // shift count range by half (from all uint values to
+               // the positive int values).
+               // 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<<1075 /* ERROR "invalid shift" */
                _ = 2.0<<1
+               _ = 1<<1.0
+               _ = 1<<(1+0i)
 
                _ int = 2<<s
                _ float32 = 2<<s
@@ -35,11 +47,13 @@ func shifts1() {
                u uint
 
                _ = 1<<0
-               _ = 1<<i /* ERROR "must be unsigned" */
+               _ = 1<<i
                _ = 1<<u
                _ = 1<<"foo" /* ERROR "cannot convert" */
                _ = i<<0
-               _ = i<<- /* ERROR "overflows uint" */ 1
+               _ = i<<- /* ERROR "negative shift count" */ 1
+               _ = i<<1.0
+               _ = 1<<(1+0i)
                _ = 1 /* ERROR "overflows" */ <<100
 
                _ uint = 1 << 0
@@ -47,10 +61,10 @@ func shifts1() {
                _ float32 = 1 /* ERROR "must be integer" */ << u
 
                // for issue 14822
-               _ = 1<<( /* ERROR "invalid shift count" */ 1<<63)
-               _ = 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<<( /* ERROR "overflows uint" */ 1<<64)
+               _ = u<<(1<<64) // valid
        )
 }