[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 <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>

Author: griesemer

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

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)
 	)
 }