Commit 1772227e authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Fields in system headers (and generated code) are not really affected.

Fields in system headers (and generated) code were not rewritten even
before this CL, because |apply_edits.py| ignores edits that apply to
files that |git| doesn't know anything about.  But, before this CL, such
fields would be considered "affected" and therefore Chromium code might
see |.get()| appended when using such fields in |printf|,
|reinterpret_cast|, etc.  Such inconsistency would lead to build errors
after the rewrite.

After this CL, fields in system headers (and generated code) are
explicitly excluded (early on, in the |field_decl_matcher|).

Bug: 1069567
Change-Id: Icc9735b94558828c002572b38dcef12a812de35c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204305
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775250}
parent 0588fdae
......@@ -183,6 +183,13 @@ AST_MATCHER(clang::FieldDecl, isInThirdPartyLocation) {
return file_path.contains("third_party");
}
AST_MATCHER(clang::FieldDecl, isInGeneratedLocation) {
llvm::StringRef file_path =
GetFilePath(Finder->getASTContext().getSourceManager(), Node);
return file_path.startswith("gen/") || file_path.contains("/gen/");
}
class FieldDeclFilterFile {
public:
explicit FieldDeclFilterFile(const std::string& filepath) {
......@@ -470,12 +477,13 @@ int main(int argc, const char* argv[]) {
// the source code)
FieldDeclFilterFile fields_to_exclude(exclude_fields_param);
auto field_decl_matcher =
fieldDecl(allOf(hasType(supported_pointer_types_matcher),
hasUniqueTypeLoc(),
unless(anyOf(isInThirdPartyLocation(),
isInMacroLocation(), isInExternCContext(),
isListedInFilterFile(fields_to_exclude),
implicit_field_decl_matcher))))
fieldDecl(
allOf(hasType(supported_pointer_types_matcher), hasUniqueTypeLoc(),
unless(anyOf(isInThirdPartyLocation(), isInGeneratedLocation(),
isExpansionInSystemHeader(), isInMacroLocation(),
isInExternCContext(),
isListedInFilterFile(fields_to_exclude),
implicit_field_decl_matcher))))
.bind("fieldDecl");
FieldDeclRewriter field_decl_rewriter(&replacements_printer);
match_finder.addMatcher(field_decl_matcher, &field_decl_rewriter);
......
......@@ -5,6 +5,7 @@
#include <stdint.h> // for uintptr_t
#include "base/memory/checked_ptr.h"
#include "gen/generated_header.h"
class SomeClass {};
class DerivedClass : public SomeClass {};
......@@ -168,3 +169,17 @@ void foo(int x) {
}
} // namespace ternary_operator_tests
namespace generated_code_tests {
void MyPrintf(const char* fmt, ...) {}
void foo() {
GeneratedStruct s;
// No rewrite expected below (i.e. no |.get()| appended), because the field
// dereferenced below comes from (simulated) generated code.
MyPrintf("%p", s.ptr_field);
}
} // namespace generated_code_tests
// 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 <stdint.h>
// WARNING: This test uses a platform-specific system header. This means that
// the test might (expectedly) fail on platforms that don't provide such header.
#include <dlfcn.h>
void foo(const Dl_info& dl_info) {
// Fields in non-Chromium locations are not affected (e.g. |apply_edits.py|
// filters out edit directives that apply to files that |git| doesn't know
// about). For example, |Dl_info::dli_fbase| won't be rewritten and therefore
// shouldn't be treated as "affected" in the expression below.
//
// No rewrite expected below - the expression below shouldn't turn into
// |dl_info.dli_fbase.get()|.
uintptr_t v = reinterpret_cast<uintptr_t>(dl_info.dli_fbase);
}
// 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 <stdint.h>
// WARNING: This test uses a platform-specific system header. This means that
// the test might (expectedly) fail on platforms that don't provide such header.
#include <dlfcn.h>
void foo(const Dl_info& dl_info) {
// Fields in non-Chromium locations are not affected (e.g. |apply_edits.py|
// filters out edit directives that apply to files that |git| doesn't know
// about). For example, |Dl_info::dli_fbase| won't be rewritten and therefore
// shouldn't be treated as "affected" in the expression below.
//
// No rewrite expected below - the expression below shouldn't turn into
// |dl_info.dli_fbase.get()|.
uintptr_t v = reinterpret_cast<uintptr_t>(dl_info.dli_fbase);
}
......@@ -4,6 +4,8 @@
#include <stdint.h> // for uintptr_t
#include "gen/generated_header.h"
class SomeClass {};
class DerivedClass : public SomeClass {};
......@@ -166,3 +168,17 @@ void foo(int x) {
}
} // namespace ternary_operator_tests
namespace generated_code_tests {
void MyPrintf(const char* fmt, ...) {}
void foo() {
GeneratedStruct s;
// No rewrite expected below (i.e. no |.get()| appended), because the field
// dereferenced below comes from (simulated) generated code.
MyPrintf("%p", s.ptr_field);
}
} // namespace generated_code_tests
// 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.
#ifndef TOOLS_CLANG_REWRITE_RAW_PTR_FIELDS_TESTS_GEN_GENERATED_HEADER_H_
#define TOOLS_CLANG_REWRITE_RAW_PTR_FIELDS_TESTS_GEN_GENERATED_HEADER_H_
class SomeClass;
// This file simulates a header that was generated during build. This is
// simulated by virtue of being located inside a directory named "gen".
struct GeneratedStruct {
// No rewrite expected inside generated code.
int* ptr_field;
SomeClass* ptr_field2;
};
#endif // TOOLS_CLANG_REWRITE_RAW_PTR_FIELDS_TESTS_GEN_GENERATED_HEADER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment