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 0dc0070..66d62d6 100644
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -655,11 +655,10 @@
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 @@
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 @@
}
}
- // 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 84908fd..b254b29 100644
--- a/src/go/types/stdlib_test.go
+++ b/src/go/types/stdlib_test.go
@@ -167,6 +167,7 @@
}
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 0740546..e6beb78 100644
--- a/src/go/types/testdata/decls1.src
+++ b/src/go/types/testdata/decls1.src
@@ -43,7 +43,7 @@
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 @@
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 eaaf610..4ead815 100644
--- a/src/go/types/testdata/expr1.src
+++ b/src/go/types/testdata/expr1.src
@@ -35,8 +35,8 @@
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 @@
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 52e340e..ebc95ba 100644
--- a/src/go/types/testdata/shifts.src
+++ b/src/go/types/testdata/shifts.src
@@ -10,9 +10,21 @@
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 @@
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 @@
_ 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
)
}
