Repository navigation
cmd/fix, x/tools/go/analysis/passes/modernize: stditerators rewrites allocation-free reflect field loops into heap-allocating Value.Fields form #81564
Description
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.
Activity
greedyivan commentedon Sep 16, 2026
Follow-up: mechanism analysis and possible ways to close this, for triage.
Why the rewrite allocates
Three mechanics compound; each is individually reasonable.
1. API shape: the iterator is a returned func value.
Value.Fields()must return aniter.Seq2[StructField, Value]— a function value. A returned closure outlives its frame, so the compiler heap-allocates it together with the captured context (theTypeinterface and the 24-byteValueget boxed into the closure object). This is roughly half of the objects (pprof on the flat case:reflect.Value.Fieldsflat ≈ 50% of alloc_objects). The classic loop creates no func values at all, so it has nothing to box.2. Consumer side: the range-over-func body is also a closure, and it escapes.
for _, field := range v.Fields()lowers to "call seq, passing a yield closure". But seq arrives from a non-inlined call (-gcflags=-m=2on therepro:
cannot inline IsZeroIter: function too complex: cost 243 exceeds budget 80;Fieldsis not inlined either) — an opaque func value that might store yield. Escape analysis verdict, verbatim:func literal escapes to heap. Together with the body, the iterator-protocol machinery escapes too: the range state variable#state1(re-entry check viaruntime.panicrangestate— this one is required by the spec, not optional), and, when the body exits early viareturn, the#rv1/#nextslots (function result + "where to go after the loop").3. Cascade: parameter leak crosses the non-inlined call boundary.
The subtlest part: because
vleaks (it is captured by theFieldsclosure), the parameter is markedleaks to {heap}, and every yieldedfieldpassed to the recursive call is then boxed on every iteration. One leak at the root of the loop turns the whole walk into a per-field allocation.Net effect on the flat 6-field benchmark: 0 → 10 allocs/op, ≈50/50 between the reflect side (returned closure + captures) and the consumer side (yield closure + state machine + exit plumbing + boxed yielded values).
None of these allocations is semantically necessary — they are artifacts of the compiler not being able to see through the chain
"call → returned closure → single immediate call". That capability gap is exactly what #80714 and #69015 track.
Ways to close it
A. Compiler — the root fix (tracked by #80714 / #69015).
Specialize
for range f()for the case "f is a direct call returning a closure": inline the returned closure into the range site and pass yielddirectly, with no func values — both sides stay on the stack. Requires either larger inline budgets or a targeted pass; an alternative route is an escape- analysis refinement for func values that are called exactly once, synchronously (a property the range-over-func shape already implicitly guarantees). Fixes the whole class, not just reflect — but it is compiler-team-scale work.
B. Modernizer policy — pragmatic guard (the ask in this issue).
Keep the reflect rewrites (
NumField/Field → Fields,NumMethod/Method → Methods,In/Out → Ins) out of the default stditerators / defaultgo fixset until A lands; or move them behind an opt-in flag; or at minimum document the allocation cost next to the fixit and in release notes. This does not fix the cause, but it removes the silent tree-wide application on hot paths.C. Userland mitigation — current practice.
Keep the classic loops on hot paths and pin
-stditerators=falsein CI (what we had to do). Works today, but per-project perimeter defense by hand is precisely what B is meant to remove.Realistic combination: B now (cheap, removes the regression risk for everyone), A as the root fix for the class.
Evidence pointers
-gcflags=-m=2on the repro:cannot inline IsZeroIter: ... cost 243 exceeds budget 80;func literal escapes to heap;#state1 escapes to heap;#rv1 escapes to heap;parameter v leaks to {heap}.#tmpState := #state1; #state1 = 2; if #tmpState != 1 { runtime.panicrangestate(#tmpState) }; ... #rv1 = false; #next = -1; #state1 = 0; return false.reflect.Value.Fieldsinternals, ≈50% to the consumer (IsZeroIter+IsZeroIter-range1).gabyhelp commentedon Sep 16, 2026
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
madelinekalil commentedon Sep 17, 2026
Thanks for the bug report! We are hoping to add support for off-by-default analyzers (#81514) which could help address this.