[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");
----------------
xazax.hun wrote:
> 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. 
I would be fine with only using the uglier version if there's a narrowing conversion, or removing the fixit entirely in the narrowing case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787