Tool to fix calls to scoped_ptr<T>(NULL) to use the default ctor instead

BUG=173286
R=thakis@chromium.org

Review URL: https://codereview.chromium.org/16606002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205621 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 295 insertions, 3 deletions

diff --git a/tools/clang/rewrite_scoped_ptr_ctor_null/Makefile b/tools/clang/rewrite_scoped_ptr_ctor_null/Makefile
new file mode 100644
index 0000000..f807d89
--- /dev/null
+++ b/tools/clang/rewrite_scoped_ptr_ctor_null/Makefile
@@ -0,0 +1,23 @@
+# Copyright (c) 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+#
+# This Makefile requires the LLVM build system. In order to build this tool,
+# please run tools/clang/scripts/build_tool.py.
+
+CLANG_LEVEL := ../..
+
+TOOLNAME = rewrite_scoped_ptr_ctor_null
+
+NO_INSTALL = 1
+
+include $(CLANG_LEVEL)/../../Makefile.config
+
+LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc
+USEDLIBS = clangTooling.a clangFrontend.a clangSerialization.a clangDriver.a \
+           clangRewriteFrontend.a clangRewriteCore.a clangParse.a clangSema.a \
+           clangAnalysis.a clangAST.a clangASTMatchers.a clangEdit.a \
+           clangLex.a clangBasic.a
+
+include $(CLANG_LEVEL)/Makefile
+
diff --git a/tools/clang/rewrite_scoped_ptr_ctor_null/RewriteScopedPtrCtorNull.cpp b/tools/clang/rewrite_scoped_ptr_ctor_null/RewriteScopedPtrCtorNull.cpp
new file mode 100644
index 0000000..c1b25ed
--- /dev/null
+++ b/tools/clang/rewrite_scoped_ptr_ctor_null/RewriteScopedPtrCtorNull.cpp
@@ -0,0 +1,195 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// This implements a Clang tool to convert all instances of std::string("") to
+// std::string(). The latter is more efficient (as std::string doesn't have to
+// take a copy of an empty string) and generates fewer instructions as well. It
+// should be run using the tools/clang/scripts/run_tool.py helper.
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using clang::ast_matchers::MatchFinder;
+using clang::ast_matchers::argumentCountIs;
+using clang::ast_matchers::bindTemporaryExpr;
+using clang::ast_matchers::constructorDecl;
+using clang::ast_matchers::constructExpr;
+using clang::ast_matchers::expr;
+using clang::ast_matchers::forEach;
+using clang::ast_matchers::has;
+using clang::ast_matchers::hasArgument;
+using clang::ast_matchers::hasDeclaration;
+using clang::ast_matchers::matchesName;
+using clang::ast_matchers::id;
+using clang::ast_matchers::methodDecl;
+using clang::ast_matchers::newExpr;
+using clang::ast_matchers::ofClass;
+using clang::ast_matchers::varDecl;
+using clang::tooling::CommonOptionsParser;
+using clang::tooling::Replacement;
+using clang::tooling::Replacements;
+
+namespace {
+
+bool IsNullConstant(const clang::Expr& expr, clang::ASTContext* context) {
+  return expr.isNullPointerConstant(*context,
+                                    clang::Expr::NPC_ValueDependentIsNotNull) !=
+         clang::Expr::NPCK_NotNull;
+}
+
+// Handles replacements for stack and heap-allocated instances, e.g.:
+// scoped_ptr<T> a(NULL);
+// scoped_ptr<T>* b = new scoped_ptr<T>(NULL);
+// ...though the latter should be pretty rare.
+class ConstructorCallback : public MatchFinder::MatchCallback {
+ public:
+  ConstructorCallback(Replacements* replacements)
+      : replacements_(replacements) {}
+
+  virtual void run(const MatchFinder::MatchResult& result) LLVM_OVERRIDE;
+
+ private:
+  Replacements* const replacements_;
+};
+
+// Handles replacements for invocations of scoped_ptr<T>(NULL) in an initializer
+// list.
+class InitializerCallback : public MatchFinder::MatchCallback {
+ public:
+  InitializerCallback(Replacements* replacements)
+      : replacements_(replacements) {}
+
+  virtual void run(const MatchFinder::MatchResult& result) LLVM_OVERRIDE;
+
+ private:
+  Replacements* const replacements_;
+};
+
+// Handles replacements for invocations of scoped_ptr<T>(NULL) in a temporary
+// context, e.g. return scoped_ptr<T>(NULL).
+class TemporaryCallback : public MatchFinder::MatchCallback {
+ public:
+  TemporaryCallback(Replacements* replacements) : replacements_(replacements) {}
+
+  virtual void run(const MatchFinder::MatchResult& result) LLVM_OVERRIDE;
+
+ private:
+  Replacements* const replacements_;
+};
+
+class EmptyStringConverter {
+ public:
+  explicit EmptyStringConverter(Replacements* replacements)
+      : constructor_callback_(replacements),
+        initializer_callback_(replacements),
+        temporary_callback_(replacements) {}
+
+  void SetupMatchers(MatchFinder* match_finder);
+
+ private:
+  ConstructorCallback constructor_callback_;
+  InitializerCallback initializer_callback_;
+  TemporaryCallback temporary_callback_;
+};
+
+void EmptyStringConverter::SetupMatchers(MatchFinder* match_finder) {
+  const char kPattern[] = "^::(scoped_ptr|scoped_ptr_malloc)$";
+  const clang::ast_matchers::StatementMatcher& constructor_call = id(
+      "call",
+      constructExpr(hasDeclaration(methodDecl(ofClass(matchesName(kPattern)))),
+                    argumentCountIs(1),
+                    hasArgument(0, id("arg", expr()))));
+
+  match_finder->addMatcher(varDecl(forEach(constructor_call)),
+                           &constructor_callback_);
+  match_finder->addMatcher(newExpr(has(constructor_call)),
+                           &constructor_callback_);
+  match_finder->addMatcher(bindTemporaryExpr(has(constructor_call)),
+                           &temporary_callback_);
+  match_finder->addMatcher(constructorDecl(forEach(constructor_call)),
+                           &initializer_callback_);
+}
+
+void ConstructorCallback::run(const MatchFinder::MatchResult& result) {
+  const clang::Expr* arg = result.Nodes.getNodeAs<clang::Expr>("arg");
+  if (!IsNullConstant(*arg, result.Context))
+    return;
+
+  const clang::CXXConstructExpr* call =
+      result.Nodes.getNodeAs<clang::CXXConstructExpr>("call");
+  clang::CharSourceRange range =
+      clang::CharSourceRange::getTokenRange(call->getParenRange());
+  replacements_->insert(Replacement(*result.SourceManager, range, ""));
+}
+
+void InitializerCallback::run(const MatchFinder::MatchResult& result) {
+  const clang::Expr* arg = result.Nodes.getNodeAs<clang::Expr>("arg");
+  if (!IsNullConstant(*arg, result.Context))
+    return;
+
+  const clang::CXXConstructExpr* call =
+      result.Nodes.getNodeAs<clang::CXXConstructExpr>("call");
+  replacements_->insert(Replacement(*result.SourceManager, call, ""));
+}
+
+void TemporaryCallback::run(const MatchFinder::MatchResult& result) {
+  const clang::Expr* arg = result.Nodes.getNodeAs<clang::Expr>("arg");
+  if (!IsNullConstant(*arg, result.Context))
+    return;
+
+  // TODO(dcheng): File a bug with clang. There should be an easier way to do
+  // this replacement, but getTokenRange(call->getParenRange()) and the obvious
+  // (but incorrect) arg both don't work. The former is presumably just buggy,
+  // while the latter probably has to do with the fact that NULL is actually a
+  // macro which expands to a built-in.
+  clang::SourceRange range = arg->getSourceRange();
+  clang::SourceRange expansion_range(
+      result.SourceManager->getExpansionLoc(range.getBegin()),
+      result.SourceManager->getExpansionLoc(range.getEnd()));
+  replacements_->insert(
+      Replacement(*result.SourceManager,
+                  clang::CharSourceRange::getTokenRange(expansion_range),
+                  ""));
+}
+
+}  // namespace
+
+static llvm::cl::extrahelp common_help(CommonOptionsParser::HelpMessage);
+
+int main(int argc, const char* argv[]) {
+  CommonOptionsParser options(argc, argv);
+  clang::tooling::ClangTool tool(options.getCompilations(),
+                                 options.getSourcePathList());
+
+  Replacements replacements;
+  EmptyStringConverter converter(&replacements);
+  MatchFinder match_finder;
+  converter.SetupMatchers(&match_finder);
+
+  int result =
+      tool.run(clang::tooling::newFrontendActionFactory(&match_finder));
+  if (result != 0)
+    return result;
+
+  // Each replacement line should have the following format:
+  // r:<file path>:<offset>:<length>:<replacement text>
+  // Only the <replacement text> field can contain embedded ":" characters.
+  // TODO(dcheng): Use a more clever serialization.
+  llvm::outs() << "==== BEGIN EDITS ====\n";
+  for (Replacements::const_iterator it = replacements.begin();
+       it != replacements.end();
+       ++it) {
+    llvm::outs() << "r:" << it->getFilePath() << ":" << it->getOffset() << ":"
+                 << it->getLength() << ":" << it->getReplacementText() << "\n";
+  }
+  llvm::outs() << "==== END EDITS ====\n";
+
+  return 0;
+}
diff --git a/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc b/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc
new file mode 100644
index 0000000..16f3116
--- /dev/null
+++ b/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc
@@ -0,0 +1,33 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/scoped_ptr.h"
+
+void TestDeclarations() {
+  scoped_ptr<int> a, b(new int), c;
+  scoped_ptr_malloc<int> d;
+}
+
+void TestNew() {
+  scoped_ptr<int>* a = new scoped_ptr<int>, *b = new scoped_ptr<int>(new int),
+                   *c = new scoped_ptr<int>;
+}
+
+class TestInitializers {
+ public:
+  TestInitializers() {}
+  TestInitializers(bool) {}
+  TestInitializers(double) : b(new int), c() {}
+
+ private:
+  scoped_ptr<int> a;
+  scoped_ptr<int> b;
+  scoped_ptr<int> c;
+};
+
+scoped_ptr<int> TestTemporaries(scoped_ptr<int> a, scoped_ptr<int> b) {
+  scoped_ptr<int> c =
+      TestTemporaries(scoped_ptr<int>(), scoped_ptr<int>(new int));
+  return scoped_ptr<int>();
+}
diff --git a/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc b/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc
new file mode 100644
index 0000000..4fb3f2e
--- /dev/null
+++ b/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc
@@ -0,0 +1,35 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/scoped_ptr.h"
+
+void TestDeclarations() {
+  scoped_ptr<int> a(NULL), b(new int), c(NULL);
+  scoped_ptr_malloc<int> d(NULL);
+}
+
+void TestNew() {
+  scoped_ptr<int>* a = new scoped_ptr<int>(NULL),
+                   *b = new scoped_ptr<int>(new int),
+                   *c = new scoped_ptr<int>(NULL);
+}
+
+class TestInitializers {
+ public:
+  TestInitializers() : a(NULL) {}
+  TestInitializers(bool) : a(NULL), b(NULL) {}
+  TestInitializers(double) : a(NULL), b(new int), c() {}
+
+ private:
+  scoped_ptr<int> a;
+  scoped_ptr<int> b;
+  scoped_ptr<int> c;
+};
+
+scoped_ptr<int> TestTemporaries(scoped_ptr<int> a, scoped_ptr<int> b) {
+  scoped_ptr<int> c =
+      TestTemporaries(scoped_ptr<int>(NULL), scoped_ptr<int>(new int));
+  return scoped_ptr<int>(NULL);
+}
+
diff --git a/tools/clang/scripts/test_tool.py b/tools/clang/scripts/test_tool.py
index d2b87a0e..a99e9f4 100755
--- a/tools/clang/scripts/test_tool.py
+++ b/tools/clang/scripts/test_tool.py
@@ -15,10 +15,13 @@ import shutil
 import sys
 
 
