Go version
go version go1.24.1 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/jille/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/jille/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2881335881=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/jille/src/ilsa-sanity/go.mod'
GOMODCACHE='/home/jille/go/pkg/mod'
GONOPROXY=''
GONOSUMDB='src.hexon.nl/*'
GOOS='linux'
GOPATH='/home/jille/go'
GOPRIVATE=''
GOROOT='/usr/lib/go-1.24'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/jille/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go-1.24/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.1'
GOWORK=''
PKG_CONFIG='pkg-config'
What did you do?
I ran modernize which upgraded some benchmarking loops from for i := 0; i < b.N; i++ {} to for b.Loop {} on https://github.com/bluele/psort/blob/master/psort_test.go.
What did you see happen?
The benchmark times out after 10 minutes, whereas it previously finished in seconds.
What did you expect to see?
The code changed by modernize should do the exact same thing.
I suspect we need to teach modernize not to touch loops which call b.StopTimer() :(
Comment From: gabyhelp
Related Issues
- x/tools/gopls/internal/analysis/modernize: replacing non-equivalent code #73073 (closed)
- testing: b.StopTimer breaks b.Loop #72922 (closed)
- x/tools/gopls: modernizer max changes logic #71878 (closed)
- x/tools/gopls/internal/analysis/modernize: rangeint: transformation unsound when loop variable is defined outside loop #73009 (closed)
- x/tools/gopls: modernize's rangeint check is too aggressive #74295 (closed)
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
Comment From: Jille
As @gabyhelp nicely found is that this issue is fixed in https://github.com/golang/go/issues/72922, which was backported to Go 1.24.2.
What is the policy for this? Do we expect users to just upgrade, or should modernize take this into account somehow?
Comment From: findleyr
Hmm, @adonovan is doing a talk at GopherCon about the difficulty of writing modernizers. This would be a good edge case to include!
Generally speaking, modernizers should never break your code (that's part of the idea), but this issue illustrates part of why that's so hard to guarantee!
Milestoning for the next release. I suppose the simplest fix is to disable the Bloop modernizer if the Go version is <1.24.2. Too much work to allow it if StopTimer/StartTimer are not called.