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