-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: cmd/vet: warn about recover inside non-deferred function #64116
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
Comments
This may be hard in general. It is true that |
@cherrymui's point is well taken. Solving this is roughly the same as a pointer/may-escape analysis in the limit. This example will be much harder:
The reason is that a warning should probably only be given if a recover() cannot be directly called by some defer statement. So off the bat, exported functions cannot be warned on in vet. Some cases should not be too hard though, like #64116 (comment) , or unexported functions that are never stored. FWIW something useful would be simpler in x/tools/go/ssa. I do not have a good sense for how frequent these issues are. Getting a good grasp might be hard without an Analyzer implementation to test. |
I also don't have any intuition for whether this would meet the "frequency" requirement for cmd/vet. @justpretending have you seen this frequently in practice? |
In the absence of data supporting that this is a significant problem I think we should decline this. |
In the absence of evidence that this is a common mistake it seems like this is headed for likely decline. |
In general literally the only time we could issue a definitive positive would be 'go func() { ... }()'. There's no evidence people accidentally write defer instead of go, and even if they did, we'd only catch it when they happened to call recover. |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
Here, the
go
keyword is mistakenly used instead of thedefer
keyword. It would be lovely ifvet
warned about this case.The text was updated successfully, but these errors were encountered: