JonasToth added inline comments.
================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:20
+
+static const char *MatchText = "::std::uncaught_exception";
+
----------------
Could that be a local variable in `registerMatchers`? Even though its static
and const it might be best to reduce its scope as much as possible.
================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32
+ Finder->addMatcher(
+ declRefExpr(to(functionDecl(hasName(MatchText)))).bind("call_expr"),
+ this);
----------------
Interesting. Did you consider `callExpr` as well? I never used `declRefExpr` in
this context but it might be a good choice too. Would it catch taking a
function pointer to `std::uncaught_exception` too? If so, i need to overthink
some of my code :)
================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:45
+ } else {
+ const auto *U = Result.Nodes.getNodeAs<UsingDecl>("using_decl");
+ BeginLoc = U->getNameInfo().getBeginLoc();
----------------
The implicit assumption here is that one of both must have been matched which
is true now.
But adding an `assert` on `U` might still be a good thing. You never known how
the code evolves and what bugs might come alive.
That just ensures that there is no possible way to dereference a `nullptr`.
================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+ Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
----------------
Could the location not simply be `EndLoc`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
