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