We propose to change for loop variables declared with := from one-instance-per-loop to one-instance-per-iteration. This change would apply only to packages in modules that explicitly declare a new enough Go version in the go.mod file, so all existing code is unaffected. Changing the loop semantics would prevent unintended sharing in per-iteration closures and goroutines (see this entry in the Go FAQ for one explanation). We expect this change to fix many subtly broken for loops in new code, as well as in old code as it is updated to the newer Go version. There was an earlier pre-proposal discussion of this idea at #56010.
For example, consider a loop like the one in this test:
func TestAllEven(t *testing.T) {
testCases := []int{0, 2, 4, 6}
for _, v := range testCases {
t.Run("sub", func(t *testing.T) {
t.Parallel()
if v&1 != 0 {
t.Fatal("odd v", v)
}
})
}
}
This test aims to check that all the test cases are even, but it doesn’t check them all. The problem is that t.Parallel stops the closure and lets the loop continue, and then it runs all the closures in parallel when the loop is over and TestAllEven has returned. By the time the if statement in the closure executes, the loop is done, and v has its final iteration value, 6. All four subtests now continue executing in parallel, and they all check that 6 is even, instead of checking each of the test cases.
Most Go developers are familiar with this mistake and know the answer: add v := v to the loop body:
func TestAllEven(t *testing.T) {
testCases := []int{0, 2, 4, 6}
for _, v := range testCases {
v := v // MAKE ITERATION VALUE SHARING BUG GO AWAY
t.Run("sub", func(t *testing.T) {
t.Parallel()
if v&1 != 0 {
t.Fatal("odd v", v)
}
})
}
}
Changing the loop semantics would in essence insert this kind of v := v statement for every for loop variable declared with := . It would fix this loop and many others to do what the author clearly intends.
A subtler variation is when the code says testCases := []int{1, 2, 4, 6} (note the non-even test case 1):
func TestAllEvenBuggy(t *testing.T) {
testCases := []int{1, 2, 4, 6}
for _, v := range testCases {
t.Run("sub", func(t *testing.T) {
t.Parallel()
if v&1 != 0 {
t.Fatal("odd v", v)
}
})
}
}
TestAllEvenBuggy passes today, because the test is only checking 6, four times. Changing the loop semantics would still fix the loop to do what the author clearly intended, but it would break the test, because the test is passing incorrectly. So there is the potential to cause problems for users.
Because of this potential for problems, the new loop semantics would only apply in Go modules that have opted in to the release with the new loops. If that was Go 1.22, then only packages in a module with a go.mod that says go 1.22 would get the new loop semantics. Packages in other modules, including packages in dependencies, would get the old semantics. This would guarantee that all existing Go code keeps working the same as it does today, even when compiling with a new toolchain. People only need to debug loop changes when they opt in to the new semantics in go.mod . This approach is in keeping with our backwards and forwards compatibility work, #56986 and #57001, specifically the principle that toolchain upgrades preserve the behavior of old code, and compatibility is based on the go line.
If this proposal is accepted, users will need additional tooling support for a successful transition. That would come primarily in two forms: a compiler mode that reports affected loops, and a debugging tool that identifies the specific loops whose changed compilation is responsible for causing a test failure. There is a demo of the tooling support at the end of this comment.
The tooling support is important and necessary, but we expect it to be needed only rarely. Analysis and conversion of Google’s own Go source code found that only about 1 package test per 8,000 started failing due to the new semantics, and the bug was essentially always in the test itself, like in TestAllEvenBuggy . In contrast, the new loopclosure vet analysis that shipped in Go 1.20 flagged definite bugs in 1 test per 400. The rest stayed passing with their bugs fixed by the new semantics. That is, in Google’s code base, about 5% of the tests that contain this kind of sharing mistake were like TestAllEvenBuggy , exposed as buggy by the new semantics. The other 95% of the tests with this kind of
|