Add remove_unneeded_adapt_callback tool to base_bind_rewriters

This CL adds a rewriter to //tools/clang/base_bind_rewriters
which removes calls to base::AdaptCallbackForRepeating when the
returned base::RepeatingCallback is immediately converted
to a base::OnceCallback.​

Change-Id: I14ca9af2b3abc095b978c2cd7cf1fea1844ef428
Reviewed-on: https://chromium-review.googlesource.com/817740
Commit-Queue: Yannic Bonenberger <contact@yannic-bonenberger.com>
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571999}

docs/clang_tool_refactoring.md

@@ -123,9 +123,9 @@ ninja -C out/Debug  # For non-Windows
 ninja -d keeprsp -C out/Debug  # For Windows
 
 # experimental alternative:
-$gen_targets = $(ninja -C out/gn -t targets all \
+$gen_targets = $(ninja -C out/Debug -t targets all \
                  | grep '^gen/[^: ]*\.[ch][pc]*:' \
-                 | cut -f 1 -d :`)
+                 | cut -f 1 -d :)
 ninja -C out/Debug $gen_targets
 ```
 
@@ -150,7 +150,7 @@ from `//base` that got included by source files in `//cc`).
 
 ```shell
 tools/clang/scripts/run_tool.py --tool empty_string  \
-  --generated-compdb \
+  --generate-compdb \
   -p out/Debug net >/tmp/list-of-edits.debug
 ```
 

tools/clang/base_bind_rewriters/BaseBindRewriters.cpp

@@ -161,9 +161,8 @@ class BindOnceRewriter : public MatchFinder::MatchCallback, public Rewriter {
     auto constructor_conversion = cxxConstructExpr(
         is_once_callback, argumentCountIs(1),
         hasArgument(0, ignoringImplicit(parameter_construction)));
-    auto implicit_conversion = implicitCastExpr(
-        is_once_callback, hasSourceExpression(constructor_conversion));
-    return implicit_conversion;
+    return implicitCastExpr(is_once_callback,
+                            hasSourceExpression(constructor_conversion));
   }
 
   void run(const MatchFinder::MatchResult& result) override {
@@ -577,6 +576,64 @@ class AddStdMoveRewriter : public MatchFinder::MatchCallback, public Rewriter {
   Replacements* replacements_;
 };
 
+// Remove base::AdaptCallbackForRepeating() where resulting
+// base::RepeatingCallback is implicitly converted into base::OnceCallback.
+// Example:
+//   // Before
+//   base::PostTask(
+//       FROM_HERE,
+//       base::AdaptCallbackForRepeating(base::BindOnce(&Foo)));
+//   base::OnceCallback<void()> cb = base::AdaptCallbackForRepeating(
+//       base::OnceBind(&Foo));
+//
+//   // After
+//   base::PostTask(FROM_HERE, base::BindOnce(&Foo));
+//   base::OnceCallback<void()> cb = base::BindOnce(&Foo);
+class AdaptCallbackForRepeatingRewriter : public MatchFinder::MatchCallback,
+                                          public Rewriter {
+ public:
+  explicit AdaptCallbackForRepeatingRewriter(Replacements* replacements)
+      : replacements_(replacements) {}
+
+  StatementMatcher GetMatcher() {
+    auto is_once_callback = hasType(hasCanonicalType(hasDeclaration(
+        classTemplateSpecializationDecl(hasName("::base::OnceCallback")))));
+    auto is_repeating_callback =
+        hasType(hasCanonicalType(hasDeclaration(classTemplateSpecializationDecl(
+            hasName("::base::RepeatingCallback")))));
+
+    auto adapt_callback_call =
+        callExpr(
+            callee(namedDecl(hasName("::base::AdaptCallbackForRepeating"))))
+            .bind("target");
+    auto parameter_construction =
+        cxxConstructExpr(is_repeating_callback, argumentCountIs(1),
+                         hasArgument(0, ignoringImplicit(adapt_callback_call)));
+    auto constructor_conversion = cxxConstructExpr(
+        is_once_callback, argumentCountIs(1),
+        hasArgument(0, ignoringImplicit(parameter_construction)));
+    return implicitCastExpr(is_once_callback,
+                            hasSourceExpression(constructor_conversion));
+  }
+
+  void run(const MatchFinder::MatchResult& result) override {
+    auto* target = result.Nodes.getNodeAs<clang::CallExpr>("target");
+
+    auto left = clang::CharSourceRange::getTokenRange(
+        result.SourceManager->getSpellingLoc(target->getLocStart()),
+        result.SourceManager->getSpellingLoc(target->getArg(0)->getExprLoc())
+            .getLocWithOffset(-1));
+    replacements_->emplace_back(*result.SourceManager, left, "");
+    auto r_paren = clang::CharSourceRange::getTokenRange(
+        result.SourceManager->getSpellingLoc(target->getRParenLoc()),
+        result.SourceManager->getSpellingLoc(target->getRParenLoc()));
+    replacements_->emplace_back(*result.SourceManager, r_paren, "");
+  }
+
+ private:
+  Replacements* replacements_;
+};
+
 llvm::cl::extrahelp common_help(CommonOptionsParser::HelpMessage);
 llvm::cl::OptionCategory rewriter_category("Rewriter Options");
 
@@ -588,6 +645,7 @@ Available rewriters are:
     bind_to_bind_once
     pass_by_value
     add_std_move
+    remove_unneeded_adapt_callback
 The default is remove_unneeded_passed.
 )"),
     llvm::cl::init("remove_unneeded_passed"),
@@ -623,6 +681,12 @@ int main(int argc, const char* argv[]) {
     auto add_std_move = llvm::make_unique<AddStdMoveRewriter>(&replacements);
     match_finder.addMatcher(add_std_move->GetMatcher(), add_std_move.get());
     rewriter = std::move(add_std_move);
+  } else if (rewriter_option == "remove_unneeded_adapt_callback") {
+    auto remove_unneeded_adapt_callback =
+        llvm::make_unique<AdaptCallbackForRepeatingRewriter>(&replacements);
+    match_finder.addMatcher(remove_unneeded_adapt_callback->GetMatcher(),
+                            remove_unneeded_adapt_callback.get());
+    rewriter = std::move(remove_unneeded_adapt_callback);
   } else {
     abort();
   }

tools/clang/base_bind_rewriters/tests/callback.h

@@ -95,6 +95,11 @@ RepeatingCallback<void()> BindRepeating(Functor, Args&&...) {
   return RepeatingCallback<void()>();
 }
 
+RepeatingCallback<void()> AdaptCallbackForRepeating(
+    OnceCallback<void()> callback) {
+  return Callback<void()>();
+}
+
 }  // namespace base
 
 #endif  // TOOLS_CLANG_BASE_BIND_REWRITERS_TESTS_CALLBACK_H_

