Add a check to the FindBadConstructs.cpp clang plugin for bad enum last values.

tools/clang/plugins/FindBadConstructs.cpp

Index: tools/clang/plugins/FindBadConstructs.cpp
diff --git a/tools/clang/plugins/FindBadConstructs.cpp b/tools/clang/plugins/FindBadConstructs.cpp
index c8d166e38dbf53a986b24e2e3c1f5562637b8acd..6f6fe4f8cd633f0f369af5c11ba9699d9ac79fcd 100644
--- a/tools/clang/plugins/FindBadConstructs.cpp
+++ b/tools/clang/plugins/FindBadConstructs.cpp
@@ -14,6 +14,8 @@
 //   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"
@@ -48,6 +50,9 @@ 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[] =
@@ -79,12 +84,14 @@ struct FindBadConstructsOptions {
   FindBadConstructsOptions() : check_base_classes(false),
                                check_virtuals_in_implementations(true),
                                check_url_directory(false),
-                               check_weak_ptr_factory_order(false) {
+                               check_weak_ptr_factory_order(false),
+                               check_bad_enum_last_value(true) {
   }
   bool check_base_classes;
   bool check_virtuals_in_implementations;
   bool check_url_directory;
   bool check_weak_ptr_factory_order;
+  bool check_bad_enum_last_value;
 };
 
 // Searches for constructs that we know we don't want in the Chromium code base.
@@ -107,6 +114,8 @@ 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(
@@ -143,6 +152,34 @@ class FindBadConstructsConsumer : public ChromeClassTester {
       CheckWeakPtrFactoryMembers(record_location, record);
   }
 
+  virtual void CheckChromeEnum(SourceLocation enum_location,
+                               EnumDecl* enum_decl) {
+    if (options_.check_bad_enum_last_value) {

[darin (slow to review), 2014/02/12 05:59:47]

nit: I recommend returning early if this option is not set. That way you get to
reduce indentation for the rest of the function.

+      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 {
@@ -159,6 +196,7 @@ 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_;
@@ -712,6 +750,8 @@ class FindBadConstructsAction : public PluginASTAction {
       } else if (args[i] == "check-weak-ptr-factory-order") {
         // TODO(dmichael): Remove this once http://crbug.com/303818 is fixed.
         options_.check_weak_ptr_factory_order = true;
+      } else if (args[i] == "check-bad-enum-last-value") {
+        options_.check_bad_enum_last_value = true;
       } else {
         parsed = false;
         llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";