[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked 3 inline comments as done.
PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp:71
+
+bool EmptyCatchCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
----------------
xgupta wrote:
> This can be defined in the header file itself like other checks.
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.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp:76
+
+std::optional<TraversalKind> EmptyCatchCheck::getCheckTraversalKind() const {
+  return TK_IgnoreUnlessSpelledInSource;
----------------
xgupta wrote:
> This can be defined in the header file itself like other checks.
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.
And I want to keep it close to registerMatchers, as it impact behavior of that method.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp:3
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:        {key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}"
+
----------------
xgupta wrote:
> If TODO is in default then it does not require to be in value here, right?
> I tested without that, it gives the warning. 
Setting `IgnoreCatchWithKeywords` manually override default value.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144748/new/

https://reviews.llvm.org/D144748