[tools] go/analysis/passes/modernize: rangeint: omit unneeded type cast
4 views
Skip to first unread message
Madeline Kalil (Gerrit)
Feb 20, 2026, 4:06:19 PM (7 days ago) Feb 20
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil has uploaded the change for review![]( "Open in Gerrit")
Commit message
go/analysis/passes/modernize: rangeint: omit unneeded type cast
cl/653616 introduced a type conversion on the limit of a "for range"
if the limit is not assignable to an int. However, this is not
necessary if the init value is created outside of the loop and
if the default value of the limit is an integer because the compiler
can infer the type of the iteration values without the type cast.
Fixes golang/go#77691
Change-Id: I6f2cbf3878932881596194b636d4a362dd3b9fb2
Change diff
diff --git a/go/analysis/passes/modernize/rangeint.go b/go/analysis/passes/modernize/rangeint.go
index 257f9e9..3a666f7 100644
--- a/go/analysis/passes/modernize/rangeint.go
+++ b/go/analysis/passes/modernize/rangeint.go
@@ -230,7 +230,7 @@
// such as "const limit = 1e3", its effective type may
// differ between the two forms.
// In a for loop, it must be comparable with int i,
- // for i := 0; i < limit; i++
+ // for i := 0; i < limit; i++ {}
// but in a range loop it would become a float,
// for i := range limit {}
// which is a type error. We need to convert it to int
@@ -253,6 +253,19 @@
if types.AssignableTo(tLimit, tVar) {
beforeLimit, afterLimit = "", ""
}
+ // Check if the limit's default type is an integer.
+ // If it is, and we are assigning to an existing variable, the compiler
+ // handles the type context automatically, so we don't need a cast.
+ // e.g.
+ // var i int64
+ // for i = 0; i < 10; i ++ {} --> for i = range 10 {}
+ // This also prevents us from adding a cast that would break non-target builds,
+ // if "i" uses an architecture-dependent type such as syscall.Timespec.Nsec.
+ if init.Tok == token.ASSIGN { // init is assigning to existing var
+ if b, ok := tLimit.Underlying().(*types.Basic); ok && b.Info()&types.IsInteger != 0 {
+ beforeLimit, afterLimit = "", ""
+ }
+ }
}
}
diff --git a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
index 0ec4374..567a962 100644
--- a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
+++ b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
@@ -324,3 +324,30 @@
}
}
}
+
+type C int32
+
+// Don't add an unneeded type cast if the limit's default type is an integer, and
+// the init value is assigned outside the loop, because the compiler can infer the type.
+func issue77891() {
+ const limit_float = 1e6
+ var i int
+ for i = 0; i < limit_float; i++ { // want "for loop can be modernized using range over int"
+ println(i)
+ }
+
+ var j int64
+ for j = 0; j < 20; j++ { // want "for loop can be modernized using range over int"
+ println(j)
+ }
+
+ var c C
+ for c = 0; c < 10; c++ { // want "for loop can be modernized using range over int"
+ println(c)
+ }
+
+ var ptr uintptr
+ for ptr = 0; ptr < 100; ptr++ { // want "for loop can be modernized using range over int"
+ println(ptr)
+ }
+}
diff --git a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
index ad98069..e02731d 100644
--- a/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
@@ -323,3 +323,30 @@
}
}
}
+
+type C int32
+
+// Don't add an unneeded type cast if the limit's default type is an integer, and
+// the init value is assigned outside the loop, because the compiler can infer the type.
+func issue77891() {
+ const limit_float = 1e6
+ var i int
+ for i = range int(limit_float) { // want "for loop can be modernized using range over int"
+ println(i)
+ }
+
+ var j int64
+ for j = range 20 { // want "for loop can be modernized using range over int"
+ println(j)
+ }
+
+ var c C
+ for c = range 10 { // want "for loop can be modernized using range over int"
+ println(c)
+ }
+
+ var ptr uintptr
+ for ptr = range 100 { // want "for loop can be modernized using range over int"
+ println(ptr)
+ }
+}
\ No newline at end of file
Change information
Files:
- M go/analysis/passes/modernize/rangeint.go
- M go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
- M go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
Change size: M
Delta: 3 files changed, 68 insertions(+), 1 deletion(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 20, 2026, 4:06:25 PM (7 days ago) Feb 20
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 20, 2026, 4:10:43 PM (7 days ago) Feb 20
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #2 to this change.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 24, 2026, 12:11:21 PM (3 days ago) Feb 24
to Madeline Kalil, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Madeline Kalil
Alan Donovan added 4 comments![]( "Open in Gerrit")
Commit Message
Line 9, Patchset 2 (Latest):
cl/653616 introduced a type conversion on the limit of a "for range"Alan Donovan . unresolved
"cl/" relies on Google's corp network, and is not marked up by Gerrit as a link. If you use "CL 653616", Gerrit will linkify it. Alternatively, use https://go.dev/cl/653616 anywhere.
For issues, if you use #XXX (main repo) or golang/go#XXX (x/tools) in CL descriptions (etc) then both Gerrit and GitHub will mark these up as links. Alternatively you can use https://go.dev/issue/XXX anywhere.
File go/analysis/passes/modernize/rangeint.go
Line 257, Patchset 2 (Latest):
// If it is, and we are assigning to an existing variable, the compiler
// handles the type context automatically, so we don't need a cast.Alan Donovan . unresolved
I see that this change makes the new test pass, but why is the logic below not a special case of the existing AssignableTo(untyped float, int64) check, which returns true?
Line 262, Patchset 2 (Latest):
// This also prevents us from adding a cast that would break non-target builds,Alan Donovan . unresolved
"Cast" is a term from C; in Go we call it a conversion.
This might be clearer as:
```
// Redundant casts are not only unsightly but may in some cases cause
// architecture-specific types (e.g. syscall.Timespec.Nsec) to be inserted
// into otherwise portable files.
```
Line 265, Patchset 2 (Latest):
if b, ok := tLimit.Underlying().(*types.Basic); ok && b.Info()&types.IsInteger != 0 {Alan Donovan . unresolved
Let's pull this local function out of `unsafefuncs` to package level, and use it again here:
```
func isInteger(t types.Type) bool {
basic, ok := t.Underlying().(*types.Basic)
return ok && basic.Info()&types.IsInteger != 0
}
```
Related details
Attention is currently required from:
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 24, 2026, 2:42:57 PM (3 days ago) Feb 24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Madeline Kalil
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 24, 2026, 2:43:11 PM (3 days ago) Feb 24
to goph...@pubsubhelper.golang.org, Alan Donovan, Go LUCI, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Madeline Kalil voted and added 4 comments![]( "Open in Gerrit")
Votes added by Madeline Kalil
| Commit-Queue | +1 |
4 comments
Commit Message
Line 9, Patchset 2:
cl/653616 introduced a type conversion on the limit of a "for range"Alan Donovan . resolved
"cl/" relies on Google's corp network, and is not marked up by Gerrit as a link. If you use "CL 653616", Gerrit will linkify it. Alternatively, use https://go.dev/cl/653616 anywhere.
For issues, if you use #XXX (main repo) or golang/go#XXX (x/tools) in CL descriptions (etc) then both Gerrit and GitHub will mark these up as links. Alternatively you can use https://go.dev/issue/XXX anywhere.
Madeline Kalil
Thanks! Done
File go/analysis/passes/modernize/rangeint.go
Line 257, Patchset 2:
// If it is, and we are assigning to an existing variable, the compiler
// handles the type context automatically, so we don't need a cast.Alan Donovan . unresolved
I see that this change makes the new test pass, but why is the logic below not a special case of the existing AssignableTo(untyped float, int64) check, which returns true?
Madeline Kalil
We are passing types.Default, so the types.Type passed to AssignableTo is typed, not untyped. So I don't believe this logic is a special case of the AssignableTo check (unless we change it to check AssignableTo on the "raw" type, which I'm unsure of).
Line 262, Patchset 2:
// This also prevents us from adding a cast that would break non-target builds,Alan Donovan . resolved
"Cast" is a term from C; in Go we call it a conversion.
This might be clearer as:
```
// Redundant casts are not only unsightly but may in some cases cause
// architecture-specific types (e.g. syscall.Timespec.Nsec) to be inserted
// into otherwise portable files.
```
Madeline Kalil
Done
Line 265, Patchset 2:
if b, ok := tLimit.Underlying().(*types.Basic); ok && b.Info()&types.IsInteger != 0 {Alan Donovan . resolved
Let's pull this local function out of `unsafefuncs` to package level, and use it again here:
```
func isInteger(t types.Type) bool {
basic, ok := t.Underlying().(*types.Basic)
return ok && basic.Info()&types.IsInteger != 0
}
```
Attention is currently required from:
- Alan Donovan
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 26, 2026, 4:35:06 PM (23 hours ago) Feb 26
to Madeline Kalil, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Madeline Kalil
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
| Code-Review | +2 |
1 comment
File go/analysis/passes/modernize/rangeint.go
Line 257, Patchset 2:
// If it is, and we are assigning to an existing variable, the compiler
// handles the type context automatically, so we don't need a cast.Alan Donovan . unresolved
Madeline KalilI see that this change makes the new test pass, but why is the logic below not a special case of the existing AssignableTo(untyped float, int64) check, which returns true?
We are passing types.Default, so the types.Type passed to AssignableTo is typed, not untyped. So I don't believe this logic is a special case of the AssignableTo check (unless we change it to check AssignableTo on the "raw" type, which I'm unsure of).
Alan Donovan
Still I would have expected AssignableTo to work for untyped types.
...
Ah, the problem is that removing Default causes us to check (say, for a limit of `1e6`) whether untyped float is assignable to int: it is... but we should always keep the conversion if the limit is non-integer. That leads to a more natural factoring:
```
tLimit := info2.TypeOf(limit)
// Eliminate conversion when safe.
//
// Redundant conversions are not only unsightly but may in some cases cause
// architecture-specific types (e.g. syscall.Timespec.Nsec) to be inserted
// into otherwise portable files.
//
// The operand must have an integer type (not, say, '1e6')
// even when assigning to an existing integer variable.
if isInteger(tLimit) {
// When declaring a new var from an untyped limit,
// the limit's default type is what matters.
if init.Tok != token.ASSIGN {
tLimit = types.Default(tLimit)
}
if types.AssignableTo(tLimit, tVar) {
beforeLimit, afterLimit = "", ""
}
}
Attention is currently required from:
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 26, 2026, 5:36:50 PM (22 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Madeline Kalil
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #4 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 26, 2026, 5:38:23 PM (22 hours ago) Feb 26
to goph...@pubsubhelper.golang.org, Alan Donovan, Go LUCI, golang-co...@googlegroups.com
Madeline Kalil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Madeline Kalil
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
File go/analysis/passes/modernize/rangeint.go
Line 257, Patchset 2:
// If it is, and we are assigning to an existing variable, the compiler
// handles the type context automatically, so we don't need a cast.Alan Donovan . resolved
Madeline KalilI see that this change makes the new test pass, but why is the logic below not a special case of the existing AssignableTo(untyped float, int64) check, which returns true?
Alan DonovanWe are passing types.Default, so the types.Type passed to AssignableTo is typed, not untyped. So I don't believe this logic is a special case of the AssignableTo check (unless we change it to check AssignableTo on the "raw" type, which I'm unsure of).
Still I would have expected AssignableTo to work for untyped types.
...
Ah, the problem is that removing Default causes us to check (say, for a limit of `1e6`) whether untyped float is assignable to int: it is... but we should always keep the conversion if the limit is non-integer. That leads to a more natural factoring:```
tLimit := info2.TypeOf(limit)// Eliminate conversion when safe.
//
// Redundant conversions are not only unsightly but may in some cases cause
// architecture-specific types (e.g. syscall.Timespec.Nsec) to be inserted
// into otherwise portable files.
//
// The operand must have an integer type (not, say, '1e6')
// even when assigning to an existing integer variable.
if isInteger(tLimit) {
// When declaring a new var from an untyped limit,
// the limit's default type is what matters.
if init.Tok != token.ASSIGN {
tLimit = types.Default(tLimit)
}
if types.AssignableTo(tLimit, tVar) {
beforeLimit, afterLimit = "", ""
}
}
```
Madeline Kalil
Makes sense, thanks
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
10:29 AM (5 hours ago) 10:29 AM
to goph...@pubsubhelper.golang.org, Go LUCI, Alan Donovan, golang-co...@googlegroups.com
Madeline Kalil voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
11:29 AM (4 hours ago) 11:29 AM
to Madeline Kalil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Alan Donovan, golang-co...@googlegroups.com
Gopher Robot submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: go/analysis/passes/modernize/rangeint.go
Insertions: 15, Deletions: 12.
@@ -249,21 +249,24 @@
beforeLimit, afterLimit = fmt.Sprintf("%s(", types.TypeString(tVar, qual)), ")"
info2 := &types.Info{Types: make(map[ast.Expr]types.TypeAndValue)}
if types.CheckExpr(pass.Fset, pass.Pkg, limit.Pos(), limit, info2) == nil {
- tLimit := types.Default(info2.TypeOf(limit))
- if types.AssignableTo(tLimit, tVar) {
- beforeLimit, afterLimit = "", ""
- }
- // Check if the limit's default type is an integer.
- // If it is, and we are assigning to an existing variable, the compiler
- // handles the type context automatically, so we don't need a cast.
- // e.g.
- // var i int64
- // for i = 0; i < 10; i ++ {} --> for i = range 10 {}
+ tLimit := info2.TypeOf(limit)
+ // Eliminate conversion when safe.
+ //
// Redundant conversions are not only unsightly but may in some cases cause
// architecture-specific types (e.g. syscall.Timespec.Nsec) to be inserted
// into otherwise portable files.
- if init.Tok == token.ASSIGN && isInteger(tLimit) { // init is assigning to existing var
- beforeLimit, afterLimit = "", ""
+ //
+ // The operand must have an integer type (not, say, '1e6')
+ // even when assigning to an existing integer variable.
+ if isInteger(tLimit) {
+ // When declaring a new var from an untyped limit,
+ // the limit's default type is what matters.
+ if init.Tok != token.ASSIGN {
+ tLimit = types.Default(tLimit)
+ }
+ if types.AssignableTo(tLimit, tVar) {
+ beforeLimit, afterLimit = "", ""
+ }
}
}
}
```
Change information
Commit message:
go/analysis/passes/modernize: rangeint: omit unneeded type conversion
CL 653616 introduced a type conversion on the limit of a "for range"
if the limit is not assignable to an int. However, this is not
necessary if the init value is created outside of the loop and
if the default value of the limit is an integer because the compiler
can infer the type of the iteration values without the type conversion.
A type conversion can cause build issues if the loop's index uses
an architecture-dependent type such as syscall.Timespec.Nsec.
Fixes golang/go#77691
Change-Id: I6f2cbf3878932881596194b636d4a362dd3b9fb2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/747560
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adon...@google.com>
Auto-Submit: Madeline Kalil <mka...@google.com>
Files:
- M go/analysis/passes/modernize/rangeint.go
- M go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go
- M go/analysis/passes/modernize/testdata/src/rangeint/rangeint.go.golden
- M go/analysis/passes/modernize/unsafefuncs.go
Change size: M
Delta: 4 files changed, 78 insertions(+), 9 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Alan Donovan
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages