summaryrefslogtreecommitdiffstats
path: root/tools
diff options
context:
space:
mode:
authorthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-26 20:46:51 +0000
committerthakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-26 20:46:51 +0000
commit1da30523f26bdecc2928fda29ec9f3d9c40fe17f (patch)
treed8ce009c54e6b9748b4a6c39c80d67d99afbbfda /tools
parentb887777f93c48f746d1b5ee98113d87096f4bd64 (diff)
downloadchromium_src-1da30523f26bdecc2928fda29ec9f3d9c40fe17f.zip
chromium_src-1da30523f26bdecc2928fda29ec9f3d9c40fe17f.tar.gz
chromium_src-1da30523f26bdecc2928fda29ec9f3d9c40fe17f.tar.bz2
Revert of Add a check to the FindBadConstructs.cpp clang plugin for bad enum last values. (https://codereview.chromium.org/150943005/)
Reason for revert: Caused crbug.com/356815 and crbug.com/356816 and doesn't have a flag to turn it off. Original issue's description: > Add a check to the FindBadConstructs.cpp clang plugin for bad enum last values. > > One common coding patern is to define an enum as in: > enum Color { RED, GREEN, BLUE, COLOR_LAST=BLUE }; > but this is fragile when someone adds a new constant and forgets > to update the COLOR_LAST constant. > > This change looks for enums that have a xxx_LAST or xxxLast member, > and warns if there are any higher-valued constants present. > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251894 TBR=erg@google.com,darin@chromium.org,erg@chromium.org,tsepez@chromium.org NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/212673008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259675 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'tools')
-rw-r--r--tools/clang/plugins/ChromeClassTester.cpp16
-rw-r--r--tools/clang/plugins/ChromeClassTester.h6
-rw-r--r--tools/clang/plugins/FindBadConstructs.cpp34
3 files changed, 0 insertions, 56 deletions
diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index 5ce04e5..e788c15 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -91,16 +91,6 @@ void ChromeClassTester::CheckTag(TagDecl* tag) {
return;
CheckChromeClass(record_location, record);
- } else if (EnumDecl* enum_decl = dyn_cast<EnumDecl>(tag)) {
- SourceLocation enum_location = enum_decl->getInnerLocStart();
- if (InBannedDirectory(enum_location))
- return;
-
- std::string base_name = enum_decl->getNameAsString();
- if (IsIgnoredType(base_name))
- return;
-
- CheckChromeEnum(enum_location, enum_decl);
}
}
@@ -205,9 +195,6 @@ void ChromeClassTester::BuildBannedLists() {
// non-pod class member. Probably harmless.
ignored_record_names_.insert("MockTransaction");
- // Enum type with _LAST members where _LAST doesn't mean last enum value.
- ignored_record_names_.insert("ServerFieldType");
-
// 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");
@@ -219,9 +206,6 @@ void ChromeClassTester::BuildBannedLists() {
// Measured performance improvement on cc_perftests. See
// https://codereview.chromium.org/11299290/
ignored_record_names_.insert("QuadF");
-
- // Enum type with _LAST members where _LAST doesn't mean last enum value.
- ignored_record_names_.insert("ViewID");
}
std::string ChromeClassTester::GetNamespaceImpl(const DeclContext* context,
diff --git a/tools/clang/plugins/ChromeClassTester.h b/tools/clang/plugins/ChromeClassTester.h
index 6bd19a6..588ae9c 100644
--- a/tools/clang/plugins/ChromeClassTester.h
+++ b/tools/clang/plugins/ChromeClassTester.h
@@ -54,12 +54,6 @@ class ChromeClassTester : public clang::ASTConsumer {
virtual void CheckChromeClass(clang::SourceLocation record_location,
clang::CXXRecordDecl* record) = 0;
- // Filtered versions of enum type that are only called with things defined
- // in chrome header files.
- virtual void CheckChromeEnum(clang::SourceLocation enum_location,
- clang::EnumDecl* enum_decl) {
- }
-
// Utility methods used for filtering out non-chrome classes (and ones we
// deliberately ignore) in HandleTagDeclDefinition().
std::string GetNamespaceImpl(const clang::DeclContext* context,
diff --git a/tools/clang/plugins/FindBadConstructs.cpp b/tools/clang/plugins/FindBadConstructs.cpp
index 1b37cd0..aa8e813 100644
--- a/tools/clang/plugins/FindBadConstructs.cpp
+++ b/tools/clang/plugins/FindBadConstructs.cpp
@@ -14,8 +14,6 @@
// should have protected or private destructors.
// - WeakPtrFactory members that refer to their outer class should be the last
// member.
-// - Enum types with a xxxx_LAST or xxxxLast const actually have that constant
-// have the maximal value for that type.
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/AST.h"
@@ -50,9 +48,6 @@ const char kProtectedNonVirtualDtor[] =
const char kWeakPtrFactoryOrder[] =
"[chromium-style] WeakPtrFactory members which refer to their outer class "
"must be the last member in the outer class definition.";
-const char kBadLastEnumValue[] =
- "[chromium-style] _LAST/Last constants of enum types must have the maximal "
- "value for any constant of that type.";
const char kNoteInheritance[] =
"[chromium-style] %0 inherits from %1 here";
const char kNoteImplicitDtor[] =
@@ -110,8 +105,6 @@ class FindBadConstructsConsumer : public ChromeClassTester {
getErrorLevel(), kProtectedNonVirtualDtor);
diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
getErrorLevel(), kWeakPtrFactoryOrder);
- diag_bad_enum_last_value_ = diagnostic().getCustomDiagID(
- getErrorLevel(), kBadLastEnumValue);
// Registers notes to make it easier to interpret warnings.
diag_note_inheritance_ = diagnostic().getCustomDiagID(
@@ -148,32 +141,6 @@ class FindBadConstructsConsumer : public ChromeClassTester {
CheckWeakPtrFactoryMembers(record_location, record);
}
- virtual void CheckChromeEnum(SourceLocation enum_location,
- EnumDecl* enum_decl) {
- bool got_one = false;
- llvm::APSInt max_so_far;
- EnumDecl::enumerator_iterator iter;
- for (iter = enum_decl->enumerator_begin();
- iter != enum_decl->enumerator_end(); ++iter) {
- if (!got_one) {
- max_so_far = iter->getInitVal();
- got_one = true;
- } else if (iter->getInitVal() > max_so_far)
- max_so_far = iter->getInitVal();
- }
- for (iter = enum_decl->enumerator_begin();
- iter != enum_decl->enumerator_end(); ++iter) {
- std::string name = iter->getNameAsString();
- if (((name.size() > 4 &&
- name.compare(name.size() - 4, 4, "Last") == 0) ||
- (name.size() > 5 &&
- name.compare(name.size() - 5, 5, "_LAST") == 0)) &&
- iter->getInitVal() < max_so_far) {
- diagnostic().Report(iter->getLocation(), diag_bad_enum_last_value_);
- }
- }
- }
-
private:
// The type of problematic ref-counting pattern that was encountered.
enum RefcountIssue {
@@ -190,7 +157,6 @@ class FindBadConstructsConsumer : public ChromeClassTester {
unsigned diag_public_dtor_;
unsigned diag_protected_non_virtual_dtor_;
unsigned diag_weak_ptr_factory_order_;
- unsigned diag_bad_enum_last_value_;
unsigned diag_note_inheritance_;
unsigned diag_note_implicit_dtor_;
unsigned diag_note_public_dtor_;