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: incorrectly warning for missing ellipsis in printf like function #26979

Closed
romaindoumenc opened this issue Aug 14, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@romaindoumenc
Copy link

commented Aug 14, 2018

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

Go 1.11rc1

Does this issue reproduce with the latest release?

No — only with the next (1.11) release candidate

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

GOARCH="amd64"
GOBIN=""
GOCACHE="$HOME/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="$HOME/dev"
GORACE=""
GOROOT="$HOME/sdk/go"
GOTMPDIR=""
GOTOOLDIR="$HOME/sdk/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build637964582=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Passing multiple arguments to a printf-like function, where the last argument is an array.

https://play.golang.org/p/k7ZwTL59hIu

What did you expect to see?

Go vet accepts this program (as it did in Go1.10)

What did you see instead?

An error message is displayed:

./fwd.go:10: missing ... in args forwarded to printf-like function

@romaindoumenc

This comment has been minimized.

Copy link
Author

commented Aug 14, 2018

Investigating a bit deeper, and updating the report accordingly (with the understanding that I am quite new to vet code base, so might well be missing something).

The issue seems to reside in src/cmd/vet/print.go line 127, where any second-to-last argument of type string is considered to be a format directive to the printf call — even if, in this report case, the string is actually an argument to another formatting directive.
The code is part of commit c0060360751.

Maybe we could then check in checkPrintfFwd that what we detected as a formatting string is indeed passed as such to the underling call to printf?

@romaindoumenc romaindoumenc changed the title Go vet (1.11) rejects array passed as last argument to printf like function Go vet (1.11) incorrectly warns for missing ellipsis in printf like function Aug 14, 2018

@agnivade agnivade changed the title Go vet (1.11) incorrectly warns for missing ellipsis in printf like function cmd/vet: incorrectly warning for missing ellipsis in printf like function Aug 14, 2018

@agnivade agnivade added this to the Go1.11 milestone Aug 14, 2018

@agnivade

This comment has been minimized.

Copy link
Member

commented Aug 14, 2018

/cc @mvdan

@mvdan

This comment has been minimized.

Copy link
Member

commented Aug 14, 2018

Thanks for reporting! This does seem like a regression at first glance, as the program does something correct. Your proposed solution also sounds fine, although I've never touched this part of vet. I might have a deeper look at this later today.

@romaindoumenc

This comment has been minimized.

Copy link
Author

commented Aug 16, 2018

Just a quick note about the title: while go vet itself might only warns of this issue, it now makes the test suite fail (running by default in Go 1.11). So this is actually very much an error.

This report is coming from our CI (beta) job failing the build, so I guess I am trying to say I am not just nitpicking :)

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

@cznic

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

but this really is a potential data race, so investing the time in fixing this bug will make your code more reliable.

I see no bug in the linked user code, neither do I see any data race. Please share where's the bug and where's the data race. Thanks.

@romaindoumenc

This comment has been minimized.

Copy link
Author

commented Aug 16, 2018

@davecheney thank you for your answer! I am at loss to see how this could be related in any way to a data race: the code is a simple, single-threaded nested function calls…

Furthermore, I pointed in my early investigations that the root cause is likely on the way go vet identifies a formatting argument to a printf function (every second-to-last argument which is a string), and therefore confuses a string argument with a formatting directive.

Happy to be shown otherwise of course! :)

@mvdan

This comment has been minimized.

Copy link
Member

commented Aug 16, 2018

I think Dave's comment was meant for #27001 :)

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Aug 16, 2018

Change https://golang.org/cl/129575 mentions this issue: cmd/vet: don't suggest ... if it breaks a program

@mvdan

This comment has been minimized.

Copy link
Member

commented Aug 16, 2018

I've just sent what I think is a simpler fix than what was suggested above. It simply avoids the ellipsis suggestion if it would break the program. Hopefully it's small enough to go in for 1.11 or 1.11.1.

@gopherbot gopherbot closed this in 2482451 Aug 17, 2018

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.