aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
I *really* like this as a modernize check, especially since (throwing) dynamic
exception specifications may be removed in C++1z. However, I think this check
doesn't do enough yet. If dynamic exception specifications are removed in the
next standard, `throw()` is likely to remain for backwards compatibility since
it does map directly to `noexcept`. I think this check really needs to convert
`throw(type)` and `throw(...)` into `noexcept(false)` as well. Further, I
prefer not to touch `noexcept(true)` as part of this check, since that really
isn't modernizing anything.
Thank you for working on this!
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:24
@@ +23,3 @@
+ char delimiter) {
+ SmallVector<StringRef, 5> Candidates;
+ AllStrings.split(Candidates, ',');
----------------
Why 5?
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:33
@@ +32,3 @@
+
+using namespace lexer_utils;
+
----------------
This should not be at file scope; if it really clarifies the code, it should be
at function scope where needed.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:65
@@ +64,3 @@
+
+ if (const FunctionDecl *FuncDecl =
+ Result.Nodes.getNodeAs<clang::FunctionDecl>("functionDecl")) {
----------------
Instead of indenting a considerable amount of code, I think an early return may
be a bit more clear.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:83
@@ +82,3 @@
+ BeforeThanCompare<SourceLocation> isBefore(SM);
+ while (isBefore(BeginLoc, CurrentLoc)) {
+ SourceLocation Loc = Tok.getLocation();
----------------
This while loop could use some comments to explain what it is trying to do. As
best I can tell, this appears to be looking purely at the text the user wrote
to try to determine whether there is a `throw()` or a `noexcept(true)`, but
that can be done more clearly with FunctionType::getExceptionSpecType().
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.h:21
@@ +20,3 @@
+
+/// \brief Replace noexcept specifications, or macros, with noexcept or a user
defined macro.
+/// \code
----------------
I think you mean "Replace dynamic exception specifications" instead of
"noexcept specifications".
http://reviews.llvm.org/D18575
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
