alexfh requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:18
@@ +17,3 @@
+
+// FIXME: Move to ASTMatchers.h on acceptance.
+namespace ast_matchers {
----------------
Please send a separate patch adding this matcher to ASTMatchers.h (with tests
and docs).
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:74
@@ +73,3 @@
+ SourceLocation ReplaceEnd;
+ std::string Replacement{ReplacementStr};
+ unsigned TokenLength{0};
----------------
http://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor
Use the least powerful tool for the job: `std::string Replacement =
ReplacementStr;`, this syntax makes it obvious that `ReplacementStr` is a
`std::string` (or is implicitly converted to `std::string`).
Same elsewhere.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:80
@@ +79,3 @@
+ auto TokensEnd = Tokens.rend();
+ for (auto it = Tokens.rbegin(); it != TokensEnd; ++it) {
+ SourceLocation Loc = it->getLocation();
----------------
1. Variable names should be CamelCase.
2. It's more common to use `I` for array indices or iterators in LLVM.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:109
@@ +108,3 @@
+ }
+ } else if (++it == TokensEnd)
+ return;
----------------
Braces should be used consistently on both sides of `else if`.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:115
@@ +114,3 @@
+ if (ReplaceStart.isValid() && ReplaceEnd.isValid()) {
+ std::pair<FileID, unsigned> beginInfo = SM.getDecomposedLoc(ReplaceStart);
+ std::pair<FileID, unsigned> endInfo = SM.getDecomposedLoc(ReplaceEnd);
----------------
CamelCase
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:117
@@ +116,3 @@
+ std::pair<FileID, unsigned> endInfo = SM.getDecomposedLoc(ReplaceEnd);
+ if (beginInfo.first != endInfo.first || beginInfo.second > endInfo.second)
+ return;
----------------
Looks like `Lexer::makeFileCharRange` will do this job better.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:122
@@ +121,3 @@
+ "specification '%1'; use '%2' instead")
+ << FuncDecl->getNameInfo().getAsString()
+ << StringRef(SM.getCharacterData(ReplaceStart), Len) << Replacement
----------------
Just `FuncDecl` should be enough.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.h:22
@@ +21,3 @@
+/// \brief Replace dynamic exception specifications, with
+/// noexcept (or user-defined macro) or noexcept(true).
+/// \code
----------------
Please enclose inline code snippets in backquotes.
================
Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``throw(false)``. Additinally, users can also use
+:option:``ReplacementString`` to specify a macro to use instead of
+``noexcept``. This is useful when maintaining source code that must
----------------
There's an alternative approach: the appropriate compatibility macro can be
detected automatically using the `Preprocessor::getLastMacroWithSpelling`
method. This approach will only work, if the header defining the macro is
already included. However, if it's not included, the check must also add the
corresponding #include statement to avoid breaking the code.
Also, if you detect the macro name automatically, you can provide a default
macro by the means of the command line options
(`-extra-arg=-DNOEXCEPT=noexcept(false)` or the same via the configuration
file).
http://reviews.llvm.org/D18575
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
