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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
