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: cannot run 'go vet' on testdata files #25752

Closed
dlsniper opened this issue Jun 6, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@dlsniper
Copy link
Contributor

commented Jun 6, 2018

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

go version go1.10.2 windows/amd64

Does this issue reproduce with the latest release?

yes

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

set GOHOSTARCH=amd64
set GOHOSTOS=windows

What did you do?

I ran go vet on the file here: https://raw.githubusercontent.com/golang/go/27fb26c77c0374ec1876223593078e4d6b98d4f0/src/cmd/vet/testdata/cgo/cgo.go

What did you expect to see?

Vet errors as described by the comments // ERROR "embedded pointer"

What did you see instead?

C:\Go1.10\bin\go.exe vet D:\go\src\github.com\dlsniper\u\tmp\vet\vet.go
# command-line-arguments
C:\Users\florin\AppData\Local\Temp\go-build553891458\b001\_x002.o: In function `_cgo_3d8b191c8128_Cfunc_f':
/tmp/go-build/cgo-gcc-prolog:37: undefined reference to `f'
collect2.exe: error: ld returned 1 exit status

I've tried this on Linux as well as Windows. Same compile error and vet never has a chance to actually work. Do I miss anything here?
Thank you.

@dlsniper

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

@mvdan

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

I agree that this is inconsistent and a bit confusing. At the moment, cmd/vet sometimes requires types, and sometimes doesn't. If you look at its main.go, you'll see:

// Special case for "go vet" passing an explicit configuration:
// single argument ending in vet.cfg.
// Once we have a more general mechanism for obtaining this
// information from build tools like the go command,
// vet should be changed to use it. This vet.cfg hack is an
// experiment to learn about what form that information should take.
if flag.NArg() == 1 && strings.HasSuffix(flag.Arg(0), "vet.cfg") {
        doPackageCfg(flag.Arg(0))
        os.Exit(exitCode)
}

In particular, doPackageCfg sets mustTypecheck = true, which is otherwise false.

This is how go vet gets run - cmd/go runs first, it constructs a cfg file with the package loading information, and then calls cmd/vet with that. Then, if the file has typecheck errors, the tool bails. That is what you are seeing.

However, the tests run cmd/vet directly on lists of files or directories. That is the equivalent of running go tool vet instead of go vet. You can see the result:

$ go vet testdata/cgo/cgo.go
# command-line-arguments
cgo-gcc-prolog:37: error: undefined reference to 'f'
collect2: error: ld returned 1 exit status
$ go tool vet testdata/cgo/cgo.go
testdata/cgo/cgo.go:16: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:17: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:17: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:20: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:21: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:21: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:24: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:25: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:25: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:28: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:29: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:29: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:32: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:33: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:33: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:36: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:37: possibly passing Go type with embedded pointer to C
testdata/cgo/cgo.go:37: possibly passing Go type with embedded pointer to C
$ go version
go version devel +2ce295e954 Wed Jun 6 01:47:31 2018 +0000 linux/amd64

In the long run, once vet always requires type information, I would hope that its testdata will be reworked to contain files and directories where one can run go vet directly on. If any files were designed to make vet error in some way, I'd put those in a separate place like testdata/bad/.

/cc @alandonovan @josharian in case I missed anything or said anything wrong.

@mvdan

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

If anyone wants to read the history on how we ended up with this complex situation: #21287 #16086

@mvdan mvdan changed the title cmd/vet: cgo test broken? cmd/vet: cannot run 'go vet' on testdata files Jun 6, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2018

@mvdan is correct, and this is not a bug. The cmd/vet tests test the vet tool in standalone mode. There is no reason to make them work in the mode in which vet is invoked by the go tool. If someone wants to change the vet tests to work with go vet, that's fine as long as existing functionality is preserved, but since this is not a bug I'm going to close this issue.

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