tools/clang/base_bind_rewriters/tests/remove-adapt-callback-for-repeating-expected.cc

@@ -0,0 +1,19 @@
+// Copyright 2018 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 "callback.h"
+
+void Foo(base::OnceClosure) {}
+
+void Test() {
+  base::OnceClosure cb = base::BindOnce([] {});
+  Foo(base::BindOnce([] {}));
+
+  using namespace base;
+
+  OnceClosure cb2 = BindOnce([] {});
+  Foo(BindOnce([] {}));
+
+  OnceClosure cb3 = base::BindOnce([] {});
+}

tools/clang/base_bind_rewriters/tests/remove-adapt-callback-for-repeating-original.cc

@@ -0,0 +1,19 @@
+// Copyright 2018 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 "callback.h"
+
+void Foo(base::OnceClosure) {}
+
+void Test() {
+  base::OnceClosure cb = base::AdaptCallbackForRepeating(base::BindOnce([] {}));
+  Foo(base::AdaptCallbackForRepeating(base::BindOnce([] {})));
+
+  using namespace base;
+
+  OnceClosure cb2 = AdaptCallbackForRepeating(BindOnce([] {}));
+  Foo(AdaptCallbackForRepeating(BindOnce([] {})));
+
+  OnceClosure cb3 = base::BindOnce([] {});
+}