Skip to content

Commit c0bbeb0

Browse files
committed
cmd/compile: adjust types2 shift check to match go/types (cleanup)
With this change, the shift checking code matches the corresponding go/types code, but for the differences in the internal error reporting, and call of check.overflow. The change leads to the recording of an untyped int value if the RHS of a non-constant shift is an untyped integer value. Adjust the type in the compiler's irgen accordingly. Add test/shift3.go to verify behavior. Change-Id: I20386fcb1d5c48becffdc2203081fb70c08b282d Reviewed-on: https://go-review.googlesource.com/c/go/+/398236 Trust: Robert Griesemer <[email protected]> Run-TryBot: Robert Griesemer <[email protected]> Trust: Matthew Dempsky <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 063f403 commit c0bbeb0

File tree

5 files changed

+126
-15
lines changed

5 files changed

+126
-15
lines changed

src/cmd/compile/internal/noder/expr.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package noder
66

77
import (
88
"fmt"
9+
"go/constant"
910

1011
"cmd/compile/internal/base"
1112
"cmd/compile/internal/ir"
@@ -62,6 +63,14 @@ func (g *irgen) expr(expr syntax.Expr) ir.Node {
6263
case types2.UntypedNil:
6364
// ok; can appear in type switch case clauses
6465
// TODO(mdempsky): Handle as part of type switches instead?
66+
case types2.UntypedInt, types2.UntypedFloat, types2.UntypedComplex:
67+
// Untyped rhs of non-constant shift, e.g. x << 1.0.
68+
// If we have a constant value, it must be an int >= 0.
69+
if tv.Value != nil {
70+
s := constant.ToInt(tv.Value)
71+
assert(s.Kind() == constant.Int && constant.Sign(s) >= 0)
72+
}
73+
typ = types2.Typ[types2.Uint]
6574
case types2.UntypedBool:
6675
typ = types2.Typ[types2.Bool] // expression in "if" or "for" condition
6776
case types2.UntypedString:

src/cmd/compile/internal/types2/api_test.go

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

314+
// issue 47243
315+
{`package issue47243_a; var x int32; var _ = x << 3`, `3`, `untyped int`},
316+
{`package issue47243_b; var x int32; var _ = x << 3.`, `3.`, `untyped float`},
317+
{`package issue47243_c; var x int32; var _ = 1 << x`, `1 << x`, `int`},
318+
{`package issue47243_d; var x int32; var _ = 1 << x`, `1`, `int`},
319+
{`package issue47243_e; var x int32; var _ = 1 << 2`, `1`, `untyped int`},
320+
{`package issue47243_f; var x int32; var _ = 1 << 2`, `2`, `untyped int`},
321+
{`package issue47243_g; var x int32; var _ = int(1) << 2`, `2`, `untyped int`},
322+
{`package issue47243_h; var x int32; var _ = 1 << (2 << x)`, `1`, `int`},
323+
{`package issue47243_i; var x int32; var _ = 1 << (2 << x)`, `(2 << x)`, `untyped int`},
324+
{`package issue47243_j; var x int32; var _ = 1 << (2 << x)`, `2`, `untyped int`},
325+
314326
// tests for broken code that doesn't parse or type-check
315327
{brokenPkg + `x0; func _() { var x struct {f string}; x.f := 0 }`, `x.f`, `string`},
316328
{brokenPkg + `x1; func _() { var z string; type x struct {f string}; y := &x{q: z}}`, `z`, `string`},

src/cmd/compile/internal/types2/expr.go

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -954,32 +954,48 @@ func (check *Checker) shift(x, y *operand, e syntax.Expr, op syntax.Operator) {
954954
// spec: "The right operand in a shift expression must have integer type
955955
// or be an untyped constant representable by a value of type uint."
956956

957-
// Provide a good error message for negative shift counts.
957+
// Check that constants are representable by uint, but do not convert them
958+
// (see also issue #47243).
958959
if y.mode == constant_ {
960+
// Provide a good error message for negative shift counts.
959961
yval := constant.ToInt(y.val) // consider -1, 1.0, but not -1.1
960962
if yval.Kind() == constant.Int && constant.Sign(yval) < 0 {
961963
check.errorf(y, invalidOp+"negative shift count %s", y)
962964
x.mode = invalid
963965
return
964966
}
965-
}
966967

967-
// Caution: Check for isUntyped first because isInteger includes untyped
968-
// integers (was bug #43697).
969-
if isUntyped(y.typ) {
970-
check.convertUntyped(y, Typ[Uint])
971-
if y.mode == invalid {
968+
if isUntyped(y.typ) {
969+
// Caution: Check for representability here, rather than in the switch
970+
// below, because isInteger includes untyped integers (was bug #43697).
971+
check.representable(y, Typ[Uint])
972+
if y.mode == invalid {
973+
x.mode = invalid
974+
return
975+
}
976+
}
977+
} else {
978+
// Check that RHS is otherwise at least of integer type.
979+
switch {
980+
case allInteger(y.typ):
981+
if !allUnsigned(y.typ) && !check.allowVersion(check.pkg, 1, 13) {
982+
check.errorf(y, invalidOp+"signed shift count %s requires go1.13 or later", y)
983+
x.mode = invalid
984+
return
985+
}
986+
case isUntyped(y.typ):
987+
// This is incorrect, but preserves pre-existing behavior.
988+
// See also bug #47410.
989+
check.convertUntyped(y, Typ[Uint])
990+
if y.mode == invalid {
991+
x.mode = invalid
992+
return
993+
}
994+
default:
995+
check.errorf(y, invalidOp+"shift count %s must be integer", y)
972996
x.mode = invalid
973997
return
974998
}
975-
} else if !allInteger(y.typ) {
976-
check.errorf(y, invalidOp+"shift count %s must be integer", y)
977-
x.mode = invalid
978-
return
979-
} else if !allUnsigned(y.typ) && !check.allowVersion(check.pkg, 1, 13) {
980-
check.versionErrorf(y, "go1.13", invalidOp+"signed shift count %s", y)
981-
x.mode = invalid
982-
return
983999
}
9841000

9851001
if x.mode == constant_ {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// -lang=go1.12
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package p
8+
9+
type resultFlags uint
10+
11+
// Example from #52031.
12+
//
13+
// The following shifts should not produce errors on Go < 1.13, as their
14+
// untyped constant operands are representable by type uint.
15+
const (
16+
_ resultFlags = (1 << iota) / 2
17+
18+
reportEqual
19+
reportUnequal
20+
reportByIgnore
21+
reportByMethod
22+
reportByFunc
23+
reportByCycle
24+
)
25+
26+
// Invalid cases.
27+
var x int = 1
28+
var _ = (8 << x /* ERROR "signed shift count .* requires go1.13 or later" */)
29+
30+
const _ = (1 << 1.2 /* ERROR "truncated to uint" */)
31+
32+
var y float64
33+
var _ = (1 << y /* ERROR "must be integer" */)

test/shift3.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// run
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Test that the compiler's noder uses the correct type
8+
// for RHS shift operands that are untyped. Must compile;
9+
// run for good measure.
10+
11+
package main
12+
13+
import (
14+
"fmt"
15+
"math"
16+
)
17+
18+
func f(x, y int) {
19+
if x != y {
20+
panic(fmt.Sprintf("%d != %d", x, y))
21+
}
22+
}
23+
24+
func main() {
25+
var x int = 1
26+
f(x<<1, 2)
27+
f(x<<1., 2)
28+
f(x<<(1+0i), 2)
29+
f(x<<0i, 1)
30+
31+
f(x<<(1<<x), 4)
32+
f(x<<(1.<<x), 4)
33+
f(x<<((1+0i)<<x), 4)
34+
f(x<<(0i<<x), 1)
35+
36+
// corner cases
37+
const M = math.MaxUint
38+
f(x<<(M+0), 0) // shift by untyped int representable as uint
39+
f(x<<(M+0.), 0) // shift by untyped float representable as uint
40+
f(x<<(M+0.+0i), 0) // shift by untyped complex representable as uint
41+
}

0 commit comments

Comments
 (0)