Commit 28d01461 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Avoid rewriting C-only or |extern "C"| field decls.

CheckedPtr requires C++11 - including checked_ptr.h will cause
compilation errors when compiling C-only sources.  This CL suppresses
rewrites if the clang::CompilerInstance indicates that it is compiling
a C-only source file.

Additionally, the CL excludes from the rewrite fields declared under a
|extern "C"| declaration context - such context is a strong hint that
1) the struct might need to be used from a C-only source and
2) the struct needs strict ABI compatibility (not guaranteed by the
   CheckedPtr which might repurpose some bits of the pointer).

Bug: 1069567
Change-Id: Ib146acbf1f5ae075c2498c000104cebe9cc36b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2181527Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarBartek Nowierski <bartekn@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774754}
parent 54c73a61
......@@ -53,15 +53,17 @@ const char kIncludePath[] = "base/memory/checked_ptr.h";
const char kExcludeFieldsParamName[] = "exclude-fields";
// Output format is documented in //docs/clang_tool_refactoring.md
class ReplacementsPrinter {
class ReplacementsPrinter : public clang::tooling::SourceFileCallbacks {
public:
ReplacementsPrinter() { llvm::outs() << "==== BEGIN EDITS ====\n"; }
~ReplacementsPrinter() { llvm::outs() << "==== END EDITS ====\n"; }
ReplacementsPrinter() = default;
~ReplacementsPrinter() = default;
void PrintReplacement(const clang::SourceManager& source_manager,
const clang::SourceRange& replacement_range,
std::string replacement_text) {
if (ShouldSuppressOutput())
return;
clang::tooling::Replacement replacement(
source_manager, clang::CharSourceRange::getCharRange(replacement_range),
replacement_text);
......@@ -82,7 +84,60 @@ class ReplacementsPrinter {
}
private:
// clang::tooling::SourceFileCallbacks override:
bool handleBeginSource(clang::CompilerInstance& compiler) override {
const clang::FrontendOptions& frontend_options = compiler.getFrontendOpts();
assert((frontend_options.Inputs.size() == 1) &&
"run_tool.py should invoke the rewriter one file at a time");
const clang::FrontendInputFile& input_file = frontend_options.Inputs[0];
assert(input_file.isFile() &&
"run_tool.py should invoke the rewriter on actual files");
current_language_ = input_file.getKind().getLanguage();
if (!ShouldSuppressOutput())
llvm::outs() << "==== BEGIN EDITS ====\n";
return true; // Report that |handleBeginSource| succeeded.
}
// clang::tooling::SourceFileCallbacks override:
void handleEndSource() override {
if (!ShouldSuppressOutput())
llvm::outs() << "==== END EDITS ====\n";
}
bool ShouldSuppressOutput() {
switch (current_language_) {
case clang::Language::Unknown:
case clang::Language::Asm:
case clang::Language::LLVM_IR:
case clang::Language::OpenCL:
case clang::Language::CUDA:
case clang::Language::RenderScript:
case clang::Language::HIP:
// Rewriter can't handle rewriting the current input language.
return true;
case clang::Language::C:
case clang::Language::ObjC:
// CheckedPtr requires C++. In particular, attempting to #include
// "base/memory/checked_ptr.h" from C-only compilation units will lead
// to compilation errors.
return true;
case clang::Language::CXX:
case clang::Language::ObjCXX:
return false;
}
assert(false && "Unrecognized clang::Language");
return true;
}
std::set<std::string> files_with_already_added_includes_;
clang::Language current_language_ = clang::Language::Unknown;
};
AST_MATCHER(clang::TagDecl, isNotFreeStandingTagDecl) {
......@@ -186,6 +241,10 @@ AST_MATCHER_P(clang::FieldDecl,
return Filter.Contains(Node);
}
AST_MATCHER(clang::Decl, isInExternCContext) {
return Node.getLexicalDeclContext()->isExternCContext();
}
AST_MATCHER(clang::ClassTemplateSpecializationDecl, isImplicitSpecialization) {
return !Node.isExplicitSpecialization();
}
......@@ -375,18 +434,20 @@ 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(),
isListedInFilterFile(fields_to_exclude),
implicit_field_decl_matcher))))
fieldDecl(allOf(hasType(supported_pointer_types_matcher),
hasUniqueTypeLoc(),
unless(anyOf(isInThirdPartyLocation(),
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);
// Prepare and run the tool.
std::unique_ptr<clang::tooling::FrontendActionFactory> factory =
clang::tooling::newFrontendActionFactory(&match_finder);
clang::tooling::newFrontendActionFactory(&match_finder,
&replacements_printer);
int result = tool.run(factory.get());
if (result != 0)
return result;
......
......@@ -162,4 +162,12 @@ struct MyStruct {
CheckedPtr<FreeStandingStruct> ptr_to_free_standing_struct;
};
extern "C" {
struct OtherForeignStruct;
struct ForeignStruct {
// We should not rewrite foreign, extern "C" structs.
OtherForeignStruct* ptr;
};
}
} // namespace my_namespace
......@@ -160,4 +160,12 @@ struct MyStruct {
FreeStandingStruct* ptr_to_free_standing_struct;
};
extern "C" {
struct OtherForeignStruct;
struct ForeignStruct {
// We should not rewrite foreign, extern "C" structs.
OtherForeignStruct* ptr;
};
}
} // namespace my_namespace
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