CheckedPtr rewriter: Skip fields resulting in global destructors.

After this CL, the blocklist emitted by the rewriter will include fields
that may cause presence of global destructors (if CheckedPtr has a
non-trivial destructor, such as required by BackupRefPtr
implementation).

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

tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp

@@ -673,6 +673,20 @@ AST_MATCHER(clang::FieldDecl, overlapsOtherDeclsWithinRecordDecl) {
   return has_sibling_with_overlapping_location;
 }
 
+// Matches RecordDecl if
+// 1) it has a FieldDecl that matches the InnerMatcher
+// or
+// 2) it has a FieldDecl that hasType of a RecordDecl that matches the
+//    InnerMatcher (this recurses to any depth).
+AST_MATCHER_P(clang::RecordDecl,
+              hasNestedFieldDecl,
+              clang::ast_matchers::internal::Matcher<clang::FieldDecl>,
+              InnerMatcher) {
+  auto matcher = recordDecl(has(fieldDecl(anyOf(
+      InnerMatcher, hasType(recordDecl(hasNestedFieldDecl(InnerMatcher)))))));
+  return matcher.matches(Node, Finder, Builder);
+}
+
 // Rewrites |SomeClass* field| (matched as "affectedFieldDecl") into
 // |CheckedPtr<SomeClass> field| and for each file rewritten in such way adds an
 // |#include "base/memory/checked_ptr.h"|.
@@ -1041,6 +1055,14 @@ int main(int argc, const char* argv[]) {
   match_finder.addMatcher(char_ptr_field_decl_matcher,
                           &char_ptr_field_decl_writer);
 
+  // See the testcases in tests/gen-global-destructor-test.cc.
+  auto global_destructor_matcher = varDecl(
+      allOf(hasGlobalStorage(),
+            hasType(recordDecl(hasNestedFieldDecl(field_decl_matcher)))));
+  FilteredExprWriter global_destructor_writer(&output_helper,
+                                              "global-destructor");
+  match_finder.addMatcher(global_destructor_matcher, &global_destructor_writer);
+
   // Prepare and run the tool.
   std::unique_ptr<clang::tooling::FrontendActionFactory> factory =
       clang::tooling::newFrontendActionFactory(&match_finder, &output_helper);

tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-expected.txt

+==== BEGIN EDITS ====
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::-1:::-1:::base/memory/checked_ptr.h
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::1715:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::1940:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::2192:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::2502:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-actual.cc:::2644:::10:::CheckedPtr<MyStruct> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+global_variables_test::MyStruct::ptr  # global-destructor
+nested_struct_test::MyStruct::ptr  # global-destructor
+pointer_nesting_test::MyOuterStruct::inner_struct  # global-destructor
+static_variables_test::MyStruct::ptr  # global-destructor
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/gen-global-destructor-test.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.
+
+// This file (and other gen-*-test.cc files) tests generation of output for
+// --field-filter-file and therefore the expectations file
+// (gen-global-destructor-expected.txt) needs to be compared against the raw
+// output of the rewriter (rather than against the actual edits result).  This
+// makes the test incompatible with other tests, which require passing
+// --apply-edits switch to test_tool.py and so to disable the test it is named
+// *-test.cc rather than *-original.cc.
+//
+// To run the test use tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py
+
+// Chromium is built with a warning/error that global and static variables
+// may only have trivial destructors.  See also:
+// https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
+// go/totw/110#destruction
+//
+// If CheckedPtr has a non-trivial destructor (e.g. if it is implemented via
+// BackupRefPtr) then CheckedPtr cannot be used as the type of fields in structs
+// that are (recursively/transitively) the type of a global variable:
+//     struct MyStruct {       //    Presence of CheckedPtr might mean that
+//       CheckedPtr<int> ptr;  // <- MyStruct has a non-trivial destructor.
+//     };
+//     MyStruct g_struct;  // <- Error if MyStruct has a non-trivial destructor.
+//
+// To account for the constraints described above, the rewriter tool should
+// avoid rewriting some of the fields below.
+
+namespace global_variables_test {
+
+struct MyStruct {
+  // Expected to be emitted in automated-fields-to-ignore.txt, because
+  // of |g_struct| below.
+  int* ptr;
+};
+
+MyStruct g_struct;
+
+}  // namespace global_variables_test
+
+namespace static_variables_test {
+
+struct MyStruct {
+  // Expected to be emitted in automated-fields-to-ignore.txt, because
+  // of |s_struct| below.
+  int* ptr;
+};
+
+void foo() {
+  static MyStruct s_struct;
+}
+
+}  // namespace static_variables_test
+
+namespace nested_struct_test {
+
+struct MyStruct {
+  // Expected to be emitted in automated-fields-to-ignore.txt, because
+  // of |g_outer_struct| below.
+  int* ptr;
+};
+
+struct MyOuterStruct {
+  MyStruct inner_struct;
+};
+
+static MyOuterStruct g_outer_struct;
+
+}  // namespace nested_struct_test
+
+namespace pointer_nesting_test {
+
+struct MyStruct {
+  // Should not be emitted in automated-fields-to-ignore.txt, because
+  // |inner_struct| field below is a pointer.  (i.e. this is a test that
+  // |hasNestedFieldDecl| matcher doesn't recurse/traverse over pointers)
+  int* ptr;
+};
+
+struct MyOuterStruct {
+  // Expected to be emitted in automated-fields-to-ignore.txt, because
+  // of |g_outer_struct| below.
+  MyStruct* inner_struct;
+};
+
+static MyOuterStruct g_outer_struct;
+
+}  // namespace pointer_nesting_test