cmd/fix,x/tools/go/analysis/passes/modernize: rangeint uses target platform's type in the range expression, breaking other platforms

[dagood]

Go version

go version go1.26.0 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/dagood/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/dagood/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2735933558=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/dagood/go/pkg/mod'
GONOPROXY=[...]
GONOSUMDB=[...]
GOOS='linux'
GOPATH='/home/dagood/go'
GOPRIVATE=[...]
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/dagood/sdk/go1.26.0'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/dagood/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/dagood/sdk/go1.26.0/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.26.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

This main.go file gives us a for loop over int64 on linux-amd64 but int32 on linux-arm:

package main

import "syscall"

func main() {
	var ts syscall.Timespec
	// tries needs to be the same type as syscall.Timespec.Nsec
	// but the fields are int32 on 32bit and int64 on 64bit.
	// tries is assigned to syscall.Timespec.Nsec in order to match its type.
	tries := ts.Nsec
	for tries = 0; tries < 20; tries++ {
		println(tries)
	}
}

I'm trying to use go fix on a larger repo that includes some code like this, and it broke that code and caused PR testing to fail. (Just to say that it happened in a real project, not that it's necessarily severe. 😄 This is admittedly a bit of an odd case.)

What did you see happen?

The go fix output depends on the current platform, and produces code that isn't compatible between platforms:

$ go fix -rangeint -diff ./main.go
--- /home/dagood/try/fix/main.go (old)
+++ /home/dagood/try/fix/main.go (new)
@@ -8,7 +8,7 @@
        // but the fields are int32 on 32bit and int64 on 64bit.
        // tries is assigned to syscall.Timespec.Nsec in order to match its type.
        tries := ts.Nsec
-       for tries = 0; tries < 20; tries++ {
+       for tries = range int64(20) {
                println(tries)
        }
 }

$ GOARCH=arm go fix -rangeint -diff ./main.go
--- /home/dagood/try/fix/main.go (old)
+++ /home/dagood/try/fix/main.go (new)
@@ -8,7 +8,7 @@
        // but the fields are int32 on 32bit and int64 on 64bit.
        // tries is assigned to syscall.Timespec.Nsec in order to match its type.
        tries := ts.Nsec
-       for tries = 0; tries < 20; tries++ {
+       for tries = range int32(20) {
                println(tries)
        }
 }

Specifically, if I try to use the fixed code in a linux-arm build, I get a build error:

$ go fix -rangeint ./main.go
$ GOARCH=arm go build ./main.go
# command-line-arguments
./main.go:11:20: cannot use int64(20) (constant 20 of type int64) as int32 value in range clause

What did you expect to see?

The result should compile.

I would expect this code result, but I don't know if there are subtleties that make it infeasible:

	for tries = range 20 {
		println(tries)
	}

[gabyhelp]

Related Issues

• x/tools/gopls/internal/analysis/modernize: rangeint should be silent when RHS of 'i < limit' depends on loop var #72726 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint generates correct but not idiomatic transformation #71725 (closed)
• x/tools/gopls: modernize's rangeint check is too aggressive #74295 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint: transformation unsound when loop variable is defined outside loop #73009 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint suggests changes that alter some programs' semantics #75289 (closed)
• x/tools/go/analysis/passes/modernize: doesn't convert 'for i := 0; i < 10; i += 1 {}' #76977 (closed)
• x/tools/go/analysis/passes/modernize: bad rangeint rewriting #77034 (closed)
• x/tools/gopls/internal/analysis/modernize: rangeint produces invalid syntax due to use of types.RelativeTo #74687 (closed)
• x/tools/gopls/internal/analyze/modernize: rangeint: transformation introduces invalid type #73037 (closed)
• x/tools/gopls/internal/analysis/modernize: range over int rule doesn't consider potential slice appends #73022 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mrkfrmn]

cc @golang/tools-team

[madelinekalil]

Thank you very much for the bug report. We shouldn't need a type cast in this case because the init value is assigned outside the loop, and the default type of the limit is an integer, so the compiler can infer the type. Will send a fix

[gopherbot]

Change https://go.dev/cl/747560 mentions this issue: go/analysis/passes/modernize: rangeint: omit unneeded type cast

[qmuntal]

@gopherbot please consider this for backport to 1.26, it's generating code that doesn't compile on some platforms.

[gopherbot]

Backport issue(s) opened: #77766 (for 1.26).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.