Skip to content

Commit

Permalink
Revert "Reland: Add check-bad-raw-ptr-cast to the clang plugin"
Browse files Browse the repository at this point in the history
This reverts commit f8a3c56.

Reason for revert: Breaks clang plugins build on the build bots. See crbug.com/1381529

Original change's description:
> Reland: Add check-bad-raw-ptr-cast to the clang plugin
>
> change in reland: fix build error
>
> Original description:
> check-bad-raw-ptr-cast checks for raw_ptr<T>* being cast to another type.
>
> This prevents issues like this: https://chromium-review.googlesource.com/c/chromium/src/+/2587397/9/base/memory/checked_ptr.md#230
>
> As discussed in https://chat.google.com/room/AAAAdj-LwK0/QOr9OiIVv5M
>
> Change-Id: I2da17fc4dbcb080f334871dc85be6d581e867ee5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961908
> Commit-Queue: Keishi Hattori <keishi@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1066236}

Change-Id: I8d6129b2ff7c31d1fb2048bac5a1bc0836389a3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4006577
Commit-Queue: Amy Huang <akhuang@google.com>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067631}
  • Loading branch information
amykhuang authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent 16e6d7a commit bbf47b5
Show file tree
Hide file tree
Showing 15 changed files with 23 additions and 208 deletions.
5 changes: 2 additions & 3 deletions tools/clang/plugins/BlinkDataMemberTypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ BlinkDataMemberTypeChecker::BlinkDataMemberTypeChecker(

void BlinkDataMemberTypeChecker::CheckClass(SourceLocation location,
const CXXRecordDecl* record) {
std::string filename = GetFilename(instance_.getSourceManager(), location);
std::string filename = GetFilename(instance_, location);
if (!included_filenames_regex_.match(filename))
return;
if (excluded_filenames_regex_.match(filename))
Expand Down Expand Up @@ -147,8 +147,7 @@ void BlinkDataMemberTypeChecker::CheckField(const FieldDecl* field) {
// Similarly, stop finding the root underlying type if the intermediate
// type is defined in a file that should not be checked, e.g. in a file
// under third_party/blink/public/common.
std::string filename =
GetFilename(instance_.getSourceManager(), decl->getLocation());
std::string filename = GetFilename(instance_, decl->getLocation());
if (!included_filenames_regex_.match(filename))
return;
}
Expand Down
1 change: 0 additions & 1 deletion tools/clang/plugins/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ set(plugin_sources
ChromeClassTester.cpp
FindBadConstructsAction.cpp
FindBadConstructsConsumer.cpp
FindBadRawPtrPatterns.cpp
CheckIPCVisitor.cpp
CheckLayoutObjectMethodsVisitor.cpp
Util.cpp
Expand Down
4 changes: 2 additions & 2 deletions tools/clang/plugins/ChromeClassTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ ChromeClassTester::LocationType ChromeClassTester::ClassifyLocation(
if (instance().getSourceManager().isInSystemHeader(loc))
return LocationType::kThirdParty;

std::string filename = GetFilename(instance().getSourceManager(), loc);
std::string filename = GetFilename(instance(), loc);
if (filename.empty()) {
// If the filename cannot be determined, simply treat this as a banned
// location, instead of going through the full lookup process.
Expand Down Expand Up @@ -139,7 +139,7 @@ bool ChromeClassTester::InImplementationFile(SourceLocation record_location) {
// If |record_location| is a macro, check the whole chain of expansions.
const SourceManager& source_manager = instance_.getSourceManager();
while (true) {
filename = GetFilename(instance().getSourceManager(), record_location);
filename = GetFilename(instance(), record_location);
if (ends_with(filename, ".cc") || ends_with(filename, ".cpp") ||
ends_with(filename, ".mm")) {
return true;
Expand Down
2 changes: 0 additions & 2 deletions tools/clang/plugins/FindBadConstructsAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
options_.check_layout_object_methods = true;
} else if (args[i] == "raw-ref-template-as-trivial-member") {
options_.raw_ref_template_as_trivial_member = true;
} else if (args[i] == "check-bad-raw-ptr-cast") {
options_.check_bad_raw_ptr_cast = true;
} else {
parsed = false;
llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
Expand Down
2 changes: 0 additions & 2 deletions tools/clang/plugins/FindBadConstructsConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "FindBadConstructsConsumer.h"

#include "FindBadRawPtrPatterns.h"
#include "Util.h"
#include "clang/AST/Attr.h"
#include "clang/Frontend/CompilerInstance.h"
Expand Down Expand Up @@ -231,7 +230,6 @@ void FindBadConstructsConsumer::Traverse(ASTContext& context) {
RecursiveASTVisitor::TraverseDecl(context.getTranslationUnitDecl());
if (ipc_visitor_)
ipc_visitor_->set_context(nullptr);
FindBadRawPtrPatterns(options_, context, instance());
}

bool FindBadConstructsConsumer::TraverseDecl(Decl* decl) {
Expand Down
105 changes: 0 additions & 105 deletions tools/clang/plugins/FindBadRawPtrPatterns.cpp

This file was deleted.

20 changes: 0 additions & 20 deletions tools/clang/plugins/FindBadRawPtrPatterns.h

This file was deleted.

1 change: 0 additions & 1 deletion tools/clang/plugins/Options.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ struct Options {
bool check_ipc = false;
bool check_layout_object_methods = false;
bool raw_ref_template_as_trivial_member = false;
bool check_bad_raw_ptr_cast = false;
};

} // namespace chrome_checker
Expand Down
3 changes: 2 additions & 1 deletion tools/clang/plugins/Util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ std::string GetNamespace(const clang::Decl* record) {
return GetNamespaceImpl(record->getDeclContext(), std::string());
}

std::string GetFilename(const clang::SourceManager& source_manager,
std::string GetFilename(clang::CompilerInstance& instance,
clang::SourceLocation location) {
const clang::SourceManager& source_manager = instance.getSourceManager();
clang::SourceLocation spelling_location =
source_manager.getSpellingLoc(location);
clang::PresumedLoc ploc = source_manager.getPresumedLoc(spelling_location);
Expand Down
4 changes: 2 additions & 2 deletions tools/clang/plugins/Util.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include "clang/AST/DeclBase.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/CompilerInstance.h"

// Utility method for subclasses to determine the namespace of the
// specified record, if any. Unnamed namespaces will be identified as
Expand All @@ -18,7 +18,7 @@ std::string GetNamespace(const clang::Decl* record);

// Attempts to determine the filename for the given SourceLocation.
// Returns an empty string if the filename could not be determined.
std::string GetFilename(const clang::SourceManager& instance,
std::string GetFilename(clang::CompilerInstance& instance,
clang::SourceLocation location);

#endif // TOOLS_CLANG_PLUGINS_UTIL_H_
17 changes: 0 additions & 17 deletions tools/clang/plugins/tests/bad_raw_ptr_cast.cpp

This file was deleted.

1 change: 0 additions & 1 deletion tools/clang/plugins/tests/bad_raw_ptr_cast.flags

This file was deleted.

18 changes: 0 additions & 18 deletions tools/clang/plugins/tests/bad_raw_ptr_cast.h

This file was deleted.

19 changes: 0 additions & 19 deletions tools/clang/plugins/tests/bad_raw_ptr_cast.txt

This file was deleted.

29 changes: 15 additions & 14 deletions ui/views/examples/table_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,21 @@ void TableExample::CreateExampleView(View* container) {
MaximumFlexSizeRule::kUnbounded)
.WithWeight(1);

const auto make_checkbox =
[](const std::u16string& label, int id, raw_ptr<TableView>* table,
raw_ptr<Checkbox>* checkbox, FlexSpecification full_flex) {
return Builder<Checkbox>()
.CopyAddressTo(checkbox)
.SetText(label)
.SetCallback(base::BindRepeating(
[](int id, TableView* table, Checkbox* checkbox) {
table->SetColumnVisibility(id, checkbox->GetChecked());
},
id, *table, *checkbox))
.SetChecked(true)
.SetProperty(kFlexBehaviorKey, full_flex);
};
const auto make_checkbox = [](const std::u16string& label, int id,
raw_ptr<TableView>* table,
raw_ptr<Checkbox>* checkbox,
FlexSpecification full_flex) {
return Builder<Checkbox>()
.CopyAddressTo(checkbox)
.SetText(label)
.SetCallback(base::BindRepeating(
[](int id, raw_ptr<TableView>* table, raw_ptr<Checkbox>* checkbox) {
(*table)->SetColumnVisibility(id, (*checkbox)->GetChecked());
},
id, table, checkbox))
.SetChecked(true)
.SetProperty(kFlexBehaviorKey, full_flex);
};

// Make table
Builder<View>(container)
Expand Down

0 comments on commit bbf47b5

Please sign in to comment.