Skip to content

Commit

Permalink
go/types, types2: fix range clause checks for constant range expressions
Browse files Browse the repository at this point in the history 
Add missing checks for the case where the range expression is
a (possibly untyped) constant integer expression.

Add context parameter to assignVar for better error message
where the expression is part of a range clause.

Also, rename s/expr/Expr/ where it denotes an AST expression,
for clarity.

Fixes #65133.
For #65137.

Change-Id: I72962d76741abe79f613e251f7b060e99261d3ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/556398
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
  • Loading branch information
@griesemer @gopherbot
griesemer authored and gopherbot committed Jan 18, 2024
1 parent 97daa6e commit 5f949c4
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 31 deletions.
9 changes: 4 additions & 5 deletions src/cmd/compile/internal/types2/assignments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (check *Checker) lhsVar(lhs syntax.Expr) Type {
// assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil).
// If x != nil, it must be the evaluation of rhs (and rhs will be ignored).
// If the assignment check fails and x != nil, x.mode is set to invalid.
func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) {
func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand, context string) {
T := check.lhsVar(lhs) // nil if lhs is _
if !isValid(T) {
if x != nil {
Expand All @@ -255,8 +255,7 @@ func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) {
check.expr(target, x, rhs)
}

context := "assignment"
if T == nil {
if T == nil && context == "assignment" {
context = "assignment to _ identifier"
}
check.assignment(x, T, context)
Expand Down Expand Up @@ -454,7 +453,7 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
// each value can be assigned to its corresponding variable.
if l == r && !isCall {
for i, lhs := range lhs {
check.assignVar(lhs, orig_rhs[i], nil)
check.assignVar(lhs, orig_rhs[i], nil, "assignment")
}
return
}
Expand All @@ -475,7 +474,7 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
r = len(rhs)
if l == r {
for i, lhs := range lhs {
check.assignVar(lhs, nil, rhs[i])
check.assignVar(lhs, nil, rhs[i], "assignment")
}
// Only record comma-ok expression if both assignments succeeded
// (go.dev/issue/59371).
Expand Down
34 changes: 24 additions & 10 deletions src/cmd/compile/internal/types2/stmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
check.errorf(s.Lhs, NonNumericIncDec, invalidOp+"%s%s%s (non-numeric type %s)", s.Lhs, s.Op, s.Op, x.typ)
return
}
check.assignVar(s.Lhs, nil, &x)
check.assignVar(s.Lhs, nil, &x, "assignment")
return
}

Expand All @@ -478,7 +478,7 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {

var x operand
check.binary(&x, nil, lhs[0], rhs[0], s.Op)
check.assignVar(lhs[0], nil, &x)
check.assignVar(lhs[0], nil, &x, "assignment")

case *syntax.CallStmt:
kind := "go"
Expand Down Expand Up @@ -826,7 +826,7 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu

func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *syntax.RangeClause) {
// Convert syntax form to local variables.
type expr = syntax.Expr
type Expr = syntax.Expr
type identType = syntax.Name
identName := func(n *identType) string { return n.Value }
sKey := rclause.Lhs // possibly nil
Expand Down Expand Up @@ -899,8 +899,10 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
// (irregular assignment, cannot easily map to existing assignment checks)

// lhs expressions and initialization value (rhs) types
lhs := [2]expr{sKey, sValue}
rhs := [2]Type{key, val} // key, val may be nil
lhs := [2]Expr{sKey, sValue} // sKey, sValue may be nil
rhs := [2]Type{key, val} // key, val may be nil

constIntRange := x.mode == constant_ && isInteger(x.typ)

if isDef {
// short variable declaration
Expand All @@ -927,11 +929,13 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
}

// initialize lhs variable
if typ := rhs[i]; typ != nil {
if constIntRange {
check.initVar(obj, &x, "range clause")
} else if typ := rhs[i]; typ != nil {
x.mode = value
x.expr = lhs // we don't have a better rhs expression to use here
x.typ = typ
check.initVar(obj, &x, "range clause")
check.initVar(obj, &x, "assignment") // error is on variable, use "assignment" not "range clause"
} else {
obj.typ = Typ[Invalid]
obj.used = true // don't complain about unused variable
Expand All @@ -947,19 +951,29 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
} else {
check.error(noNewVarPos, NoNewVar, "no new variables on left side of :=")
}
} else {
} else if sKey != nil /* lhs[0] != nil */ {
// ordinary assignment
for i, lhs := range lhs {
if lhs == nil {
continue
}
if typ := rhs[i]; typ != nil {

if constIntRange {
check.assignVar(lhs, nil, &x, "range clause")
} else if typ := rhs[i]; typ != nil {
x.mode = value
x.expr = lhs // we don't have a better rhs expression to use here
x.typ = typ
check.assignVar(lhs, nil, &x)
check.assignVar(lhs, nil, &x, "assignment") // error is on variable, use "assignment" not "range clause"
}
}
} else if constIntRange {
// If we don't have any iteration variables, we still need to
// check that a (possibly untyped) integer range expression x
// is valid.
// We do this by checking the assignment _ = x. This ensures
// that an untyped x can be converted to a value of type int.
check.assignment(&x, nil, "range clause")
}

check.stmt(inner, s.Body)
Expand Down
9 changes: 4 additions & 5 deletions src/go/types/assignments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (check *Checker) lhsVar(lhs ast.Expr) Type {
// assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil).
// If x != nil, it must be the evaluation of rhs (and rhs will be ignored).
// If the assignment check fails and x != nil, x.mode is set to invalid.
func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) {
func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand, context string) {
T := check.lhsVar(lhs) // nil if lhs is _
if !isValid(T) {
if x != nil {
Expand All @@ -254,8 +254,7 @@ func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) {
check.expr(target, x, rhs)
}

context := "assignment"
if T == nil {
if T == nil && context == "assignment" {
context = "assignment to _ identifier"
}
check.assignment(x, T, context)
Expand Down Expand Up @@ -453,7 +452,7 @@ func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
// each value can be assigned to its corresponding variable.
if l == r && !isCall {
for i, lhs := range lhs {
check.assignVar(lhs, orig_rhs[i], nil)
check.assignVar(lhs, orig_rhs[i], nil, "assignment")
}
return
}
Expand All @@ -474,7 +473,7 @@ func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
r = len(rhs)
if l == r {
for i, lhs := range lhs {
check.assignVar(lhs, nil, rhs[i])
check.assignVar(lhs, nil, rhs[i], "assignment")
}
// Only record comma-ok expression if both assignments succeeded
// (go.dev/issue/59371).
Expand Down
34 changes: 24 additions & 10 deletions src/go/types/stmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
if x.mode == invalid {
return
}
check.assignVar(s.X, nil, &x)
check.assignVar(s.X, nil, &x, "assignment")

case *ast.AssignStmt:
switch s.Tok {
Expand Down Expand Up @@ -492,7 +492,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
if x.mode == invalid {
return
}
check.assignVar(s.Lhs[0], nil, &x)
check.assignVar(s.Lhs[0], nil, &x, "assignment")
}

case *ast.GoStmt:
Expand Down Expand Up @@ -833,7 +833,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {

func (check *Checker) rangeStmt(inner stmtContext, s *ast.RangeStmt) {
// Convert go/ast form to local variables.
type expr = ast.Expr
type Expr = ast.Expr
type identType = ast.Ident
identName := func(n *identType) string { return n.Name }
sKey, sValue := s.Key, s.Value
Expand Down Expand Up @@ -890,8 +890,10 @@ func (check *Checker) rangeStmt(inner stmtContext, s *ast.RangeStmt) {
// (irregular assignment, cannot easily map to existing assignment checks)

// lhs expressions and initialization value (rhs) types
lhs := [2]expr{sKey, sValue}
rhs := [2]Type{key, val} // key, val may be nil
lhs := [2]Expr{sKey, sValue} // sKey, sValue may be nil
rhs := [2]Type{key, val} // key, val may be nil

constIntRange := x.mode == constant_ && isInteger(x.typ)

if isDef {
// short variable declaration
Expand All @@ -918,11 +920,13 @@ func (check *Checker) rangeStmt(inner stmtContext, s *ast.RangeStmt) {
}

// initialize lhs variable
if typ := rhs[i]; typ != nil {
if constIntRange {
check.initVar(obj, &x, "range clause")
} else if typ := rhs[i]; typ != nil {
x.mode = value
x.expr = lhs // we don't have a better rhs expression to use here
x.typ = typ
check.initVar(obj, &x, "range clause")
check.initVar(obj, &x, "assignment") // error is on variable, use "assignment" not "range clause"
} else {
obj.typ = Typ[Invalid]
obj.used = true // don't complain about unused variable
Expand All @@ -938,19 +942,29 @@ func (check *Checker) rangeStmt(inner stmtContext, s *ast.RangeStmt) {
} else {
check.error(noNewVarPos, NoNewVar, "no new variables on left side of :=")
}
} else {
} else if sKey != nil /* lhs[0] != nil */ {
// ordinary assignment
for i, lhs := range lhs {
if lhs == nil {
continue
}
if typ := rhs[i]; typ != nil {

if constIntRange {
check.assignVar(lhs, nil, &x, "range clause")
} else if typ := rhs[i]; typ != nil {
x.mode = value
x.expr = lhs // we don't have a better rhs expression to use here
x.typ = typ
check.assignVar(lhs, nil, &x)
check.assignVar(lhs, nil, &x, "assignment") // error is on variable, use "assignment" not "range clause"
}
}
} else if constIntRange {
// If we don't have any iteration variables, we still need to
// check that a (possibly untyped) integer range expression x
// is valid.
// We do this by checking the assignment _ = x. This ensures
// that an untyped x can be converted to a value of type int.
check.assignment(&x, nil, "range clause")
}

check.stmt(inner, s.Body)
Expand Down
68 changes: 67 additions & 1 deletion src/internal/types/testdata/spec/range_int.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@

package p

// test framework assumes 64-bit int/uint sizes by default
const (
maxInt = 1<<63 - 1
maxUint = 1<<64 - 1
)

type MyInt int32

func _() {
Expand Down Expand Up @@ -38,7 +44,7 @@ func _() {
for i, j /* ERROR "range over 10 (untyped int constant) permits only one iteration variable" */ := range 10 {
_, _ = i, j
}
for i /* ERROR "cannot use i (value of type MyInt) as int value in assignment" */ = range MyInt(10) {
for i = range MyInt /* ERROR "cannot use MyInt(10) (constant 10 of type MyInt) as int value in range clause" */ (10) {
_ = i
}
for mi := range MyInt(10) {
Expand All @@ -63,3 +69,63 @@ func _[T ~int](x T) {
for range x { // ok
}
}

func issue65133() {
for range maxInt {
}
for range maxInt /* ERROR "cannot use maxInt + 1 (untyped int constant 9223372036854775808) as int value in range clause (overflows)" */ + 1 {
}
for range maxUint /* ERROR "cannot use maxUint (untyped int constant 18446744073709551615) as int value in range clause (overflows)" */ {
}

for i := range maxInt {
_ = i
}
for i := range maxInt /* ERROR "cannot use maxInt + 1 (untyped int constant 9223372036854775808) as int value in range clause (overflows)" */ + 1 {
_ = i
}
for i := range maxUint /* ERROR "cannot use maxUint (untyped int constant 18446744073709551615) as int value in range clause (overflows)" */ {
_ = i
}

var i int
_ = i
for i = range maxInt {
}
for i = range maxInt /* ERROR "cannot use maxInt + 1 (untyped int constant 9223372036854775808) as int value in range clause (overflows)" */ + 1 {
}
for i = range maxUint /* ERROR "cannot use maxUint (untyped int constant 18446744073709551615) as int value in range clause (overflows)" */ {
}

var j uint
_ = j
for j = range maxInt {
}
for j = range maxInt + 1 {
}
for j = range maxUint {
}
for j = range maxUint /* ERROR "cannot use maxUint + 1 (untyped int constant 18446744073709551616) as uint value in range clause (overflows)" */ + 1 {
}

for range 256 {
}
for _ = range 256 {
}
for i = range 256 {
}
for i := range 256 {
_ = i
}

var u8 uint8
_ = u8
for u8 = range - /* ERROR "cannot use -1 (untyped int constant) as uint8 value in range clause (overflows)" */ 1 {
}
for u8 = range 0 {
}
for u8 = range 255 {
}
for u8 = range 256 /* ERROR "cannot use 256 (untyped int constant) as uint8 value in range clause (overflows)" */ {
}
}

0 comments on commit 5f949c4

Please sign in to comment.