Skip some scenarios via blocklist, rather than unconditionally.

After this CL, the rewriter will no longer unconditionally skip
fields in the following scenarios:
- Overlapping fields (e.g. |SomeClass *field1, *field2;|).
- Char pointer fields (e.g. |char* field| or |wchar_t* field|)
- Fields expanding in macro scratch space

Instead, the rewriter will emit such fields as candidates for the
--field-filter-file blocklist - in the following section of the output:

    ==== BEGIN FIELD FILTERS ====
    MyStruct::const_char_ptr  # char
    MacroTest1::ptr_field  # macro
    my_namespace::MyStruct::overlapping_1a  # overlapping
    ==== END FIELD FILTERS ====

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

tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp

@@ -8,6 +8,20 @@
 // becomes:
 //     CheckedPtr<Pointee> field_
 //
+// Note that the tool always emits two kinds of output:
+// 1. Fields to exclude:
+//    - FilteredExprWriter
+// 2. Edit/replacement directives:
+//    - FieldDeclRewriter
+//    - AffectedExprRewriter
+// The rewriter is expected to be used twice, in two passes:
+// 1. Output from the 1st pass should be used to generate fields-to-ignore.txt
+//    (or to augment the manually created exclusion list file)
+// 2. The 2nd pass should use fields-to-ignore.txt from the first pass as input
+//    for the --exclude-fields cmdline parameter.  The output from the 2nd pass
+//    can be used to perform the actual rewrite via extract_edits.py and
+//    apply_edits.py.
+//
 // For more details, see the doc here:
 // https://docs.google.com/document/d/1chTvr3fSofQNV_PDPEHRyUgcJCQBgTDOOBriW9gIm9M
 
@@ -33,6 +47,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -133,7 +148,8 @@ class OutputHelper : public clang::tooling::SourceFileCallbacks {
         source_manager, clang::CharSourceRange::getCharRange(replacement_range),
         replacement_text);
     llvm::StringRef file_path = replacement.getFilePath();
-    assert(!file_path.empty());
+    if (file_path.empty())
+      return;
 
     std::replace(replacement_text.begin(), replacement_text.end(), '\n', '\0');
     llvm::outs() << "r:::" << file_path << ":::" << replacement.getOffset()
