Skip to content

Commit 33ff155

Browse files
committed
go/types: preserve untyped constants on the RHS of a shift expression
CL 291316 fixed go/types to verify that untyped shift counts are representable by uint, but as a side effect also converted their types to uint. Rearrange the logic to keep the check for representability, but not actually convert untyped integer constants. Untyped non-integer constants are still converted, to preserve the behavior of 1.16. This behavior for non-integer types is a bug, filed as #47410. Updates #47410 Fixes #47243 Change-Id: I5eab4aab35b97f932fccdee2d4a18623ee2ccad5 Reviewed-on: https://go-review.googlesource.com/c/go/+/337529 Trust: Robert Findley <[email protected]> Trust: Robert Griesemer <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent 840e583 commit 33ff155

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

src/go/types/api_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,18 @@ func TestTypesInfo(t *testing.T) {
322322
`[][]struct{}`,
323323
},
324324

325+
// issue 47243
326+
{`package issue47243_a; var x int32; var _ = x << 3`, `3`, `untyped int`},
327+
{`package issue47243_b; var x int32; var _ = x << 3.`, `3.`, `uint`}, // issue 47410: should be untyped float
328+
{`package issue47243_c; var x int32; var _ = 1 << x`, `1 << x`, `int`},
329+
{`package issue47243_d; var x int32; var _ = 1 << x`, `1`, `int`},
330+
{`package issue47243_e; var x int32; var _ = 1 << 2`, `1`, `untyped int`},
331+
{`package issue47243_f; var x int32; var _ = 1 << 2`, `2`, `untyped int`},
332+
{`package issue47243_g; var x int32; var _ = int(1) << 2`, `2`, `untyped int`},
333+
{`package issue47243_h; var x int32; var _ = 1 << (2 << x)`, `1`, `int`},
334+
{`package issue47243_i; var x int32; var _ = 1 << (2 << x)`, `(2 << x)`, `untyped int`},
335+
{`package issue47243_j; var x int32; var _ = 1 << (2 << x)`, `2`, `untyped int`},
336+
325337
// tests for broken code that doesn't parse or type-check
326338
{broken + `x0; func _() { var x struct {f string}; x.f := 0 }`, `x.f`, `string`},
327339
{broken + `x1; func _() { var z string; type x struct {f string}; y := &x{q: z}}`, `z`, `string`},

src/go/types/check_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,13 @@ func TestIssue46453(t *testing.T) {
344344
checkFiles(t, nil, "", []string{"issue46453.go"}, [][]byte{[]byte(src)}, false, nil)
345345
}
346346

347+
func TestIssue47243_TypedRHS(t *testing.T) {
348+
// The RHS of the shift expression below overflows uint on 32bit platforms,
349+
// but this is OK as it is explicitly typed.
350+
const src = "package issue47243\n\nvar a uint64; var _ = a << uint64(4294967296)" // uint64(1<<32)
351+
checkFiles(t, &StdSizes{4, 4}, "", []string{"p.go"}, [][]byte{[]byte(src)}, false, nil)
352+
}
353+
347354
func TestCheck(t *testing.T) { DefPredeclaredTestFuncs(); testDir(t, "check") }
348355
func TestExamples(t *testing.T) { testDir(t, "examples") }
349356
func TestFixedbugs(t *testing.T) { testDir(t, "fixedbugs") }

src/go/types/expr.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -778,32 +778,48 @@ func (check *Checker) shift(x, y *operand, e ast.Expr, op token.Token) {
778778
// spec: "The right operand in a shift expression must have integer type
779779
// or be an untyped constant representable by a value of type uint."
780780

781-
// Provide a good error message for negative shift counts.
781+
// Check that constants are representable by uint, but do not convert them
782+
// (see also issue #47243).
782783
if y.mode == constant_ {
784+
// Provide a good error message for negative shift counts.
783785
yval := constant.ToInt(y.val) // consider -1, 1.0, but not -1.1
784786
if yval.Kind() == constant.Int && constant.Sign(yval) < 0 {
785787
check.invalidOp(y, _InvalidShiftCount, "negative shift count %s", y)
786788
x.mode = invalid
787789
return
788790
}
791+
792+
if isUntyped(y.typ) {
793+
// Caution: Check for representability here, rather than in the switch
794+
// below, because isInteger includes untyped integers (was bug #43697).
795+
check.representable(y, Typ[Uint])
796+
if y.mode == invalid {
797+
x.mode = invalid
798+
return
799+
}
800+
}
789801
}
790802

791-
// Caution: Check for isUntyped first because isInteger includes untyped
792-
// integers (was bug #43697).
793-
if isUntyped(y.typ) {
803+
// Check that RHS is otherwise at least of integer type.
804+
switch {
805+
case isInteger(y.typ):
806+
if !isUnsigned(y.typ) && !check.allowVersion(check.pkg, 1, 13) {
807+
check.invalidOp(y, _InvalidShiftCount, "signed shift count %s requires go1.13 or later", y)
808+
x.mode = invalid
809+
return
810+
}
811+
case isUntyped(y.typ):
812+
// This is incorrect, but preserves pre-existing behavior.
813+
// See also bug #47410.
794814
check.convertUntyped(y, Typ[Uint])
795815
if y.mode == invalid {
796816
x.mode = invalid
797817
return
798818
}
799-
} else if !isInteger(y.typ) {
819+
default:
800820
check.invalidOp(y, _InvalidShiftCount, "shift count %s must be integer", y)
801821
x.mode = invalid
802822
return
803-
} else if !isUnsigned(y.typ) && !check.allowVersion(check.pkg, 1, 13) {
804-
check.invalidOp(y, _InvalidShiftCount, "signed shift count %s requires go1.13 or later", y)
805-
x.mode = invalid
806-
return
807823
}
808824

809825
if x.mode == constant_ {

0 commit comments

Comments
 (0)