Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Unified Diff: tools/clang/plugins/ChromeClassTester.cpp

Issue 10005022: Check for public dtors on base::RefCounted types (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Copyright update Created 8 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.h ('k') | tools/clang/plugins/FindBadConstructs.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/clang/plugins/ChromeClassTester.cpp
diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index dd983bd81410cb1460ec27747ccfa0a7fc2836a7..7a3b89ebc91c84d0093aa4317464609d9194b248 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -40,72 +40,6 @@ ChromeClassTester::ChromeClassTester(CompilerInstance& instance)
BuildBannedLists();
}
-void ChromeClassTester::BuildBannedLists() {
- banned_namespaces_.push_back("std");
- banned_namespaces_.push_back("__gnu_cxx");
- banned_namespaces_.push_back("WebKit");
-
- banned_directories_.push_back("third_party/");
- banned_directories_.push_back("native_client/");
- banned_directories_.push_back("breakpad/");
- banned_directories_.push_back("courgette/");
- banned_directories_.push_back("pdf/");
- banned_directories_.push_back("ppapi/");
- banned_directories_.push_back("usr/");
- banned_directories_.push_back("testing/");
- banned_directories_.push_back("googleurl/");
- banned_directories_.push_back("v8/");
- banned_directories_.push_back("dart/");
- banned_directories_.push_back("sdch/");
- banned_directories_.push_back("icu4c/");
- banned_directories_.push_back("frameworks/");
-
- // Don't check autogenerated headers.
- // Make puts them below $(builddir_name)/.../gen and geni.
- // Ninja puts them below OUTPUT_DIR/.../gen
- // Xcode has a fixed output directory for everything.
- banned_directories_.push_back("gen/");
- banned_directories_.push_back("geni/");
- banned_directories_.push_back("xcodebuild/");
-
- // You are standing in a mazy of twisty dependencies, all resolved by
- // putting everything in the header.
- banned_directories_.push_back("automation/");
-
- // Don't check system headers.
- banned_directories_.push_back("/Developer/");
-
- // Used in really low level threading code that probably shouldn't be out of
- // lined.
- ignored_record_names_.insert("ThreadLocalBoolean");
-
- // A complicated pickle derived struct that is all packed integers.
- ignored_record_names_.insert("Header");
-
- // Part of the GPU system that uses multiple included header
- // weirdness. Never getting this right.
- ignored_record_names_.insert("Validators");
-
- // Has a UNIT_TEST only constructor. Isn't *terribly* complex...
- ignored_record_names_.insert("AutocompleteController");
- ignored_record_names_.insert("HistoryURLProvider");
-
- // Because of chrome frame
- ignored_record_names_.insert("ReliabilityTestSuite");
-
- // Used over in the net unittests. A large enough bundle of integers with 1
- // non-pod class member. Probably harmless.
- ignored_record_names_.insert("MockTransaction");
-
- // Used heavily in ui_unittests and once in views_unittests. Fixing this
- // isn't worth the overhead of an additional library.
- ignored_record_names_.insert("TestAnimationDelegate");
-
- // Part of our public interface that nacl and friends use. (Arguably, this
- // should mean that this is a higher priority but fixing this looks hard.)
- ignored_record_names_.insert("PluginVersionInfo");
-}
-
ChromeClassTester::~ChromeClassTester() {}
void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
@@ -146,7 +80,8 @@ void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
}
}
-void ChromeClassTester::emitWarning(SourceLocation loc, const char* raw_error) {
+void ChromeClassTester::emitWarning(SourceLocation loc,
+ const char* raw_error) {
FullSourceLoc full(loc, instance().getSourceManager());
std::string err;
err = "[chromium-style] ";
@@ -159,10 +94,6 @@ void ChromeClassTester::emitWarning(SourceLocation loc, const char* raw_error) {
DiagnosticBuilder B = diagnostic().Report(full, id);
}
-bool ChromeClassTester::InTestingNamespace(const Decl* record) {
- return GetNamespace(record).find("testing") != std::string::npos;
-}
-
bool ChromeClassTester::InBannedNamespace(const Decl* record) {
std::string n = GetNamespace(record);
if (n != "") {
@@ -177,8 +108,88 @@ std::string ChromeClassTester::GetNamespace(const Decl* record) {
return GetNamespaceImpl(record->getDeclContext(), "");
}
+bool ChromeClassTester::InImplementationFile(SourceLocation record_location) {
+ std::string filename;
+ if (!GetFilename(record_location, &filename)) {
+ return false;
+ }
+
+ if (ends_with(filename, ".cc") || ends_with(filename, ".cpp") ||
+ ends_with(filename, ".mm")) {
+ return true;
+ }
+
+ return false;
+}
+
+void ChromeClassTester::BuildBannedLists() {
+ banned_namespaces_.push_back("std");
+ banned_namespaces_.push_back("__gnu_cxx");
+ banned_namespaces_.push_back("WebKit");
+
+ banned_directories_.push_back("third_party/");
+ banned_directories_.push_back("native_client/");
+ banned_directories_.push_back("breakpad/");
+ banned_directories_.push_back("courgette/");
+ banned_directories_.push_back("pdf/");
+ banned_directories_.push_back("ppapi/");
+ banned_directories_.push_back("usr/");
+ banned_directories_.push_back("testing/");
+ banned_directories_.push_back("googleurl/");
+ banned_directories_.push_back("v8/");
+ banned_directories_.push_back("dart/");
+ banned_directories_.push_back("sdch/");
+ banned_directories_.push_back("icu4c/");
+ banned_directories_.push_back("frameworks/");
+
+ // Don't check autogenerated headers.
+ // Make puts them below $(builddir_name)/.../gen and geni.
+ // Ninja puts them below OUTPUT_DIR/.../gen
+ // Xcode has a fixed output directory for everything.
+ banned_directories_.push_back("gen/");
+ banned_directories_.push_back("geni/");
+ banned_directories_.push_back("xcodebuild/");
+
+ // You are standing in a mazy of twisty dependencies, all resolved by
+ // putting everything in the header.
+ banned_directories_.push_back("automation/");
+
+ // Don't check system headers.
+ banned_directories_.push_back("/Developer/");
+
+ // Used in really low level threading code that probably shouldn't be out of
+ // lined.
+ ignored_record_names_.insert("ThreadLocalBoolean");
+
+ // A complicated pickle derived struct that is all packed integers.
+ ignored_record_names_.insert("Header");
+
+ // Part of the GPU system that uses multiple included header
+ // weirdness. Never getting this right.
+ ignored_record_names_.insert("Validators");
+
+ // Has a UNIT_TEST only constructor. Isn't *terribly* complex...
+ ignored_record_names_.insert("AutocompleteController");
+ ignored_record_names_.insert("HistoryURLProvider");
+
+ // Because of chrome frame
+ ignored_record_names_.insert("ReliabilityTestSuite");
+
+ // Used over in the net unittests. A large enough bundle of integers with 1
+ // non-pod class member. Probably harmless.
+ ignored_record_names_.insert("MockTransaction");
+
+ // Used heavily in ui_unittests and once in views_unittests. Fixing this
+ // isn't worth the overhead of an additional library.
+ ignored_record_names_.insert("TestAnimationDelegate");
+
+ // Part of our public interface that nacl and friends use. (Arguably, this
+ // should mean that this is a higher priority but fixing this looks hard.)
+ ignored_record_names_.insert("PluginVersionInfo");
+}
+
std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
- std::string candidate) {
+ const std::string& candidate) {
switch (context->getDeclKind()) {
case Decl::TranslationUnit: {
return candidate;
@@ -201,60 +212,49 @@ std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
}
bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
- const SourceManager &SM = instance_.getSourceManager();
- SourceLocation spelling_location = SM.getSpellingLoc(loc);
- PresumedLoc ploc = SM.getPresumedLoc(spelling_location);
- std::string buffer_name;
- if (ploc.isInvalid()) {
- // If we're in an invalid location, we're looking at things that aren't
- // actually stated in the source; treat this as a banned location instead
- // of going through our full lookup process.
+ std::string filename;
+ if (!GetFilename(loc, &filename)) {
+ // If the filename cannot be determined, simply treat this as a banned
+ // location, instead of going through the full lookup process.
return true;
- } else {
- std::string b = ploc.getFilename();
-
- // We need to special case scratch space; which is where clang does its
- // macro expansion. We explicitly want to allow people to do otherwise bad
- // things through macros that were defined due to third party libraries.
- if (b == "<scratch space>")
- return true;
-
- // Don't complain about these things in implementation files.
- if (ends_with(b, ".cc") || ends_with(b, ".cpp") || ends_with(b, ".mm")) {
- return true;
- }
+ }
- // Don't complain about autogenerated protobuf files.
- if (ends_with(b, ".pb.h")) {
- return true;
- }
+ // We need to special case scratch space; which is where clang does its
+ // macro expansion. We explicitly want to allow people to do otherwise bad
+ // things through macros that were defined due to third party libraries.
+ if (filename == "<scratch space>")
+ return true;
- // We need to munge the paths so that they are relative to the repository
- // srcroot. We first resolve the symlinktastic relative path and then
- // remove our known srcroot from it if needed.
- char resolvedPath[MAXPATHLEN];
- if (realpath(b.c_str(), resolvedPath)) {
- b = resolvedPath;
- }
+ // Don't complain about autogenerated protobuf files.
+ if (ends_with(filename, ".pb.h")) {
+ return true;
+ }
+
+ // We need to munge the paths so that they are relative to the repository
+ // srcroot. We first resolve the symlinktastic relative path and then
+ // remove our known srcroot from it if needed.
+ char resolvedPath[MAXPATHLEN];
+ if (realpath(filename.c_str(), resolvedPath)) {
+ filename = resolvedPath;
+ }
- // On linux, chrome is often checked out to /usr/local/google. Due to the
- // "usr" rule in banned_directories_, all diagnostics would be suppressed
- // in that case. As a workaround, strip that prefix.
- b = lstrip(b, "/usr/local/google");
-
- for (std::vector<std::string>::const_iterator it =
- banned_directories_.begin();
- it != banned_directories_.end(); ++it) {
- // If we can find any of the banned path components in this path, then
- // this file is rejected.
- size_t index = b.find(*it);
- if (index != std::string::npos) {
- bool matches_full_dir_name = index == 0 || b[index - 1] == '/';
- if ((*it)[0] == '/')
- matches_full_dir_name = true;
- if (matches_full_dir_name)
- return true;
- }
+ // On linux, chrome is often checked out to /usr/local/google. Due to the
+ // "usr" rule in banned_directories_, all diagnostics would be suppressed
+ // in that case. As a workaround, strip that prefix.
+ filename = lstrip(filename, "/usr/local/google");
+
+ for (std::vector<std::string>::const_iterator it =
+ banned_directories_.begin();
+ it != banned_directories_.end(); ++it) {
+ // If we can find any of the banned path components in this path, then
+ // this file is rejected.
+ size_t index = filename.find(*it);
+ if (index != std::string::npos) {
+ bool matches_full_dir_name = index == 0 || filename[index - 1] == '/';
+ if ((*it)[0] == '/')
+ matches_full_dir_name = true;
+ if (matches_full_dir_name)
+ return true;
}
}
@@ -264,3 +264,18 @@ bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
bool ChromeClassTester::IsIgnoredType(const std::string& base_name) {
return ignored_record_names_.find(base_name) != ignored_record_names_.end();
}
+
+bool ChromeClassTester::GetFilename(SourceLocation loc,
+ std::string* filename) {
+ const SourceManager &SM = instance_.getSourceManager();
+ SourceLocation spelling_location = SM.getSpellingLoc(loc);
+ PresumedLoc ploc = SM.getPresumedLoc(spelling_location);
+ if (ploc.isInvalid()) {
+ // If we're in an invalid location, we're looking at things that aren't
+ // actually stated in the source.
+ return false;
+ }
+
+ *filename = ploc.getFilename();
+ return true;
+}
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.h ('k') | tools/clang/plugins/FindBadConstructs.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698