diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-26 20:46:51 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-26 20:46:51 +0000 |
commit | 1da30523f26bdecc2928fda29ec9f3d9c40fe17f (patch) | |
tree | d8ce009c54e6b9748b4a6c39c80d67d99afbbfda /tools | |
parent | b887777f93c48f746d1b5ee98113d87096f4bd64 (diff) | |
download | chromium_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.cpp | 16 | ||||
-rw-r--r-- | tools/clang/plugins/ChromeClassTester.h | 6 | ||||
-rw-r--r-- | tools/clang/plugins/FindBadConstructs.cpp | 34 |
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_; |