aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:24
+
+ const char *MatchText = "::std::uncaught_exception";
+
----------------
You might as well make this a `std::string` rather than `const char *` because
the `hasName()` matcher accepts that datatype (removes a few implicit
converting constructor calls).
================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:61
+
+ // we don't want to modify template definitions
+ Text.consume_front("std::");
----------------
Comments are full sentences (with correct capitalization and punctuation).
================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+ }
+ Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
+ }
----------------
Same concerns here as with the previous review: 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. Alternatively, you can identify the
narrowing conversion case and skip the fix-it entirely in that case (only warn).
This example should be a test case.
================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.h:19
+
+/// This check will warn for the occurrences of std::uncaught_exception and
replace it with
+/// std::uncaught_exceptions. Since C++17 std::uncaught_exception is
deprecated. In case of
----------------
warn for the occurrences of -> warn on calls to
replace it -> replace them with calls to
================
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.h:20
+/// This check will warn for the occurrences of std::uncaught_exception and
replace it with
+/// std::uncaught_exceptions. Since C++17 std::uncaught_exception is
deprecated. In case of
+/// macro ID there will be only a warning without fixits.
----------------
Since C++17... -> std::uncaught_exception was deprecated in C++17.
================
Comment at: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst:6-8
+This check will warn for the occurrences of ``std::uncaught_exception`` and
+replace it with ``std::uncaught_exceptions``. Since C++17
+``std::uncaught_exception`` is deprecated.
----------------
Same wording suggestions here as above.
https://reviews.llvm.org/D40787
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