@@ -668,21 +684,19 @@ int main(int argc, const char* argv[]) {
   //   struct MyStrict {
   //     int* int_ptr;
   //     int i;
-  //     char* char_ptr;
   //     int (*func_ptr)();
   //     int (MyStruct::* member_func_ptr)(char);
   //     int (*ptr_to_array_of_ints)[123]
   //     StructOrClassWithDeletedOperatorNew* stack_or_gc_ptr;
-  //     struct { int i }* ptr_to_non_free_standing_record_or_union_or_class;
   //   };
   // matches |int*|, but not the other types.
   auto record_with_deleted_allocation_operator_type_matcher =
       recordType(hasDeclaration(cxxRecordDecl(
           hasMethod(allOf(hasOverloadedOperatorName("new"), isDeleted())))));
   auto supported_pointer_types_matcher =
-      pointerType(unless(pointee(hasUnqualifiedDesugaredType(anyOf(
-          record_with_deleted_allocation_operator_type_matcher, functionType(),
-          memberPointerType(), anyCharType(), arrayType())))));
+      pointerType(unless(pointee(hasUnqualifiedDesugaredType(
+          anyOf(record_with_deleted_allocation_operator_type_matcher,
+                functionType(), memberPointerType(), arrayType())))));
 
   // Implicit field declarations =========
   // Matches field declarations that do not explicitly appear in the source
@@ -715,10 +729,8 @@ int main(int argc, const char* argv[]) {
   auto field_decl_matcher =
       fieldDecl(
           allOf(hasType(supported_pointer_types_matcher),
-                unless(anyOf(overlapsOtherDeclsWithinRecordDecl(),
-                             isInThirdPartyLocation(), isInGeneratedLocation(),
-                             isExpansionInSystemHeader(), isInMacroLocation(),
-                             isInExternCContext(),
+                unless(anyOf(isInThirdPartyLocation(), isInGeneratedLocation(),
+                             isExpansionInSystemHeader(), isInExternCContext(),
                              isListedInFilterFile(fields_to_exclude),
                              implicit_field_decl_matcher))))
           .bind("affectedFieldDecl");
@@ -755,6 +767,8 @@ int main(int argc, const char* argv[]) {
   //     reinterpret_cast<...>(s.y)
   //   }
   // matches the |s.y| expr if it matches the |affected_expr_matcher| above.
+  //
+  // See also testcases in tests/affected-expr-original.cc
   auto affected_expr_that_needs_fixing_matcher = expr(allOf(
       affected_expr_matcher,
       hasParent(expr(anyOf(callExpr(callee(functionDecl(isVariadic()))),
@@ -769,6 +783,8 @@ int main(int argc, const char* argv[]) {
   //     cond ? s.y : ...
   //   }
   // binds the |s.y| expr if it matches the |affected_expr_matcher| above.
+  //
+  // See also testcases in tests/affected-expr-original.cc
   auto affected_ternary_operator_arg_matcher =
       conditionalOperator(eachOf(hasTrueExpression(affected_expr_matcher),
                                  hasFalseExpression(affected_expr_matcher)));
@@ -784,6 +800,8 @@ int main(int argc, const char* argv[]) {
   //     templatedFunc(s.y);
   //   }
   // binds the |s.y| expr if it matches the |affected_expr_matcher| above.
+  //
+  // See also testcases in tests/affected-expr-original.cc
   auto templated_function_arg_matcher = forEachArgumentWithParam(
       affected_expr_matcher, parmVarDecl(hasType(qualType(allOf(
                                  findAll(qualType(substTemplateTypeParmType())),
@@ -800,6 +818,8 @@ int main(int argc, const char* argv[]) {
   //     auto* p = s.y;
   //   }
   // binds the |s.y| expr if it matches the |affected_expr_matcher| above.
+  //
+  // See also testcases in tests/affected-expr-original.cc
   auto auto_var_decl_matcher = declStmt(forEach(
       varDecl(allOf(hasType(pointerType(pointee(autoType()))),
                     hasInitializer(anyOf(
@@ -812,6 +832,8 @@ int main(int argc, const char* argv[]) {
   //   ... &s.y ...
   // matches the |s.y| expr if it matches the |affected_member_expr_matcher|
   // above.
+  //
+  // See also the testcases in tests/gen-in-out-arg-test.cc.
   auto affected_addr_of_expr_matcher = expr(allOf(
       affected_expr_matcher, hasParent(unaryOperator(hasOperatorName("&")))));
   FilteredExprWriter filtered_addr_of_expr_writer(&output_helper, "addr-of");
@@ -829,6 +851,8 @@ int main(int argc, const char* argv[]) {
   // matches the |s.ptr_field| expr if it matches the
   // |affected_member_expr_matcher| and is passed as a function argument that
   // has |FooBar*&| type.
+  //
+  // See also the testcases in tests/gen-in-out-arg-test.cc.
   auto affected_in_out_ref_arg_matcher = callExpr(forEachArgumentWithParam(
       affected_expr_matcher.bind("expr"),
       parmVarDecl(hasType(referenceType(pointee(pointerType()))))));
@@ -837,6 +861,31 @@ int main(int argc, const char* argv[]) {
   match_finder.addMatcher(affected_in_out_ref_arg_matcher,
                           &filtered_in_out_ref_arg_writer);
 
+  // See the doc comment for the overlapsOtherDeclsWithinRecordDecl matcher
+  // and the testcases in tests/gen-overlaps-test.cc.
+  auto overlapping_field_decl_matcher = fieldDecl(
+      allOf(field_decl_matcher, overlapsOtherDeclsWithinRecordDecl()));
+  FilteredExprWriter overlapping_field_decl_writer(&output_helper,
+                                                   "overlapping");
+  match_finder.addMatcher(overlapping_field_decl_matcher,
+                          &overlapping_field_decl_writer);
+
+  // See the doc comment for the isInMacroLocation matcher
+  // and the testcases in tests/gen-macro-test.cc.
+  auto macro_field_decl_matcher =
+      fieldDecl(allOf(field_decl_matcher, isInMacroLocation()));
+  FilteredExprWriter macro_field_decl_writer(&output_helper, "macro");
+  match_finder.addMatcher(macro_field_decl_matcher, &macro_field_decl_writer);
+
+  // See the doc comment for the anyCharType matcher
+  // and the testcases in tests/gen-char-test.cc.
+  auto char_ptr_field_decl_matcher = fieldDecl(allOf(
+      field_decl_matcher, hasType(pointerType(pointee(
+                              hasUnqualifiedDesugaredType(anyCharType()))))));
+  FilteredExprWriter char_ptr_field_decl_writer(&output_helper, "char");
+  match_finder.addMatcher(char_ptr_field_decl_matcher,
+                          &char_ptr_field_decl_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/basics-expected.cc

@@ -29,12 +29,6 @@ struct MyStruct {
   // clang-format off
   CheckedPtr<SomeClass> raw_ptr_field2;
   // clang-format on
-
-  // No rewrite expected.  (The syntax below should be rare + it is difficult to
-  // generate correct, non-overlapping replacements that cover both fields.)
-  SomeClass *overlapping_fields_1a, *overlapping_fields_1b;
-  int overlapping_fields_2a, *overlapping_fields_2b;
-  int *overlapping_fields_3a, overlapping_fields_3b;
 };
 
 template <typename T>

tools/clang/rewrite_raw_ptr_fields/tests/basics-original.cc

@@ -27,12 +27,6 @@ struct MyStruct {
   // clang-format off
   SomeClass *raw_ptr_field2;
   // clang-format on
-
-  // No rewrite expected.  (The syntax below should be rare + it is difficult to
-  // generate correct, non-overlapping replacements that cover both fields.)
-  SomeClass *overlapping_fields_1a, *overlapping_fields_1b;
-  int overlapping_fields_2a, *overlapping_fields_2b;
-  int *overlapping_fields_3a, overlapping_fields_3b;
 };
 
 template <typename T>

tools/clang/rewrite_raw_ptr_fields/tests/gen-char-expected.txt

+==== BEGIN EDITS ====
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-char-actual.cc:::1343:::6:::CheckedPtr<char> 
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-char-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-char-actual.cc:::1361:::12:::CheckedPtr<const char> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-char-actual.cc:::1391:::9:::CheckedPtr<wchar_t> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-char-actual.cc:::1417:::15:::CheckedPtr<const wchar_t> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+MyStruct::char_ptr  # char
+MyStruct::const_char_ptr  # char
+MyStruct::const_wide_char_ptr  # char
+MyStruct::wide_char_ptr  # char
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/gen-char-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-char-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
+
+struct MyStruct {
+  // Chromium is built with a warning/error that there are no user-defined
+  // constructors invoked when initializing global-scoped values.
+  // CheckedPtr<char> conversion might trigger a global constructor for string
+  // literals:
+  //     struct MyStruct {
+  //       int foo;
+  //       CheckedPtr<const char> bar;
+  //     }
+  //     MyStruct g_foo = {123, "string literal" /* global constr! */};
+  //
+  // Because of the above, we have a heuristic for all fields that point to a
+  // char-like type - the heuristic causes such fields to be emitted as
+  // candidates for the --field-filter-file.
+  char* char_ptr;
+  const char* const_char_ptr;
+  wchar_t* wide_char_ptr;
+  const wchar_t* const_wide_char_ptr;
+};

tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-expected.txt → tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-expected.txt

 ==== BEGIN EDITS ====
-r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-actual.cc:::861:::11:::CheckedPtr<SomeClass> 
-include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-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/in-out-arg-actual.cc:::885:::11:::CheckedPtr<SomeClass> 
-r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-actual.cc:::914:::11:::CheckedPtr<SomeClass> 
-r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-actual.cc:::943:::11:::CheckedPtr<SomeClass> 
-r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-actual.cc:::1137:::3:::CheckedPtr<T> 
-r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-actual.cc:::1809:::3:::CheckedPtr<T> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-actual.cc:::789:::11:::CheckedPtr<SomeClass> 
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-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-in-out-arg-actual.cc:::813:::11:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-actual.cc:::842:::11:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-actual.cc:::871:::11:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-actual.cc:::1065:::3:::CheckedPtr<T> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-actual.cc:::1737:::3:::CheckedPtr<T> 
 ==== END EDITS ====
 ==== BEGIN FIELD FILTERS ====
 my_namespace::MyStruct::in_out_via_auto_reset  # addr-of

tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-test.cc → tools/clang/rewrite_raw_ptr_fields/tests/gen-in-out-arg-test.cc

@@ -2,18 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// This file tests generation of output for --field-filter-file and therefore
-// the expectations file (in-out-arg-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.
+// This file (and other gen-*-test.cc files) tests generation of output for
+// --field-filter-file and therefore the expectations file
+// (gen-in-out-arg-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 enable and run the test:
-// 1. Rename this file to in-out-arg-original.cc
-// 2. Run:
-//    $ tools/clang/scripts/test_tool.py rewrite_raw_ptr_fields \
-//         --test-filter=in-out-arg
+// To run the test use tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py
 
 namespace my_namespace {
 

tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-expected.txt

+==== BEGIN EDITS ====
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-actual.cc:::1940:::13:::CheckedPtr<struct event> 
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-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-macros-actual.cc:::2009:::14:::CheckedPtr<struct event *> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-actual.cc:::1940:::13:::CheckedPtr<struct event> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-actual.cc:::2009:::14:::CheckedPtr<struct event *> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-actual.cc:::1940:::13:::CheckedPtr<struct event> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-actual.cc:::2009:::14:::CheckedPtr<struct event *> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+MacroTest1::ptr_field  # macro
+MacroTest2::(anonymous struct)::tqe_next  # macro
+MacroTest2::(anonymous struct)::tqe_prev  # macro
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/macros-expected.cc → tools/clang/rewrite_raw_ptr_fields/tests/gen-macros-test.cc

@@ -2,8 +2,21 @@
 // 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-macros-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
+
 //////////////////////////////////////////////////////////////////////////////
 // Based on build/linux/debian_sid_amd64-sysroot/usr/include/link.h
+//
+// We expect that |ptr_field| will be emitted as a candidate for
+// --field-filter-file.
 
 struct Elf64_Dyn;
 
@@ -18,6 +31,15 @@ struct MacroTest1 {
 
 //////////////////////////////////////////////////////////////////////////////
 // Based on base/third_party/libevent/event.h
+//
+// We expect that |tqe_next| and |tqe_prev| fields below will both be emitted as
+// candidates for --field-filter-file.
+//
+// This test is also interesting for noting that a fully-qualified name of a
+// field decl is not sufficient to uniquely identify a field.  In the test below
+// there are 3 anonymous structs and all 3 have the following fields:
+//   MacroTest2::(anonymous struct)::tqe_next
+//   MacroTest2::(anonymous struct)::tqe_prev
 
 struct event;
 struct event_base;

tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-expected.txt

+==== BEGIN EDITS ====
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::941:::11:::CheckedPtr<SomeClass> 
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-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-overlapping-actual.cc:::941:::28:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::1095:::11:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::1140:::27:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::1385:::54:::CheckedPtr<struct NonFreeStandingStruct> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::1780:::11:::CheckedPtr<SomeClass> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-actual.cc:::1744:::64:::CheckedPtr<struct NonFreeStandingStruct2> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+my_namespace::MyStruct::overlapping_1a  # overlapping
+my_namespace::MyStruct::overlapping_1b  # overlapping
+my_namespace::MyStruct::overlapping_2a  # overlapping
+my_namespace::MyStruct::overlapping_3b  # overlapping
+my_namespace::MyStruct::ptr_to_non_free_standing_struct  # overlapping
+my_namespace::MyStruct::ptr_to_non_free_standing_struct2  # overlapping
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/gen-overlapping-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-overlapping-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
+
+namespace my_namespace {
+
+class SomeClass {
+  int x;
+};
+
+struct MyStruct {
+  // The fields below have an overlapping |replacement_range| and therefore
+  // should be emitted as candidates for --field-filter-file.
+  SomeClass *overlapping_1a, *overlapping_1b;
+
+  // It is sufficient to emit pointer fields (e.g. no need to emit
+  // overlapping_2b or overlapping_3a).
+  SomeClass *overlapping_2a, overlapping_2b;
+  SomeClass overlapping_3a, *overlapping_3b;
+
+  // Definition of the struct overlaps with the |replacement_range| of the
+  // |ptr_to_non_free_standing_struct|.  Therefore the field should be emitted
+  // as a candidate for --field-filter-file.
+  struct NonFreeStandingStruct {
+    int non_ptr;
+  } * ptr_to_non_free_standing_struct;
+
+  // Similarly to the above, definition of the struct overlaps with the
+  // |replacement_range| of |ptr_to_non_free_standing_struct2|.  OTOH, it is
+  // okay to proceed with rewriting |inner_ptr| - it should not be emitted as a
+  // candidate for --field-filter-file.
+  struct NonFreeStandingStruct2 {
+    SomeClass* inner_ptr;
+  } * ptr_to_non_free_standing_struct2;
+};
+
+}  // namespace my_namespace

tools/clang/rewrite_raw_ptr_fields/tests/macros-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.
-
-//////////////////////////////////////////////////////////////////////////////
-// Based on build/linux/debian_sid_amd64-sysroot/usr/include/link.h
-
-struct Elf64_Dyn;
-
-#define __ELF_NATIVE_CLASS 64
-#define ElfW(type) _ElfW(Elf, __ELF_NATIVE_CLASS, type)
-#define _ElfW(e, w, t) _ElfW_1(e, w, _##t)
-#define _ElfW_1(e, w, t) e##w##t
-
-struct MacroTest1 {
-  ElfW(Dyn) * ptr_field;
-};
-
-//////////////////////////////////////////////////////////////////////////////
-// Based on base/third_party/libevent/event.h
-
-struct event;
-struct event_base;
-
-#define TAILQ_ENTRY(type)                                          \
-  struct {                                                         \
-    struct type* tqe_next;  /* next element */                     \
-    struct type** tqe_prev; /* address of previous next element */ \
-  }
-
-struct MacroTest2 {
-  TAILQ_ENTRY(event) ev_next;
-  TAILQ_ENTRY(event) ev_active_next;
-  TAILQ_ENTRY(event) ev_signal_next;
-};

tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py

@@ -3,6 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import glob
 import os.path
 import shutil
 import subprocess
@@ -30,6 +31,12 @@ def RunGeneratingTest(test_path):
     os.remove(tmp_test_path)
 
 
+def RunGeneratingTests():
+  tests = glob.glob("tools/clang/rewrite_raw_ptr_fields/tests/gen-*-test.cc")
+  for test_path in tests:
+    RunGeneratingTest(test_path)
+
+
 def main():
   if not os.path.exists("ENG_REVIEW_OWNERS"):
     sys.stderr.write(
@@ -42,8 +49,7 @@ def main():
     return -1
 
   RunRewritingTests()
-  RunGeneratingTest(
-      "tools/clang/rewrite_raw_ptr_fields/tests/in-out-arg-test.cc")
+  RunGeneratingTests()
 
 
 if __name__ == "__main__":

tools/clang/rewrite_raw_ptr_fields/tests/various-types-expected.cc

@@ -117,20 +117,15 @@ struct MyStruct {
   // No rewrite expected (for now - in V1 we only rewrite field decls).
   using SomeClassPtrAlias = SomeClass*;
 
-  // Chromium is built with a warning/error that there are no user-defined
-  // constructors invoked when initializing global-scoped values.
-  // CheckedPtr<char> conversion might trigger a global constructor for string
-  // literals:
-  //     struct MyStruct {
-  //       int foo;
-  //       CheckedPtr<const char> bar;
-  //     }
-  //     MyStruct g_foo = {123, "string literal" /* global constr! */};
-  // Because of the above, no rewrite is expected below.
-  char* char_ptr;
-  const char* const_char_ptr;
-  wchar_t* wide_char_ptr;
-  const wchar_t* const_wide_char_ptr;
+  // Char pointer fields should be rewritten, unless they are on the
+  // --field-filter-file blocklist.  See also gen-char-test.cc for tests
+  // covering generating the blocklist.
+  //
+  // Expected rewrite: CheckedPtr<char>, etc.
+  CheckedPtr<char> char_ptr;
+  CheckedPtr<const char> const_char_ptr;
+  CheckedPtr<wchar_t> wide_char_ptr;
+  CheckedPtr<const wchar_t> const_wide_char_ptr;
 
   // |array_of_ptrs| is an array 123 of pointer to SomeClass.
   // No rewrite expected (this is not a pointer - this is an array).
@@ -145,44 +140,6 @@ struct MyStruct {
   // replacement is tricky, because the |replacement_range| needs to cover
   // "[123]" that comes *after* the field name).
   const SomeClass (*ptr_to_array)[123];
-
-  // Definition of the non-freestanding struct should not disappear - i.e.
-  // we do not want the rewrite to be: CheckedPtr<struct NonFreestandingStruct>.
-  //
-  // Expected rewrite: ??? (as long as the struct definition doesn't disappear).
-  struct NonFreeStandingStruct {
-    int non_ptr;
-  } * ptr_to_non_free_standing_struct;
-
-  // Pointer to an inline definition of a struct.  There is a risk of generating
-  // an overlapping replacement (wrt the pointer field within the inline
-  // struct).
-  //
-  // Note that before a fix, the rewriter would generate an overlapping
-  // replacement under
-  // //sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
-  // (see the ArgValue struct and the non-free-standing Tests struct inside).
-  //
-  // Expected rewrite: ??? (as long as there are no overlapping replacements).
-  struct NonFreeStandingStruct2 {
-    CheckedPtr<SomeClass> inner_ptr;
-  } * ptr_to_non_free_standing_struct2;
-
-  // The following is an unnamed/anonymous, free-standing struct.
-  //
-  // Expected rewrite: ??? (as long as there are no overlapping replacements).
-  struct {
-    CheckedPtr<SomeClass> inner_ptr;
-  } * ptr_to_non_free_standing_struct3;
-
-  // Despite avoiding the problems in NonFreeStandingStruct and
-  // NonFreeStandingStruct2 above, we should still rewrite the example below.
-  struct FreeStandingStruct {
-    // Expected rewrite: CheckedPtr<SomeClass> inner_ptr;
-    CheckedPtr<SomeClass> inner_ptr;
-  };
-  // Expected rewrite: CheckedPtr<InnerStruct2> ...
-  CheckedPtr<FreeStandingStruct> ptr_to_free_standing_struct;
 };
 
 extern "C" {

tools/clang/rewrite_raw_ptr_fields/tests/various-types-original.cc

@@ -115,16 +115,11 @@ struct MyStruct {
   // No rewrite expected (for now - in V1 we only rewrite field decls).
   using SomeClassPtrAlias = SomeClass*;
 
-  // Chromium is built with a warning/error that there are no user-defined
-  // constructors invoked when initializing global-scoped values.
-  // CheckedPtr<char> conversion might trigger a global constructor for string
-  // literals:
-  //     struct MyStruct {
-  //       int foo;
-  //       CheckedPtr<const char> bar;
-  //     }
-  //     MyStruct g_foo = {123, "string literal" /* global constr! */};
-  // Because of the above, no rewrite is expected below.
+  // Char pointer fields should be rewritten, unless they are on the
+  // --field-filter-file blocklist.  See also gen-char-test.cc for tests
+  // covering generating the blocklist.
+  //
+  // Expected rewrite: CheckedPtr<char>, etc.
   char* char_ptr;
   const char* const_char_ptr;
   wchar_t* wide_char_ptr;
@@ -143,44 +138,6 @@ struct MyStruct {
   // replacement is tricky, because the |replacement_range| needs to cover
   // "[123]" that comes *after* the field name).
   const SomeClass (*ptr_to_array)[123];
-
-  // Definition of the non-freestanding struct should not disappear - i.e.
-  // we do not want the rewrite to be: CheckedPtr<struct NonFreestandingStruct>.
-  //
-  // Expected rewrite: ??? (as long as the struct definition doesn't disappear).
-  struct NonFreeStandingStruct {
-    int non_ptr;
-  } * ptr_to_non_free_standing_struct;
-
-  // Pointer to an inline definition of a struct.  There is a risk of generating
-  // an overlapping replacement (wrt the pointer field within the inline
-  // struct).
-  //
-  // Note that before a fix, the rewriter would generate an overlapping
-  // replacement under
-  // //sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
-  // (see the ArgValue struct and the non-free-standing Tests struct inside).
-  //
-  // Expected rewrite: ??? (as long as there are no overlapping replacements).
-  struct NonFreeStandingStruct2 {
-    SomeClass* inner_ptr;
-  } * ptr_to_non_free_standing_struct2;
-
-  // The following is an unnamed/anonymous, free-standing struct.
-  //
-  // Expected rewrite: ??? (as long as there are no overlapping replacements).
-  struct {
-    SomeClass* inner_ptr;
-  } * ptr_to_non_free_standing_struct3;
-
-  // Despite avoiding the problems in NonFreeStandingStruct and
-  // NonFreeStandingStruct2 above, we should still rewrite the example below.
-  struct FreeStandingStruct {
-    // Expected rewrite: CheckedPtr<SomeClass> inner_ptr;
-    SomeClass* inner_ptr;
-  };
-  // Expected rewrite: CheckedPtr<InnerStruct2> ...
-  FreeStandingStruct* ptr_to_free_standing_struct;
 };
 
 extern "C" {