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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+  if (!BeginLoc.isMacroID()) {
+    Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+                                       "s");
----------------
aaron.ballman wrote:
> JonasToth wrote:
> > 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.
> This fixit can break code if the code disallows narrowing conversions. e.g.,
> ```
> bool b{std::uncaught_exception()};
> ```
> In this case, the fixit will take the deprecated code and make it ill-formed instead. Perhaps a better fix-it would be to transform the code into `(std::uncaught_exceptions() > 0)` to keep the resulting expression type a `bool` and still not impact operator precedence?
Good point, but this kind of fix it is a bit ugly. Maybe skipping the fixit in narrowing cases or only generate the more complicated replacement in the narrowing case would be nicer. 


================
Comment at: test/clang-tidy/modernize-replace-uncaught-exception.cpp:16
+template <typename T>
+int doSomething(T t) { 
+    return t();
----------------
It would be great to have a test where the template parameter is a function pointer and it is called with `uncaught_exception`. And with a check fixes make sure that the definition of the template is untouched.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787