Detects and suggests addressing issues with empty catch statements.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst | ||
---|---|---|
170 | Yy, what mistake ? (empty) not needed, or something else should be used instead of `` ? |
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst | ||
---|---|---|
170 | Sorry, I mistook (empty) as part of value. May be just say this in words to avoid such confusions? |
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp | ||
---|---|---|
101 | Assert that it's not null, or exit early. | |
105 | Nit: I believe a semicolon or dot is more appropriate here | |
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst | ||
148–154 | I'm not sure this option is worth the added complexity - if people want to allow an empty catch with a comment, they might as well just add a NOLINT to suppress the warning? It will be even more clear that it's actually a problem: it's not a regular TODO that doesn't necessarily imply a problem, but rather something that a static analyzer considers a problem. | |
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp | ||
12 | Fix indentation to 2 chars. | |
17 | Align comments |
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst | ||
---|---|---|
104 | Use 2 spaces indentation. |
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp | ||
---|---|---|
101 | It* |
clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst | ||
---|---|---|
148–154 | I added this so people could have ability to ignore some issues without putting NOLINT's. This can be also used when developer put some macro into catch block, that would be build dependent, like assert or something. |
Other than these three points everything looks good to me.
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp | ||
---|---|---|
72 | This can be defined in the header file itself like other checks. | |
77 | This can be defined in the header file itself like other checks. | |
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp | ||
4 | If TODO is in default then it does not require to be in value here, right? |
clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp | ||
---|---|---|
72 | Can be, but as this is already virtual function we do not gain anything by putting it into header. vtable will be emited anyway only on this TU. | |
77 | Can be, but as this is already virtual function we do not gain anything by putting it into header. vtable will be emited anyway only on this TU. | |
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp | ||
4 | Setting IgnoreCatchWithKeywords manually override default value. |
LGTM, but we can wait for a couple of before committing in case other reviewers have more review comments.
This can be defined in the header file itself like other checks.