[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32
+  Finder->addMatcher(
+      declRefExpr(to(functionDecl(hasName(MatchText)))).bind("call_expr"),
+      this);
----------------
koldaniel wrote:
> JonasToth wrote:
> > 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 :)
> According to the original concept callExpr was used, but to match template instantiations and other usages than calling the function directly, it seemed to me a better and simpler solution to use declRefExpr. In case of function pointer initializations like the this:
> 
>     bool (*foo)();
>     foo = &std::uncaught_exception;
>     res = foo();
> 
> there will be a warning and the fix will be applied too.
Could you add testcases for function pointers please?

Having tests for the template instantiations (in which form? using `std::uncaught_exceptions` as template argument for callables?) would be a good thing as well.


================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+  if (!BeginLoc.isMacroID()) {
+    Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+                                       "s");
----------------
koldaniel wrote:
> JonasToth wrote:
> > Could the location not simply be `EndLoc`?
> As I could see, when EndLoc was used in Diag (diag(..) of CreateInsertion(...) methods,  it still pointed to the beginning of the token marking the whole call. So if the CreateInsertion function looked like this: 
> 
>   Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength), "s");
> 
> had the same effect after applying the fix its as
> 
>     Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> 
> for calls like 'uncaught_exception()' (without std:: namespace written at the beginning, because it increases TextLength, and in case of EndLoc the offset will be counted from the beginning of the function name, not the namespace identifier).
Thats odd. You could do a Replacement with `getSourceRange` probably. Sounds more inefficient, but might be shorter in Code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787