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

Jump to bottom

cmd/go: "+build comment … blank line" warning not produced by "go run" or "go build" #37803

Open
graywolf-at-work opened this issue Mar 11, 2020 · 7 comments
Open

cmd/go: "+build comment … blank line" warning not produced by "go run" or "go build" #37803

graywolf-at-work opened this issue Mar 11, 2020 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@graywolf-at-work
Copy link

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

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/wolf/.cache/go-build"
GOENV="/home/wolf/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY="go.showmax.cc"
GONOSUMDB="go.showmax.cc"
GOOS="linux"
GOPATH="/home/wolf/go"
GOPRIVATE="go.showmax.cc"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/ww/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build147562273=/tmp/go-build -gno-record-gcc-switches"

What did you do?

+$ tree .
.
├── a.go
├── b.go
├── c.go
└── go.mod

0 directories, 4 files
+$ cat a.go
package main

import "fmt"

func main() {
        fmt.Println(value)
}
+$ cat b.go
// +build !windows
package main

var value = "POSIX"
+$ cat c.go
// +build windows
package main

var value = "WIN"
+$ cat go.mod
module test

go 1.14
+$ go run .
# test
./c.go:4:5: value redeclared in this block
        previous declaration at ./b.go:4:5

What did you expect to see?

# test
./b.go:1:1: +build comment must appear before package clause and be followed by a blank line
./c.go:1:1: +build comment must appear before package clause and be followed by a blank line

What did you see instead?

# test
./c.go:4:5: value redeclared in this block
        previous declaration at ./b.go:4:5
@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2020

The +build comment warning comes from cmd/vet, not the compiler. (A malformed // +build comment decays to “just a comment”.)

cmd/vet is (intentionally) only run by go vet and go test, not go run.

@bcmills bcmills changed the title Misleading error message when blank line is missing after build tag line and variable is defined under in two separate files cmd/go: "+build comment … blank line" warning not produced by "go run" Mar 11, 2020
@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2020 •
edited

It's unfortunate that this doesn't get caught, but it seems like the alternative is to make go run even slower (by having it invoke cmd/vet), and folks already complain about go run being too slow as it is.

CC @jayconrod @matloob @ianthehat

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 11, 2020
@bcmills bcmills added this to the Unplanned milestone Mar 11, 2020
@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2020

On the other hand, go run implies that we're linking a final binary, and linking a binary is relatively slow anyway. Maybe also running vet (concurrently, even!) wouldn't be too bad.

@jayconrod
Copy link
Contributor

I'd prefer to keep vet separate, but it's pretty subjective. I think if we ran vet with go run, it would also make sense to run it with go build, and possibly also go list -export.

It's not clear there would be much performance cost. vet can run concurrently with compilation, and the results can be cached.

@ianlancetaylor
Copy link
Contributor

Personally I'd rather keep go vet attached to go test, and not start running it everywhere.

@bcmills bcmills changed the title cmd/go: "+build comment … blank line" warning not produced by "go run" cmd/go: "+build comment … blank line" warning not produced by "go run" or "go build" May 21, 2020
@bcmills bcmills mentioned this issue May 21, 2020
Closed
@DryHumour
Copy link

Perhaps go vet only if the compilation has failed, with the intent to provide the best diagnostic messages possible? You're on the slow path then anyway 😀

@bcmills
Copy link
Contributor

bcmills commented May 21, 2020

Ooh, that's a nice idea!

@bcmills bcmills mentioned this issue Dec 9, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@jayconrod @ianlancetaylor @bcmills @DryHumour @graywolf-at-work