Rewriting |printf("%p", s.ptr_field)| into |...s.ptr_field.get()...|.

Bug: 1069567
Change-Id: Ib1711e1ee47ca14d448b32fb680d061c658a2eff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2159578
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774790}

tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp

@@ -60,7 +60,8 @@ class ReplacementsPrinter : public clang::tooling::SourceFileCallbacks {
 
   void PrintReplacement(const clang::SourceManager& source_manager,
                         const clang::SourceRange& replacement_range,
-                        std::string replacement_text) {
+                        std::string replacement_text,
+                        bool should_add_include = false) {
     if (ShouldSuppressOutput())
       return;
 
@@ -75,12 +76,14 @@ class ReplacementsPrinter : public clang::tooling::SourceFileCallbacks {
                  << ":::" << replacement.getLength()
                  << ":::" << replacement_text << "\n";
 
-    bool was_inserted = false;
-    std::tie(std::ignore, was_inserted) =
-        files_with_already_added_includes_.insert(file_path.str());
-    if (was_inserted)
-      llvm::outs() << "include-user-header:::" << file_path
-                   << ":::-1:::-1:::" << kIncludePath << "\n";
+    if (should_add_include) {
+      bool was_inserted = false;
+      std::tie(std::ignore, was_inserted) =
+          files_with_already_added_includes_.insert(file_path.str());
+      if (was_inserted)
+        llvm::outs() << "include-user-header:::" << file_path
+                     << ":::-1:::-1:::" << kIncludePath << "\n";
+    }
   }
 
  private:
@@ -290,6 +293,9 @@ AST_MATCHER(clang::FieldDecl, hasUniqueTypeLoc) {
   return !has_sibling_with_same_type_loc;
 }
 
+// Rewrites |SomeClass* field| (matched as "fieldDecl") into
+// |CheckedPtr<SomeClass> field| and for each file rewritten in such way adds an
+// |#include "base/memory/checked_ptr.h"|.
 class FieldDeclRewriter : public MatchFinder::MatchCallback {
  public:
   explicit FieldDeclRewriter(ReplacementsPrinter* replacements_printer)
@@ -334,7 +340,8 @@ class FieldDeclRewriter : public MatchFinder::MatchCallback {
 
     // Generate and print a replacement.
     replacements_printer_->PrintReplacement(source_manager, replacement_range,
-                                            replacement_text);
+                                            replacement_text,
+                                            true /* should_add_include */);
   }
 
  private:
@@ -366,6 +373,35 @@ class FieldDeclRewriter : public MatchFinder::MatchCallback {
   ReplacementsPrinter* const replacements_printer_;
 };
 
+// Rewrites |my_struct.ptr_field| (matched as "affectedMemberExpr") into
+// |my_struct.ptr_field.get()|.
+class AffectedExprRewriter : public MatchFinder::MatchCallback {
+ public:
+  AffectedExprRewriter(ReplacementsPrinter* replacements_printer)
+      : replacements_printer_(replacements_printer) {}
+
+  void run(const MatchFinder::MatchResult& result) override {
+    const clang::SourceManager& source_manager = *result.SourceManager;
+
+    const clang::MemberExpr* member_expr =
+        result.Nodes.getNodeAs<clang::MemberExpr>("affectedMemberExpr");
+    assert(member_expr && "matcher should bind 'affectedMemberExpr'");
+
+    clang::SourceLocation member_name_start = member_expr->getMemberLoc();
+    size_t member_name_length = member_expr->getMemberDecl()->getName().size();
+    clang::SourceLocation insertion_loc =
+        member_name_start.getLocWithOffset(member_name_length);
+
+    clang::SourceRange replacement_range(insertion_loc, insertion_loc);
+
+    replacements_printer_->PrintReplacement(source_manager, replacement_range,
+                                            ".get()");
+  }
+
+ private:
+  ReplacementsPrinter* const replacements_printer_;
+};
+
 }  // namespace
 
 int main(int argc, const char* argv[]) {
@@ -444,6 +480,31 @@ int main(int argc, const char* argv[]) {
   FieldDeclRewriter field_decl_rewriter(&replacements_printer);
   match_finder.addMatcher(field_decl_matcher, &field_decl_rewriter);
 
+  // Matches expressions that used to return a value of type |SomeClass*|
+  // but after the rewrite return an instance of |CheckedPtr<SomeClass>|.
+  auto affected_member_expr_matcher =
+      memberExpr(member(field_decl_matcher)).bind("affectedMemberExpr");
+  auto affected_implicit_expr_matcher = implicitCastExpr(has(expr(anyOf(
+      // Only single implicitCastExpr is present in case of:
+      // |auto* v = s.ptr_field;|
+      expr(affected_member_expr_matcher),
+      // 2nd nested implicitCastExpr is present in case of:
+      // |const auto* v = s.ptr_field;|
+      expr(implicitCastExpr(has(affected_member_expr_matcher)))))));
+
+  // Printf-style call expr =========
+  // Given
+  //   void foo(const S& s) {
+  //     printf("%p", s.y);
+  //   }
+  // matches the |s.y| expr if it matches the |affected_implicit_expr_matcher|
+  // above.
+  auto affected_printf_arg_matcher =
+      expr(allOf(affected_implicit_expr_matcher,
+                 hasParent(callExpr(callee(functionDecl(isVariadic()))))));
+  AffectedExprRewriter printf_arg_rewriter(&replacements_printer);
+  match_finder.addMatcher(affected_printf_arg_matcher, &printf_arg_rewriter);
+
   // Prepare and run the tool.
   std::unique_ptr<clang::tooling::FrontendActionFactory> factory =
       clang::tooling::newFrontendActionFactory(&match_finder,

tools/clang/rewrite_raw_ptr_fields/tests/affected-expr-expected.cc

+// Copyright 2020 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/checked_ptr.h"
+
+class SomeClass;
+
+struct MyStruct {
+  CheckedPtr<SomeClass> ptr;
+  CheckedPtr<SomeClass> ptr2;
+};
+
+int ConvertSomeClassToInt(SomeClass* some_class) {
+  return 123;
+}
+
+void MyPrintf(const char* fmt, ...) {}
+
+void foo() {
+  MyStruct s;
+
+  // Expected rewrite: MyPrintf("%p", s.ptr.get());
+  MyPrintf("%p", s.ptr.get());
+
+  // Test - all arguments are rewritten.
+  // Expected rewrite: MyPrintf("%p, %p", s.ptr.get(), s.ptr2.get());
+  MyPrintf("%p, %p", s.ptr.get(), s.ptr2.get());
+
+  // Test - only |s.ptr|-style arguments are rewritten.
+  // Expected rewrite: MyPrintf("%d, %p", 123, s.ptr.get());
+  MyPrintf("%d, %p", 123, s.ptr.get());
+
+  // Test - |s.ptr| is deeply nested.
+  // No rewrite expected.
+  MyPrintf("%d", ConvertSomeClassToInt(s.ptr));
+}

tools/clang/rewrite_raw_ptr_fields/tests/affected-expr-original.cc

+// Copyright 2020 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.
+
+class SomeClass;
+
+struct MyStruct {
+  SomeClass* ptr;
+  SomeClass* ptr2;
+};
+
+int ConvertSomeClassToInt(SomeClass* some_class) {
+  return 123;
+}
+
+void MyPrintf(const char* fmt, ...) {}
+
+void foo() {
+  MyStruct s;
+
+  // Expected rewrite: MyPrintf("%p", s.ptr.get());
+  MyPrintf("%p", s.ptr);
+
+  // Test - all arguments are rewritten.
+  // Expected rewrite: MyPrintf("%p, %p", s.ptr.get(), s.ptr2.get());
+  MyPrintf("%p, %p", s.ptr, s.ptr2);
+
+  // Test - only |s.ptr|-style arguments are rewritten.
+  // Expected rewrite: MyPrintf("%d, %p", 123, s.ptr.get());
+  MyPrintf("%d, %p", 123, s.ptr);
+
+  // Test - |s.ptr| is deeply nested.
+  // No rewrite expected.
+  MyPrintf("%d", ConvertSomeClassToInt(s.ptr));
+}