value_cleanup: fixup ListValue::Append() calls that already have a unique_ptr to use.

tools/clang/value_cleanup/ListValueRewriter.cpp

Index: tools/clang/value_cleanup/ListValueRewriter.cpp
diff --git a/tools/clang/value_cleanup/ListValueRewriter.cpp b/tools/clang/value_cleanup/ListValueRewriter.cpp
index ce27dd1f5b41368ddbf1ac5eb19955ae67d3bee2..b5e5260eab1e6210d71425bb85079440e94f6246 100644
--- a/tools/clang/value_cleanup/ListValueRewriter.cpp
+++ b/tools/clang/value_cleanup/ListValueRewriter.cpp
@@ -108,11 +108,39 @@ void ListValueRewriter::AppendStringCallback::run(
   AppendCallback::run(result);
 }
 
+ListValueRewriter::AppendReleasedUniquePtrCallback::
+    AppendReleasedUniquePtrCallback(Replacements* replacements)
+    : replacements_(replacements) {}
+
+void ListValueRewriter::AppendReleasedUniquePtrCallback::run(
+    const MatchFinder::MatchResult& result) {
+  auto* object_expr = result.Nodes.getNodeAs<clang::Expr>("objectExpr");
+  bool arg_is_rvalue = object_expr->Classify(*result.Context).isRValue();
+
+  // Remove .release()
+  auto* member_call =
+      result.Nodes.getNodeAs<clang::CXXMemberCallExpr>("memberCall");
+  auto* member_expr = result.Nodes.getNodeAs<clang::MemberExpr>("memberExpr");

[vmpstr, 2016/06/09 00:15:59]

Does this always find the release/last method?

I'm thinking of something like 

foo.clone().release()

Or I guess it will be called with a match "foo.clone()" instead of "foo"

[dcheng, 2016/06/09 00:57:12]

This will match the rvalue reference case, with things bound as such:

objectExpr will be bound to foo.clone()
memberCall will be bound to foo.clone.release()
memberExpr will be bound to foo.clone.release

The only reason I bind to memberExpr is to easily grab the location of the . /
-> operator.

+  clang::CharSourceRange release_range = clang::CharSourceRange::getTokenRange(
+      member_expr->getOperatorLoc(), member_call->getLocEnd());
+  replacements_->emplace(*result.SourceManager, release_range,
+                         arg_is_rvalue ? "" : ")");
+
+  if (arg_is_rvalue)
+    return;
+
+  // Insert `std::move(' for non-rvalue expressions.
+  clang::CharSourceRange insertion_range = clang::CharSourceRange::getCharRange(
+      object_expr->getLocStart(), object_expr->getLocStart());
+  replacements_->emplace(*result.SourceManager, insertion_range, "std::move(");
+}
+
 ListValueRewriter::ListValueRewriter(Replacements* replacements)
     : append_boolean_callback_(replacements),
       append_integer_callback_(replacements),
       append_double_callback_(replacements),
-      append_string_callback_(replacements) {}
+      append_string_callback_(replacements),
+      append_released_unique_ptr_callback_(replacements) {}
 
 void ListValueRewriter::RegisterMatchers(MatchFinder* match_finder) {
   auto is_list_append =
@@ -191,4 +219,26 @@ void ListValueRewriter::RegisterMatchers(MatchFinder* match_finder) {
                             argumentCountIs(1),
                             hasArgument(0, id("argExpr", expr())))))))))),
       &append_string_callback_);
+
+  auto is_unique_ptr_release =
+      allOf(callee(cxxMethodDecl(
+                hasName("release"),
+                ofClass(cxxRecordDecl(hasName("::std::unique_ptr"))))),
+            argumentCountIs(0));
+
+  // base::ListValue::Append(ReturnsUniquePtr().release())
+  //     => base::ListValue::Append(ReturnsUniquePtr())
+  //   or
+  // base::ListValue::Append(unique_ptr_var.release())
+  //     => base::ListValue::Append(std::move(unique_ptr_var))
+  match_finder->addMatcher(
+      cxxMemberCallExpr(
+          is_list_append,
+          hasArgument(
+              0, ignoringParenImpCasts(
+                     id("memberCall",
+                        cxxMemberCallExpr(has(id("memberExpr", memberExpr())),
+                                          is_unique_ptr_release,
+                                          on(id("objectExpr", expr()))))))),

[vmpstr, 2016/06/09 00:15:59]

:)))))))))))

+      &append_released_unique_ptr_callback_);
 }