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 emits "unreachable code" for jumped over consts #27760
Comments
It seems like In any case, I don't think we should fix this. The code is bug-prone and should be corrected. /cc @mvdan if there is anything else to do here. |
Vet should never have false positives, but I too don't think this is one. Always jumping over a number of statements doesn't seem to ever make sense, even if that can still be used to declare constants. In other words - has anyone ever encountered this in real code? If not, I think we should just close this issue until someone does. |
This is indeed code go vet should complain about, but I think go vet should, in this case, not complain about "unreachable code" but something more specific in the vein of "goto used to jump over const definitions". schmichael, would be more in line with what you expected to happen? |
Like @mvdan says, I don't think we should try to optimize for cases that does not occur in real code. If it was indeed found in real code, I would like to understand why. |
I kind of did? I wanted to simulate a pause here: https://github.com/hashicorp/nomad/blob/a333efb8369540081e1f79514e44bb93e259d2e3/client/allocrunnerv2/taskrunner/task_runner.go#L271 So I added the following code: dur := 30 * time.Second
tr.logger.Debug("sleeping", "duration", dur)
time.Sleep(dur) Which gave me the expected error:
Since this was temporary testing code I figured a const dur = 30 * time.Second
tr.logger.Debug("sleeping", "duration", dur)
time.Sleep(dur) Of course I wanted to disgust and horrify my coworkers with this clever hack, so I created the simplified playground posted in the ticket and saw the vet error.
SGTM. That being said it's probably not worth worrying about as @agnivade pointed out. I'd be fine with closing this until someone comes up with a more compelling reason to fix it. |
But in your task_runner.go code, did you get vet warnings for unreachable code ? As I see, the |
@agnivade You're right! No vet warnings in my real example! Sorry for the noise! |
No worries. Closing as discussed above. |
What version of Go are you using (
go version
)?go version go1.11 linux/amd64
(and playground which as of 2018-09-19 is on 1.10.3)
What did you do?
https://play.golang.org/p/O7If8gy-bUc
What did you expect to see?
I don't even know what I expected. This might not even be a bug. Try stupid things, get stupid results?
What did you see instead?
In the playground:
Locally:
The text was updated successfully, but these errors were encountered: