[PATCH] D12081: Add use-nullptr check to clang-tidy.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:16
@@ +15,3 @@
+
+using namespace clang::ast_matchers;
+using namespace clang;
----------------
nit: The list should be sorted.

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:72
@@ +71,3 @@
+/// Returns true if and only if a replacement was made.
+void ReplaceWithNullptr(ClangTidyCheck &check, SourceManager &SM,
+                        SourceLocation StartLoc, SourceLocation EndLoc) {
----------------
Function names should start with a lower-case character. Also, s/check/Check/.

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:95
@@ +94,3 @@
+/// If \p Loc points to NULL, this function will return the name MY_NULL.
+llvm::StringRef GetOutermostMacroName(SourceLocation Loc,
+                                      const SourceManager &SM,
----------------
s/llvm:://

Function names should start with a lower-case character.

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:106
@@ +105,3 @@
+
+  return clang::Lexer::getImmediateMacroName(OutermostMacroLoc, SM, LO);
+}
----------------
s/clang:://

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:205
@@ +204,3 @@
+      return true;
+    } else if (!FirstSubExpr) {
+      FirstSubExpr = C->getSubExpr()->IgnoreParens();
----------------
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:232
@@ +231,3 @@
+          SM.isMacroBodyExpansion(EndLoc)) {
+        llvm::StringRef OutermostMacroName =
+            GetOutermostMacroName(StartLoc, SM, Context.getLangOpts());
----------------
s/llvm:://

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:340
@@ +339,3 @@
+      FileID MacroFID = SM.getFileID(MacroLoc);
+      if (SM.isInFileID(ArgLoc, MacroFID))
+        // Don't transform this case. If the characters that caused the
----------------
I'd use braces around the `if` body here.

================
Comment at: clang-tidy/modernize/UseNullptrCheck.cpp:367
@@ +366,3 @@
+
+    while (1) {
+      std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
----------------
`while (true)` or `for (;;)`

================
Comment at: clang-tidy/modernize/UseNullptrCheck.h:27
@@ +26,3 @@
+private:
+  llvm::SmallVector<llvm::StringRef, 1> UserNullMacros;
+};
----------------
nit: I suspect that `llvm::` is not needed on both types here, as they should be imported to the `clang` namespace as well. Please try without it.

================
Comment at: test/clang-tidy/modernize-use-nullptr.cpp:2
@@ +1,3 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s modernize-use-nullptr %t \
+// -config="{CheckOptions: [{key: modernize-use-nullptr.UserNullMacros, value: 'MY_NULL'}]}" \
+// -- -std=c++11
----------------
This line must start with RUN: as well, and the next one. Also, please add two more spaces after RUN:, so that it's more obvious that the command line is wrapped.


http://reviews.llvm.org/D12081