Passionate test-driven developer
I have commit access to clang-tidy.
Passionate test-driven developer
I have commit access to clang-tidy.
In D128697#3638467, @Sockke wrote:In D128697#3619310, @LegalizeAdulthood wrote:This whole check seems weird to me. I mean, almost every use of a standard container could throw std::bad_alloc but we don't insist on a local catch for bad_alloc at every possible throwing call site.
Why would we assume that every call site of stoi or stod must have an exception handler immediately around it? It's perfectly acceptable for an application to handle this at an outer scope that can't be detected by clang-tidy.
Makes sense, I implemented this check here because some projects in ByteDance used stoi with missing exception handlers caused an online crash, I think this is a relatively common problem. Perhaps only report diagnostics for stoi calls in nothrow functions?
In D133102#3774151, @alexfh wrote:For example, LLVM coding standards recommends using early exits: https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
I'm not certain that the result of the transformation is more "readable"; is this check intended to aid conformance to a style guide?
How does this check interact with macros that expand to if statements?
No action taken, removing myself as a reviewer
LGTM
I will submit it
In D128157#3612741, @myhsu wrote:You forgot to add "Differential Revison: https://reviews.llvm.org/DXXXXXX" in the commit message, which is how Phabricator identifies the differential to close. I don't think it has anything to do with rebasing.
Thanks for this, like Nathan James this has been pestering me for a while :)
Please provide the name and email address you wish to use on the commit and I will submit.
I can submit after you address additional comments by Nathan James.
Clang-tidy tests and docs have been moved to subdirectories by module, please rebase to main:HEAD
Please add a summary of the fix to the release notes for this check.
This whole check seems weird to me. I mean, almost every use of a standard container could throw std::bad_alloc but we don't insist on a local catch for bad_alloc at every possible throwing call site.
I can submit after you address additional comments by Nathan James.
I pushed this change, but for some reason phabricator doesn't show it and close the review... I wonder if it's because I rebased it?
LGTM
Well, it must have been unrelated to your change or some weird interaction on my machine because I rebased and ran check-clang-extra again and it passed this time.
Rebase against main to get updated docs
I get a test failure when I run with your change:
In D128157#3605829, @jspam wrote:I see :) Yes, looks like Git's rename detection did its job.
Thanks for the review. Could you or someone else please merge this for me? Author: Joachim Priesner <llvm-project-704996@jspam.de>
OK, I've pushed the link fix and the sorting fix to main so if you rebase again, it should just show your changes to the release notes.
LGTM