cmd/go: vet incorrectly flags loop variable captured by func literal in GOPATH mode

[rittneje]

Go version

go version go1.22.9 darwin/amd64

Output of go env in your module/workspace:

n/a

What did you do?

I wrote the follow unit test in GOPATH mode.

package looptest

import "testing"

func TestLoopSemantics(t *testing.T) {
	values := []string{"1", "2"}
	for _, x := range values {
		t.Run(x, func(t *testing.T) {
			t.Parallel()
			t.Log(&x)
		})
	}
}

Then I ran go vet on the package.

What did you see happen?

$ go vet looptest
# looptest
# [looptest]
src/looptest/loop_test.go:10:11: loop variable x captured by func literal

What did you expect to see?

No issues.

In Go 1.22+, the compiler uses the new loop semantics, even in GOPATH mode. This is proven by running the test, which gives output like so. (The exact address is different each time of course, but the important part is that x has a different address in each test case.)

$ go test looptest -v
=== RUN   TestLoopSemantics
=== RUN   TestLoopSemantics/1
=== PAUSE TestLoopSemantics/1
=== RUN   TestLoopSemantics/2
=== PAUSE TestLoopSemantics/2
=== CONT  TestLoopSemantics/1
=== CONT  TestLoopSemantics/2
=== NAME  TestLoopSemantics/1
    loop_test.go:10: 0xc000032350
=== NAME  TestLoopSemantics/2
    loop_test.go:10: 0xc0000323d0
--- PASS: TestLoopSemantics (0.00s)
    --- PASS: TestLoopSemantics/1 (0.00s)
    --- PASS: TestLoopSemantics/2 (0.00s)
PASS
ok      looptest        0.498s

Therefore, go vet should not have applied this check at all.

[gabyhelp]

Related Issues

• Go vet misses closure using variable from outer loop #32876 (closed)
• cmd/vet: -rangeloops check only checks last statement in range loop body #21412 (closed)
• cmd/vet: rangeloop: inspect all go/defer statements prior to a loop back edge #30649 (closed)
• cmd/vet: don't report "loop variable captured" error for versions >= go1.22 #63891 (closed)
• cmd/vet: complains about captured range var that's actually a field name #13236 (closed)
• cmd/vet: warn about capturing loop iterator variables #16520 (closed)
• x/tools/cmd/vet: not always warning about capturing range variable in function literal #8732 (closed)
• cmd/go: always pass a GoVersion to cmd/vet #65612 (closed)
• spec: less error-prone loop variable scoping #60078 (closed)

Related Documentation

• Go 1.22 Release Notes > Tools > Vet > References to loop variables

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mknyszek]

CC @golang/tools-team

[findleyr]

CC @adonovan @timothy-king

It sounds like the loopvar analyzer needs to be made aware of this behavior in GOPATH mode.

@matloob can you clarify the semantics here? Should we assume the effective language version in GOPATH mode is the runtime version?

[ianlancetaylor]

My understanding is that in the absence of a go.mod file the go tool uses Go 1.16 semantics. So the warning is correct when not using modules.

[rsc]

@ianlancetaylor I think we intended to limit GOPATH mode to Go 1.16 at first, but when we decided to keep GOPATH mode around for people who are for one reason or another not using modules, we decided to let the language version float forward, since otherwise you couldn't use generics ever, for example. The behavior of the 'go test' confirms that. Or at least, 'go test' and 'go vet' clearly disagree here.

[ianlancetaylor]

Ah, thanks.

[matloob]

This was previously reported as #65612 in the command-line-arguments context, but it's the same underlying issue for GOPATH. We fixed this in 1.23 with https://go.dev/cl/593156, explicitly setting the go version to the toolchain version when outside of a module.

We considered backporting the change to Go 1.22 but decided not to because it didn't seem to be a serious problem that prevented a program from working at all (and Go 1.22 already had issues with not passing in -lang to the compiler that we wanted to fix at the same time, and would change the behavior for building programs).

(edit: also good job gaby for surfacing #65612 in the related issues!)

[findleyr]

Thanks @matloob, I'd missed (or forgotten) that context.

I think it's reasonable not to backport this fix for 1.22 GOPATH mode. Therefore, closing as a dupe.

[zigo101]

Just FRI: The PR https://go.dev/cl/593156 tired to fix #66092 but not totally.