x/tools/go/cfg: loop variable declarations are misplaced for 1.22+ loops

[jakebailey]

Go version

go version go1.24.2 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/jabaile/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/jabaile/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1542494774=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/jabaile/work/TypeScript-go/_tools/go.mod'
GOMODCACHE='/home/jabaile/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jabaile/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jabaile/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

func F21(slice []int) {
	for i, value := range slice {
		value, ok := maybeDouble(value)
		if ok {
			slice[i] = value
		}
	}
}

What did you see happen?

.0: # Body@L10
        slice
        i
        value
        succs: 1

.1: # RangeLoop@L11
        succs: 2 3

.2: # RangeBody@L11
        value, ok := maybeDouble(value)
        ok
        succs: 4 5

.3: # RangeDone@L11
        return

.4: # IfThen@L13
        slice[i] = value
        succs: 5

.5: # IfDone@L13
        succs: 1

The loop variables are scoped along with slice as though they are declared outside the loop and are reused each iteration.

What did you expect to see?

Since Go 1.22, each time the loop runs, the loop variables are created anew. I would think that these variable declarations would be placed within RangeLoop so that the code flows into that node (and then analyzers can say "ah, these variables have been redeclared").

This came up via the shadowed variable analyzer I wrote for the TS compiler port. (Which, I would like to upstream at some point but that's another discussion.)

[dmitshur]

CC @adonovan.

[adonovan]

Thanks for reporting; I agree this is a bug.

This came up via the shadowed variable analyzer I wrote for the TS compiler port. (Which, I would like to upstream at some point but that's another discussion.)

Be aware that there is already an analyzer named shadow, but it has never been enabled because its rate of false positives was too high. If you have have an algorithm that reliably reports only true positives, we would consider using it in shadow and enabling it by default.

[jakebailey]

Be aware that there is already an analyzer named shadow, but it has never been enabled because its rate of false positives was too high. If you have have an algorithm that reliably reports only true positives, we would consider using it in shadow and enabling it by default.

Yes, I've been meaning to propose it, though it's not perfect itself (and maybe should use go/ssa instead, I'm just not clever enough to figure out how to do that easily).

You can find more info about it here, if you're interested:

• https://jakebailey.dev/posts/go-shadowing/
• https://github.com/microsoft/typescript-go/blob/main/_tools/customlint/shadow.go
• https://github.com/microsoft/typescript-go/blob/main/_tools/customlint/testdata/shadow/shadow.go.golden