cmd/fix, x/tools/go/analysis/passes/modernize: stditerators rewrites allocation-free reflect field loops into heap-allocating Value.Fields form

Go version

go version go1.27.1 linux/amd64

(All commands and benchmarks below ran inside the stock golang:1.27-bookwormDocker image.)

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

What did you do?

Ran go fix ./... with go1.27.1 on code using the standard reflect struct-field walk. The stditerators analyzer (CL 717620) rewrites

for i := range v.NumField() {
	field := v.Field(i)
	...
}

into

for _, field := range v.Fields() {
	...
}

I benchmarked both forms of a small recursive reflect walk (repro below).

What did you see happen?

go1.27.1, linux/amd64:

benchmark (classic → rewritten)	ns/op	allocs/op	B/op
IsZero, flat 6-field struct (1 struct walk)	52 → 385	0 → 10	0 → 103
IsZero, nested row, 4 struct walks, early return	142 → 1180	0 → 31	0 → 403
Count, nested row, 4 struct walks, no early exit	106 → 1003	0 → 12	0 → 352

7.4–9.5× slower. The rewrite is invisible to tests: results are identical (the repro includes a sanity test asserting equivalence).

Allocation profile of the rewritten flat case: roughly half of the objects come from reflect.Value.Fields itself — the returned closure boxes the captured Type and Value; the other half come from the consumer side — the range-over-func body closure and its state, with extra allocations when the loop body exits early via return. This is the mechanism tracked in #80714 (compiler fails to eliminate allocation of method values returned as iter.Seq) and #69015; neither of those covers the go fix/modernize blast radius.

What did you expect to see?

Either the rewrite to be allocation-neutral on this idiom, or the reflect iterator rewrites not applied by default until the compiler can elide the closures (#80714). At minimum, the allocation cost deserves a mention in the analyzer docs / release notes, because go fix ./... applies it tree-wide.

Impact

Hot reflect paths (codecs, validators, ORMs, struct-tag walkers) pay per value visited, so the cost scales with data. In a reflect-based serialization codec, a bulk go fix apply produced a 3.4× mallocs regression on encode paths with the full test suite green (byte-identical output); it was caught only by allocation-diff benchmarking, and had to be reverted at six sites with -stditerators=false pinned in CI (gbon-format/gbon-go, scripts/gate-inner.sh).

Repro

go mod init stditeratorsrepro && go mod edit -go=1.27, then four files.

iszero_classic.go (what go fix flags; *iter.go are the exact analyzer output, function renamed only so both can coexist in one benchmark):

package repro

import "reflect"

func IsZeroClassic(v reflect.Value) bool {
	switch v.Kind() {
	case reflect.Struct:
		for i := range v.NumField() {
			if !IsZeroClassic(v.Field(i)) {
				return false
			}
		}
		return true
	default:
		return v.IsZero()
	}
}

iszero_iter.go:

package repro

import "reflect"

func IsZeroIter(v reflect.Value) bool {
	switch v.Kind() {
	case reflect.Struct:
		for _, field := range v.Fields() {
			if !IsZeroIter(field) {
				return false
			}
		}
		return true
	default:
		return v.IsZero()
	}
}

count_classic.go / count_iter.go: same pair with n += Count(field) bodies (no early exit), to show the best case.

iszero_test.go:

package repro

import (
	"reflect"
	"testing"
)

type Inner struct{ A, B, C int }

type Flat struct{ A, B, C, D, E, F int }

type Row struct {
	F0 int
	F1 string
	F2 Inner
	F3 []int
	F4 bool
	F5 float64
	F6 Inner
	F7 string
	F8 Inner
}

var row Row
var flat Flat

var sink bool

func TestSameResults(t *testing.T) {
	for _, x := range []any{flat, row, Inner{1, 2, 3}, 42, "s"} {
		v := reflect.ValueOf(x)
		if IsZeroClassic(v) != IsZeroIter(v) {
			t.Fatal("IsZero mismatch")
		}
	}
	if CountClassic(reflect.ValueOf(row)) != CountIter(reflect.ValueOf(row)) {
		t.Fatal("Count mismatch")
	}
}

func benchIsZero(b *testing.B, f func(reflect.Value) bool, x any) {
	v := reflect.ValueOf(x)
	b.ReportAllocs()
	for b.Loop() {
		sink = f(v)
	}
}

func BenchmarkIsZeroClassicFlat(b *testing.B) { benchIsZero(b, IsZeroClassic, flat) }
func BenchmarkIsZeroIterFlat(b *testing.B)    { benchIsZero(b, IsZeroIter, flat) }
func BenchmarkIsZeroClassicRow(b *testing.B)  { benchIsZero(b, IsZeroClassic, row) }
func BenchmarkIsZeroIterRow(b *testing.B)     { benchIsZero(b, IsZeroIter, row) }

func benchCount(b *testing.B, f func(reflect.Value) int) {
	v := reflect.ValueOf(row)
	b.ReportAllocs()
	for b.Loop() {
		sink = f(v) > 0
	}
}

func BenchmarkCountClassicRow(b *testing.B) { benchCount(b, CountClassic) }
func BenchmarkCountIterRow(b *testing.B)    { benchCount(b, CountIter) }

Run:

go test -run TestSameResults -count 1 .   # PASS
go fix -diff ./...                        # proposes both classic → iter rewrites
go test -run '^$' -bench . -benchmem -count 3 .

Suggestion

Consider excluding the reflect rewrites (NumField/Field → Fields, NumMethod/Method → Methods, In/Out → Ins) from stditerators, or from the default go fix set, until #80714 is resolved; alternatively document the allocation cost next to the fixit.

Related: #66631 (proposal that added the API), #80714, #69015, #69411, #69539.