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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+  if (!BeginLoc.isMacroID()) {
+    Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+                                       "s");
----------------
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?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787