Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/vet: rangeloop: inspect all go/defer statements prior to a loop back edge #30649

Open
luhexcp opened this issue Mar 7, 2019 · 4 comments

Comments

@luhexcp
Copy link

commented Mar 7, 2019

Running go vet on this file:

package main

import "fmt"

func main() {
	done := make(chan bool)

	values := []string{"a", "b", "c"}
	for _, v := range values {
		go func() {
			fmt.Println(v)
			done <- true
		}()
	}

	for _ = range values {
		<-done
	}
}

prints

golang_goroutines_and_closures.go:11: loop variable v captured by func literal

If I change loop body to this:

ok := true
for _, v := range values {
	if ok {
		go func() {
			fmt.Println(v)
			done <- true
		}()
	}
}

then go vet prints nothing.

Looking at the vet source code, checkRangeLoop can just judge a go or defer statement for body to see whether a variable is capture, which means vet will be failed if it exists condition sentence before a go or defer statement

@luhexcp luhexcp changed the title cmd/vet: -rangeloops check empty if loop body exists condition sentence before go & defer statement cmd/vet: -rangeloops check empty if loop body exists condition sentence nestification before go & defer statement Mar 7, 2019

@agnivade

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

Making vet more precise for loop-variable capture is #16520. Marking as duplicate.

@bcmills bcmills closed this Mar 7, 2019

@luhexcp

This comment has been minimized.

Copy link
Author

commented Mar 17, 2019

Making vet more precise for loop-variable capture is #16520. Marking as duplicate.

can the question be sovled?

@alandonovan

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2019

This is an interesting special case that we could easily extend the check to handle without the full generality of #16520, which is much harder to solve. Specifically: the go/defer statement is still the last statement within the loop body, if we use a slightly more generalized definition of "last" that traverses down into control constructs (such as if-statements in this example).

@alandonovan alandonovan reopened this Mar 17, 2019

@alandonovan alandonovan changed the title cmd/vet: -rangeloops check empty if loop body exists condition sentence nestification before go & defer statement cmd/vet: rangeloop: inspect all go/defer statements prior to a loop back edge Mar 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.