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: go vet *.go / go tool vet *.go ignore Build Constraints when using sync.Map #22573

Closed
nbari opened this issue Nov 3, 2017 · 2 comments

Comments

Projects
None yet
3 participants
@nbari
Copy link

commented Nov 3, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nbari/projects/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/xm/hnv7zzyx3bn58115t5x4b__40000gn/T/go-build417603261=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Create testvet.go and testvet_linux.go on same directory and run go vet *.go or go tool vet *.go

File testvet.go:

// +build freebsd netbsd openbsd dragonfly darwin

package testvet

import "sync"

type ScanDir struct {
	dir      string
	services sync.Map
}

func main() {
	s := new(ScanDir)
	s.services.Delete("foo")
}

File testvet_linux.go:

// +build linux

package testvet

type ScanDir struct {
	dir      string
	services map[string]string
}

func main() {
	s := new(ScanDir)
	delete(s.services, "foo")
}

What did you expect to see?

nothing like in go versions <=1.9

What did you see instead?

With go vet *.go:

 testvet_linux.go:12: call of delete copies lock value: sync.Map contains sync.Mutex
 exit status 1

With go tool vet *.go:

 testvet_linux.go:12: call of delete copies lock value: sync.Map contains sync.Mutex

@ianlancetaylor ianlancetaylor changed the title go vet *.go / go tool vet *.go ignore Build Constraints when using sync.Map cmd/vet: go vet *.go / go tool vet *.go ignore Build Constraints when using sync.Map Nov 4, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 4, 2017

Nothing has changed with regard to how cmd/vet handles build constraints when invoked with a list of files. In all cases it checks all the files. You can see this easily enough by adding something that triggers a warning, such as fmt.Printf("%s"), to both files. If you run go vet *.go, you will see both warnings.

What has changed is how cmd/vet handles duplicate definitions. On tip, the future 1.10, cmd/vet actually emits errors:

./a_linux.go:7:6: ScanDir redeclared in this block
	previous declaration at ./a.go:8:6
./a_linux.go:12:6: main redeclared in this block
	previous declaration at ./a.go:13:6

In all cases, go vet (without the *.go) honors build constraints as expected. I recommend using that.

I'm inlined to simply close this issue with the recommendation to use plain go vet. Is there a reason I shouldn't do that? We aren't going to change cmd/vet to honor build constraints when files are explicitly listed on the command line; it has never done that and I don't see any reason to start.

@nbari

This comment has been minimized.

Copy link
Author

commented Nov 4, 2017

Hi @ianlancetaylor many thanks for the explanation, was confusing me since before 1.9 there was no error or either warning, but also before 1.9 there was not sync.Map therefore I though could be a bug.

@nbari nbari closed this Nov 4, 2017

@golang golang locked and limited conversation to collaborators Nov 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.