-def _GenerateCompileCommands(files):
+def _GenerateCompileCommands(files, include_paths):
   """Returns a JSON string containing a compilation database for the input."""
+  include_path_flags = ''.join('-I %s' % include_path
+                               for include_path in include_paths)
   return json.dumps([{'directory': '.',
-                      'command': 'clang++ -fsyntax-only -c %s' % f,
+                      'command': 'clang++ -fsyntax-only %s -c %s' % (
+                          include_path_flags, f),
                       'file': f} for f in files], indent=2)
 
 
@@ -46,6 +49,9 @@ def main(argv):
                   for source_file in source_files]
   expected_files = ['-'.join([source_file.rsplit('-', 2)[0], 'expected.cc'])
                     for source_file in source_files]
+  include_paths = []
+  include_paths.append(
+      os.path.realpath(os.path.join(tools_clang_directory, '../..')))
 
   try:
     # Set up the test environment.
@@ -58,7 +64,7 @@ def main(argv):
     subprocess.check_call(args)
     # Generate a temporary compilation database to run the tool over.
     with open(compile_database, 'w') as f:
-      f.write(_GenerateCompileCommands(actual_files))
+      f.write(_GenerateCompileCommands(actual_files, include_paths))
 
     args = ['python',
             os.path.join(tools_clang_scripts_directory, 'run_tool.py'),