[cfe-commits] [PATCH] C++11 nullptr-convert standalone tool

[Dmitri Gribenko]

  I understand that this approach makes edits that are certainly OK from the programmer's point of view.  But there is another approach: just walk the AST and find all the null pointers.  Did you consider this one?


================
Comment at: nullptr-convert/NullptrActions.h:13
@@ +12,3 @@
+//===----------------------------------------------------------------------===//
+#ifndef __NULLPTR_ACTIONS_H__
+#define __NULLPTR_ACTIONS_H__
----------------
 __NULLPTR_ACTIONS_H__ is an identifier reserved for implementation.

(Here and in other headers.)


================
Comment at: nullptr-convert/Matchers.cpp:10
@@ +9,3 @@
+//
+//  This file defines the matchers for use in migrating nullptr.
+//
----------------
Please turn this into a Doxygen file-level comment.  (Here and in other headers.)

================
Comment at: nullptr-convert/Matchers.cpp:36
@@ +35,3 @@
+  const Expr *Initializer = Node.getAnyInitializer();
+  return (Initializer != NULL && Initializer->isNullPointerConstant(
+        Node.getASTContext(), Expr::NPC_ValueDependentIsNull)
----------------
Line break after && would be more intuitive.

================
Comment at: nullptr-convert/NullptrActions.cpp:62
@@ +61,3 @@
+  // expansion location for macros and the same location otherwise.
+  auto getSourceLoc = [&Context](SourceLocation Loc) -> SourceLocation {
+    if (Loc.isMacroID())
----------------
I don't think we can use C++11 features.  This can be a member function unless I'm missing something.

================
Comment at: nullptr-convert/NullptrConvert.cpp:80
@@ +79,3 @@
+  RefactoringTool Tool(*Compilations, SourcePaths);
+  unsigned AcceptedChanges = 0;
+  clang::ast_matchers::MatchFinder Finder;
----------------
Maybe NumAcceptedChanges?  (Here and in other places.)

Maybe also make it a member of NullptrFixer?

================
Comment at: nullptr-convert/NullptrActions.cpp:86
@@ +85,3 @@
+
+    const Expr* NullExpr = V.getNullPtrConstExpr();
+    // Check if there was a null pointer constant expression in the matched
----------------
Space should be before '*'.


http://llvm-reviews.chandlerc.com/D136