Skip to content

Commit 04980e0

Browse files
committed
go/analysis/passes/loopclosure: go run doc/generate.go
1 parent 1ba80ff commit 04980e0

File tree

2 files changed

+56
-20
lines changed

2 files changed

+56
-20
lines changed

gopls/doc/analyzers.md

+54-18
Original file line numberDiff line numberDiff line change
@@ -218,24 +218,60 @@ inferred from function arguments, or from other type arguments:
218218

219219
check references to loop variables from within nested functions
220220

221-
This analyzer checks for references to loop variables from within a function
222-
literal inside the loop body. It checks for patterns where access to a loop
223-
variable is known to escape the current loop iteration:
224-
1. a call to go or defer at the end of the loop body
225-
2. a call to golang.org/x/sync/errgroup.Group.Go at the end of the loop body
226-
3. a call testing.T.Run where the subtest body invokes t.Parallel()
227-
228-
In the case of (1) and (2), the analyzer only considers references in the last
229-
statement of the loop body as it is not deep enough to understand the effects
230-
of subsequent statements which might render the reference benign.
231-
232-
For example:
233-
234-
for i, v := range s {
235-
go func() {
236-
println(i, v) // not what you might expect
237-
}()
238-
}
221+
This analyzer reports places where a function literal references the
222+
iteration variable of an enclosing loop, and the loop calls the function
223+
in such a way (e.g. with go or defer) that it may outlive the loop
224+
iteration and possibly observe the wrong value of the variable.
225+
226+
In this example, all the deferred functions run after the loop has
227+
completed, so all observe the final value of v.
228+
229+
for _, v := range list {
230+
defer func() {
231+
use(v) // incorrect
232+
}()
233+
}
234+
235+
One fix is to create a new variable for each iteration of the loop:
236+
237+
for _, v := range list {
238+
v := v // new var per iteration
239+
defer func() {
240+
use(v) // ok
241+
}()
242+
}
243+
244+
The next example uses a go statement and has a similar problem.
245+
In addition, it has a data race because the loop updates v
246+
concurrent with the goroutines accessing it.
247+
248+
for _, v := range elem {
249+
go func() {
250+
use(v) // incorrect, and a data race
251+
}()
252+
}
253+
254+
A fix is the same as before. The checker also reports problems
255+
in goroutines started by golang.org/x/sync/errgroup.Group.
256+
A hard-to-spot variant of this form is common in parallel tests:
257+
258+
func Test(t *testing.T) {
259+
for _, test := range tests {
260+
t.Run(test.name, func(t *testing.T) {
261+
t.Parallel()
262+
use(test) // incorrect, and a data race
263+
})
264+
}
265+
}
266+
267+
The t.Parallel() call causes the rest of the function to execute
268+
concurrent with the loop.
269+
270+
The analyzer reports references only in the last statement,
271+
as it is not deep enough to understand the effects of subsequent
272+
statements that might render the reference benign.
273+
("Last statement" is defined recursively in compound
274+
statements such as if, switch, and select.)
239275

240276
See: https://golang.org/doc/go_faq.html#closures_and_goroutines
241277

gopls/internal/lsp/source/api_json.go

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)