Chrome clang plugin cleanup part 1 of ??

The clang plugin has grown organically over its lifetime. This patch
just splits apart the options struct, the PluginASTAction (which most
developers don't care about), and the actual ASTConsumer into separate
units.

R=erg@chromium.org

Review URL: https://codereview.chromium.org/348543002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278487 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 899 insertions, 779 deletions

diff --git a/tools/clang/plugins/FindBadConstructs.cpp b/tools/clang/plugins/FindBadConstructs.cpp
deleted file mode 100644
index 4b74fbe..0000000
--- a/tools/clang/plugins/FindBadConstructs.cpp
+++ /dev/null
@@ -1,779 +0,0 @@
-// 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.
-
-// This file defines a bunch of recurring problems in the Chromium C++ code.
-//
-// Checks that are implemented:
-// - Constructors/Destructors should not be inlined if they are of a complex
-//   class type.
-// - Missing "virtual" keywords on methods that should be virtual.
-// - Non-annotated overriding virtual methods.
-// - Virtual methods with nonempty implementations in their headers.
-// - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
-//   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"
-#include "clang/AST/Attr.h"
-#include "clang/AST/CXXInheritance.h"
-#include "clang/AST/TypeLoc.h"
-#include "clang/Basic/SourceManager.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendPluginRegistry.h"
-#include "clang/Lex/Lexer.h"
-#include "llvm/Support/raw_ostream.h"
-
-#include "ChromeClassTester.h"
-
-using namespace clang;
-
-namespace {
-
-const char kMethodRequiresOverride[] =
-    "[chromium-style] Overriding method must be marked with OVERRIDE.";
-const char kMethodRequiresVirtual[] =
-    "[chromium-style] Overriding method must have \"virtual\" keyword.";
-const char kNoExplicitDtor[] =
-    "[chromium-style] Classes that are ref-counted should have explicit "
-    "destructors that are declared protected or private.";
-const char kPublicDtor[] =
-    "[chromium-style] Classes that are ref-counted should have "
-    "destructors that are declared protected or private.";
-const char kProtectedNonVirtualDtor[] =
-    "[chromium-style] Classes that are ref-counted and have non-private "
-    "destructors should declare their destructor virtual.";
-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[] =
-    "[chromium-style] No explicit destructor for %0 defined";
-const char kNotePublicDtor[] =
-    "[chromium-style] Public destructor declared here";
-const char kNoteProtectedNonVirtualDtor[] =
-    "[chromium-style] Protected non-virtual destructor declared here";
-
-bool TypeHasNonTrivialDtor(const Type* type) {
-  if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
-    return !cxx_r->hasTrivialDestructor();
-
-  return false;
-}
-
-// Returns the underlying Type for |type| by expanding typedefs and removing
-// any namespace qualifiers. This is similar to desugaring, except that for
-// ElaboratedTypes, desugar will unwrap too much.
-const Type* UnwrapType(const Type* type) {
-  if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
-    return UnwrapType(elaborated->getNamedType().getTypePtr());
-  if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
-    return UnwrapType(typedefed->desugar().getTypePtr());
-  return type;
-}
-
-struct FindBadConstructsOptions {
-  FindBadConstructsOptions() : check_base_classes(false),
-                               check_virtuals_in_implementations(true),
-                               check_weak_ptr_factory_order(false),
-                               check_enum_last_value(false) {
-  }
-  bool check_base_classes;
-  bool check_virtuals_in_implementations;
-  bool check_weak_ptr_factory_order;
-  bool check_enum_last_value;
-};
-
-// Searches for constructs that we know we don't want in the Chromium code base.
-class FindBadConstructsConsumer : public ChromeClassTester {
- public:
-  FindBadConstructsConsumer(CompilerInstance& instance,
-                            const FindBadConstructsOptions& options)
-      : ChromeClassTester(instance),
-        options_(options) {
-    // Register warning/error messages.
-    diag_method_requires_override_ = diagnostic().getCustomDiagID(
-        getErrorLevel(), kMethodRequiresOverride);
-    diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
-        getErrorLevel(), kMethodRequiresVirtual);
-    diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
-        getErrorLevel(), kNoExplicitDtor);
-    diag_public_dtor_ = diagnostic().getCustomDiagID(
-        getErrorLevel(), kPublicDtor);
-    diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
-        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(
-        DiagnosticsEngine::Note, kNoteInheritance);
-    diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
-        DiagnosticsEngine::Note, kNoteImplicitDtor);
-    diag_note_public_dtor_ = diagnostic().getCustomDiagID(
-        DiagnosticsEngine::Note, kNotePublicDtor);
-    diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
-        DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
-  }
-
-  virtual void CheckChromeClass(SourceLocation record_location,
-                                CXXRecordDecl* record) {
-    bool implementation_file = InImplementationFile(record_location);
-
-    if (!implementation_file) {
-      // Only check for "heavy" constructors/destructors in header files;
-      // within implementation files, there is no performance cost.
-      CheckCtorDtorWeight(record_location, record);
-    }
-
-    if (!implementation_file || options_.check_virtuals_in_implementations) {
-      bool warn_on_inline_bodies = !implementation_file;
-
-      // Check that all virtual methods are marked accordingly with both
-      // virtual and OVERRIDE.
-      CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
-    }
-
-    CheckRefCountedDtors(record_location, record);
-
-    if (options_.check_weak_ptr_factory_order)
-      CheckWeakPtrFactoryMembers(record_location, record);
-  }
-
-  virtual void CheckChromeEnum(SourceLocation enum_location,
-                               EnumDecl* enum_decl) {
-    if (!options_.check_enum_last_value)
-      return;
-
-    bool got_one = false;
-    bool is_signed = false;
-    llvm::APSInt max_so_far;
-    EnumDecl::enumerator_iterator iter;
-    for (iter = enum_decl->enumerator_begin();
-         iter != enum_decl->enumerator_end(); ++iter) {
-          llvm::APSInt current_value = iter->getInitVal();
-      if (!got_one) {
-        max_so_far = current_value;
-        is_signed = current_value.isSigned();
-        got_one = true;
-      } else {
-        if (is_signed != current_value.isSigned()) {
-          // This only happens in some cases when compiling C (not C++) files,
-          // so it is OK to bail out here.
-          return;
-        }
-        if (current_value > max_so_far)
-          max_so_far = current_value;
-      }
-    }
-    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 {
-    None,
-    ImplicitDestructor,
-    PublicDestructor
-  };
-
-  FindBadConstructsOptions options_;
-
-  unsigned diag_method_requires_override_;
-  unsigned diag_method_requires_virtual_;
-  unsigned diag_no_explicit_dtor_;
-  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_;
-  unsigned diag_note_protected_non_virtual_dtor_;
-
-  // Prints errors if the constructor/destructor weight is too heavy.
-  void CheckCtorDtorWeight(SourceLocation record_location,
-                           CXXRecordDecl* record) {
-    // We don't handle anonymous structs. If this record doesn't have a
-    // name, it's of the form:
-    //
-    // struct {
-    //   ...
-    // } name_;
-    if (record->getIdentifier() == NULL)
-      return;
-
-    // Count the number of templated base classes as a feature of whether the
-    // destructor can be inlined.
-    int templated_base_classes = 0;
-    for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
-         it != record->bases_end(); ++it) {
-      if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
-          TypeLoc::TemplateSpecialization) {
-        ++templated_base_classes;
-      }
-    }
-
-    // Count the number of trivial and non-trivial member variables.
-    int trivial_member = 0;
-    int non_trivial_member = 0;
-    int templated_non_trivial_member = 0;
-    for (RecordDecl::field_iterator it = record->field_begin();
-         it != record->field_end(); ++it) {
-      CountType(it->getType().getTypePtr(),
-                &trivial_member,
-                &non_trivial_member,
-                &templated_non_trivial_member);
-    }
-
-    // Check to see if we need to ban inlined/synthesized constructors. Note
-    // that the cutoffs here are kind of arbitrary. Scores over 10 break.
-    int dtor_score = 0;
-    // Deriving from a templated base class shouldn't be enough to trigger
-    // the ctor warning, but if you do *anything* else, it should.
-    //
-    // TODO(erg): This is motivated by templated base classes that don't have
-    // any data members. Somehow detect when templated base classes have data
-    // members and treat them differently.
-    dtor_score += templated_base_classes * 9;
-    // Instantiating a template is an insta-hit.
-    dtor_score += templated_non_trivial_member * 10;
-    // The fourth normal class member should trigger the warning.
-    dtor_score += non_trivial_member * 3;
-
-    int ctor_score = dtor_score;
-    // You should be able to have 9 ints before we warn you.
-    ctor_score += trivial_member;
-
-    if (ctor_score >= 10) {
-      if (!record->hasUserDeclaredConstructor()) {
-        emitWarning(record_location,
-                    "Complex class/struct needs an explicit out-of-line "
-                    "constructor.");
-      } else {
-        // Iterate across all the constructors in this file and yell if we
-        // find one that tries to be inline.
-        for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
-             it != record->ctor_end(); ++it) {
-          if (it->hasInlineBody()) {
-            if (it->isCopyConstructor() &&
-                !record->hasUserDeclaredCopyConstructor()) {
-              emitWarning(record_location,
-                          "Complex class/struct needs an explicit out-of-line "
-                          "copy constructor.");
-            } else {
-              emitWarning(it->getInnerLocStart(),
-                          "Complex constructor has an inlined body.");
-            }
-          }
-        }
-      }
-    }
-
-    // The destructor side is equivalent except that we don't check for
-    // trivial members; 20 ints don't need a destructor.
-    if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
-      if (!record->hasUserDeclaredDestructor()) {
-        emitWarning(
-            record_location,
-            "Complex class/struct needs an explicit out-of-line "
-            "destructor.");
-      } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
-        if (dtor->hasInlineBody()) {
-          emitWarning(dtor->getInnerLocStart(),
-                      "Complex destructor has an inline body.");
-        }
-      }
-    }
-  }
-
-  void CheckVirtualMethod(const CXXMethodDecl* method,
-                          bool warn_on_inline_bodies) {
-    if (!method->isVirtual())
-      return;
-
-    if (!method->isVirtualAsWritten()) {
-      SourceLocation loc = method->getTypeSpecStartLoc();
-      if (isa<CXXDestructorDecl>(method))
-        loc = method->getInnerLocStart();
-      SourceManager& manager = instance().getSourceManager();
-      FullSourceLoc full_loc(loc, manager);
-      SourceLocation spelling_loc = manager.getSpellingLoc(loc);
-      diagnostic().Report(full_loc, diag_method_requires_virtual_)
-          << FixItHint::CreateInsertion(spelling_loc, "virtual ");
-    }
-
-    // Virtual methods should not have inline definitions beyond "{}". This
-    // only matters for header files.
-    if (warn_on_inline_bodies && method->hasBody() &&
-        method->hasInlineBody()) {
-      if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
-        if (cs->size()) {
-          emitWarning(
-              cs->getLBracLoc(),
-              "virtual methods with non-empty bodies shouldn't be "
-              "declared inline.");
-        }
-      }
-    }
-  }
-
-  bool InTestingNamespace(const Decl* record) {
-    return GetNamespace(record).find("testing") != std::string::npos;
-  }
-
-  bool IsMethodInBannedOrTestingNamespace(const CXXMethodDecl* method) {
-    if (InBannedNamespace(method))
-      return true;
-    for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
-         i != method->end_overridden_methods();
-         ++i) {
-      const CXXMethodDecl* overridden = *i;
-      if (IsMethodInBannedOrTestingNamespace(overridden) ||
-          InTestingNamespace(overridden)) {
-        return true;
-      }
-    }
-
-    return false;
-  }
-
-  void CheckOverriddenMethod(const CXXMethodDecl* method) {
-    if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
-      return;
-
-    if (isa<CXXDestructorDecl>(method) || method->isPure())
-      return;
-
-    if (IsMethodInBannedOrTestingNamespace(method))
-      return;
-
-    SourceManager& manager = instance().getSourceManager();
-    SourceRange type_info_range =
-        method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
-    FullSourceLoc loc(type_info_range.getBegin(), manager);
-
-    // Build the FixIt insertion point after the end of the method definition,
-    // including any const-qualifiers and attributes, and before the opening
-    // of the l-curly-brace (if inline) or the semi-color (if a declaration).
-    SourceLocation spelling_end =
-        manager.getSpellingLoc(type_info_range.getEnd());
-    if (spelling_end.isValid()) {
-      SourceLocation token_end = Lexer::getLocForEndOfToken(
-          spelling_end, 0, manager, LangOptions());
-      diagnostic().Report(token_end, diag_method_requires_override_)
-          << FixItHint::CreateInsertion(token_end, " OVERRIDE");
-    } else {
-      diagnostic().Report(loc, diag_method_requires_override_);
-    }
-  }
-
-  // Makes sure there is a "virtual" keyword on virtual methods.
-  //
-  // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
-  // trick to get around that. If a class has member variables whose types are
-  // in the "testing" namespace (which is how gmock works behind the scenes),
-  // there's a really high chance we won't care about these errors
-  void CheckVirtualMethods(SourceLocation record_location,
-                           CXXRecordDecl* record,
-                           bool warn_on_inline_bodies) {
-    for (CXXRecordDecl::field_iterator it = record->field_begin();
-         it != record->field_end(); ++it) {
-      CXXRecordDecl* record_type =
-          it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
-          getAsCXXRecordDecl();
-      if (record_type) {
-        if (InTestingNamespace(record_type)) {
-          return;
-        }
-      }
-    }
-
-    for (CXXRecordDecl::method_iterator it = record->method_begin();
-         it != record->method_end(); ++it) {
-      if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
-        // Ignore constructors and assignment operators.
-      } else if (isa<CXXDestructorDecl>(*it) &&
-          !record->hasUserDeclaredDestructor()) {
-        // Ignore non-user-declared destructors.
-      } else {
-        CheckVirtualMethod(*it, warn_on_inline_bodies);
-        CheckOverriddenMethod(*it);
-      }
-    }
-  }
-
-  void CountType(const Type* type,
-                 int* trivial_member,
-                 int* non_trivial_member,
-                 int* templated_non_trivial_member) {
-    switch (type->getTypeClass()) {
-      case Type::Record: {
-        // Simplifying; the whole class isn't trivial if the dtor is, but
-        // we use this as a signal about complexity.
-        if (TypeHasNonTrivialDtor(type))
-          (*trivial_member)++;
-        else
-          (*non_trivial_member)++;
-        break;
-      }
-      case Type::TemplateSpecialization: {
-        TemplateName name =
-            dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
-        bool whitelisted_template = false;
-
-        // HACK: I'm at a loss about how to get the syntax checker to get
-        // whether a template is exterened or not. For the first pass here,
-        // just do retarded string comparisons.
-        if (TemplateDecl* decl = name.getAsTemplateDecl()) {
-          std::string base_name = decl->getNameAsString();
-          if (base_name == "basic_string")
-            whitelisted_template = true;
-        }
-
-        if (whitelisted_template)
-          (*non_trivial_member)++;
-        else
-          (*templated_non_trivial_member)++;
-        break;
-      }
-      case Type::Elaborated: {
-        CountType(
-            dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
-            trivial_member, non_trivial_member, templated_non_trivial_member);
-        break;
-      }
-      case Type::Typedef: {
-        while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
-          type = TT->getDecl()->getUnderlyingType().getTypePtr();
-        }
-        CountType(type, trivial_member, non_trivial_member,
-                  templated_non_trivial_member);
-        break;
-      }
-      default: {
-        // Stupid assumption: anything we see that isn't the above is one of
-        // the 20 integer types.
-        (*trivial_member)++;
-        break;
-      }
-    }
-  }
-
-  // Check |record| for issues that are problematic for ref-counted types.
-  // Note that |record| may not be a ref-counted type, but a base class for
-  // a type that is.
-  // If there are issues, update |loc| with the SourceLocation of the issue
-  // and returns appropriately, or returns None if there are no issues.
-  static RefcountIssue CheckRecordForRefcountIssue(
-      const CXXRecordDecl* record,
-      SourceLocation &loc) {
-    if (!record->hasUserDeclaredDestructor()) {
-      loc = record->getLocation();
-      return ImplicitDestructor;
-    }
-
-    if (CXXDestructorDecl* dtor = record->getDestructor()) {
-      if (dtor->getAccess() == AS_public) {
-        loc = dtor->getInnerLocStart();
-        return PublicDestructor;
-      }
-    }
-
-    return None;
-  }
-
-  // Adds either a warning or error, based on the current handling of
-  // -Werror.
-  DiagnosticsEngine::Level getErrorLevel() {
-    return diagnostic().getWarningsAsErrors() ?
-        DiagnosticsEngine::Error : DiagnosticsEngine::Warning;
-  }
-
-  // Returns true if |base| specifies one of the Chromium reference counted
-  // classes (base::RefCounted / base::RefCountedThreadSafe).
-  static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
-                                   CXXBasePath& path,
-                                   void* user_data) {
-    FindBadConstructsConsumer* self =
-        static_cast<FindBadConstructsConsumer*>(user_data);
-
-    const TemplateSpecializationType* base_type =
-        dyn_cast<TemplateSpecializationType>(
-            UnwrapType(base->getType().getTypePtr()));
-    if (!base_type) {
-      // Base-most definition is not a template, so this cannot derive from
-      // base::RefCounted. However, it may still be possible to use with a
-      // scoped_refptr<> and support ref-counting, so this is not a perfect
-      // guarantee of safety.
-      return false;
-    }
-
-    TemplateName name = base_type->getTemplateName();
-    if (TemplateDecl* decl = name.getAsTemplateDecl()) {
-      std::string base_name = decl->getNameAsString();
-
-      // Check for both base::RefCounted and base::RefCountedThreadSafe.
-      if (base_name.compare(0, 10, "RefCounted") == 0 &&
-          self->GetNamespace(decl) == "base") {
-        return true;
-      }
-    }
-
-    return false;
-  }
-
-  // Returns true if |base| specifies a class that has a public destructor,
-  // either explicitly or implicitly.
-  static bool HasPublicDtorCallback(const CXXBaseSpecifier* base,
-                                    CXXBasePath& path,
-                                    void* user_data) {
-    // Only examine paths that have public inheritance, as they are the
-    // only ones which will result in the destructor potentially being
-    // exposed. This check is largely redundant, as Chromium code should be
-    // exclusively using public inheritance.
-    if (path.Access != AS_public)
-      return false;
-
-    CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(
-        base->getType()->getAs<RecordType>()->getDecl());
-    SourceLocation unused;
-    return None != CheckRecordForRefcountIssue(record, unused);
-  }
-
-  // Outputs a C++ inheritance chain as a diagnostic aid.
-  void PrintInheritanceChain(const CXXBasePath& path) {
-    for (CXXBasePath::const_iterator it = path.begin(); it != path.end();
-         ++it) {
-      diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
-          << it->Class << it->Base->getType();
-    }
-  }
-
-  unsigned DiagnosticForIssue(RefcountIssue issue) {
-    switch (issue) {
-      case ImplicitDestructor:
-        return diag_no_explicit_dtor_;
-      case PublicDestructor:
-        return diag_public_dtor_;
-      case None:
-        assert(false && "Do not call DiagnosticForIssue with issue None");
-        return 0;
-    }
-    assert(false);
-    return 0;
-  }
-
-  // Check |record| to determine if it has any problematic refcounting
-  // issues and, if so, print them as warnings/errors based on the current
-  // value of getErrorLevel().
-  //
-  // If |record| is a C++ class, and if it inherits from one of the Chromium
-  // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
-  // ensure that there are no public destructors in the class hierarchy. This
-  // is to guard against accidentally stack-allocating a RefCounted class or
-  // sticking it in a non-ref-counted container (like scoped_ptr<>).
-  void CheckRefCountedDtors(SourceLocation record_location,
-                            CXXRecordDecl* record) {
-    // Skip anonymous structs.
-    if (record->getIdentifier() == NULL)
-      return;
-
-    // Determine if the current type is even ref-counted.
-    CXXBasePaths refcounted_path;
-    if (!record->lookupInBases(
-            &FindBadConstructsConsumer::IsRefCountedCallback, this,
-            refcounted_path)) {
-      return;  // Class does not derive from a ref-counted base class.
-    }
-
-    // Easy check: Check to see if the current type is problematic.
-    SourceLocation loc;
-    RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
-    if (issue != None) {
-      diagnostic().Report(loc, DiagnosticForIssue(issue));
-      PrintInheritanceChain(refcounted_path.front());
-      return;
-    }
-    if (CXXDestructorDecl* dtor =
-        refcounted_path.begin()->back().Class->getDestructor()) {
-      if (dtor->getAccess() == AS_protected &&
-          !dtor->isVirtual()) {
-        loc = dtor->getInnerLocStart();
-        diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
-        return;
-      }
-    }
-
-    // Long check: Check all possible base classes for problematic
-    // destructors. This checks for situations involving multiple
-    // inheritance, where the ref-counted class may be implementing an
-    // interface that has a public or implicit destructor.
-    //
-    // struct SomeInterface {
-    //   virtual void DoFoo();
-    // };
-    //
-    // struct RefCountedInterface
-    //    : public base::RefCounted<RefCountedInterface>,
-    //      public SomeInterface {
-    //  private:
-    //   friend class base::Refcounted<RefCountedInterface>;
-    //   virtual ~RefCountedInterface() {}
-    // };
-    //
-    // While RefCountedInterface is "safe", in that its destructor is
-    // private, it's possible to do the following "unsafe" code:
-    //   scoped_refptr<RefCountedInterface> some_class(
-    //       new RefCountedInterface);
-    //   // Calls SomeInterface::~SomeInterface(), which is unsafe.
-    //   delete static_cast<SomeInterface*>(some_class.get());
-    if (!options_.check_base_classes)
-      return;
-
-    // Find all public destructors. This will record the class hierarchy
-    // that leads to the public destructor in |dtor_paths|.
-    CXXBasePaths dtor_paths;
-    if (!record->lookupInBases(
-            &FindBadConstructsConsumer::HasPublicDtorCallback, this,
-            dtor_paths)) {
-      return;
-    }
-
-    for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
-         it != dtor_paths.end(); ++it) {
-      // The record with the problem will always be the last record
-      // in the path, since it is the record that stopped the search.
-      const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
-          it->back().Base->getType()->getAs<RecordType>()->getDecl());
-
-      issue = CheckRecordForRefcountIssue(problem_record, loc);
-
-      if (issue == ImplicitDestructor) {
-        diagnostic().Report(record_location, diag_no_explicit_dtor_);
-        PrintInheritanceChain(refcounted_path.front());
-        diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
-        PrintInheritanceChain(*it);
-      } else if (issue == PublicDestructor) {
-        diagnostic().Report(record_location, diag_public_dtor_);
-        PrintInheritanceChain(refcounted_path.front());
-        diagnostic().Report(loc, diag_note_public_dtor_);
-        PrintInheritanceChain(*it);
-      }
-    }
-  }
-
-  // Check for any problems with WeakPtrFactory class members. This currently
-  // only checks that any WeakPtrFactory<T> member of T appears as the last
-  // data member in T. We could consider checking for bad uses of
-  // WeakPtrFactory to refer to other data members, but that would require
-  // looking at the initializer list in constructors to see what the factory
-  // points to.
-  // Note, if we later add other unrelated checks of data members, we should
-  // consider collapsing them in to one loop to avoid iterating over the data
-  // members more than once.
-  void CheckWeakPtrFactoryMembers(SourceLocation record_location,
-                                  CXXRecordDecl* record) {
-    // Skip anonymous structs.
-    if (record->getIdentifier() == NULL)
-      return;
-
-    // Iterate through members of the class.
-    RecordDecl::field_iterator iter(record->field_begin()),
-                               the_end(record->field_end());
-    SourceLocation weak_ptr_factory_location;  // Invalid initially.
-    for (; iter != the_end; ++iter) {
-      // If we enter the loop but have already seen a matching WeakPtrFactory,
-      // it means there is at least one member after the factory.
-      if (weak_ptr_factory_location.isValid()) {
-        diagnostic().Report(weak_ptr_factory_location,
-                            diag_weak_ptr_factory_order_);
-      }
-      const TemplateSpecializationType* template_spec_type =
-          iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
-      if (template_spec_type) {
-        const TemplateDecl* template_decl =
-            template_spec_type->getTemplateName().getAsTemplateDecl();
-        if (template_decl && template_spec_type->getNumArgs() >= 1) {
-          if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
-              GetNamespace(template_decl) == "base") {
-            const TemplateArgument& arg = template_spec_type->getArg(0);
-            if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
-                record->getTypeForDecl()->getAsCXXRecordDecl()) {
-              weak_ptr_factory_location = iter->getLocation();
-            }
-          }
-        }
-      }
-    }
-  }
-};
-
-
-class FindBadConstructsAction : public PluginASTAction {
- public:
-  FindBadConstructsAction() {
-  }
-
- protected:
-  // Overridden from PluginASTAction:
-  virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
-                                         llvm::StringRef ref) {
-    return new FindBadConstructsConsumer(instance, options_);
-  }
-
-  virtual bool ParseArgs(const CompilerInstance& instance,
-                         const std::vector<std::string>& args) {
-    bool parsed = true;
-
-    for (size_t i = 0; i < args.size() && parsed; ++i) {
-      if (args[i] == "skip-virtuals-in-implementations") {
-        // TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
-        options_.check_virtuals_in_implementations = false;
-      } else if (args[i] == "check-base-classes") {
-        // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
-        options_.check_base_classes = true;
-      } 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-enum-last-value") {
-        // TODO(tsepez): Enable this by default once http://crbug.com/356815
-        // and http://crbug.com/356816 are fixed.
-        options_.check_enum_last_value = true;
-      } else {
-        parsed = false;
-        llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
-      }
-    }
-
-    return parsed;
-  }
-
- private:
-  FindBadConstructsOptions options_;
-};
-
-}  // namespace
-
-static FrontendPluginRegistry::Add<FindBadConstructsAction>
-X("find-bad-constructs", "Finds bad C++ constructs");
diff --git a/tools/clang/plugins/FindBadConstructsAction.cpp b/tools/clang/plugins/FindBadConstructsAction.cpp
new file mode 100644
index 0000000..838590d
--- /dev/null
+++ b/tools/clang/plugins/FindBadConstructsAction.cpp
@@ -0,0 +1,55 @@
+// 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.
+
+#include "FindBadConstructsAction.h"
+
+#include "clang/Frontend/FrontendPluginRegistry.h"
+
+#include "FindBadConstructsConsumer.h"
+
+using namespace clang;
+
+namespace chrome_checker {
+
+FindBadConstructsAction::FindBadConstructsAction() {
+}
+
+ASTConsumer* FindBadConstructsAction::CreateASTConsumer(
+    CompilerInstance& instance,
+    llvm::StringRef ref) {
+  return new FindBadConstructsConsumer(instance, options_);
+}
+
+bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
+                                        const std::vector<std::string>& args) {
+  bool parsed = true;
+
+  for (size_t i = 0; i < args.size() && parsed; ++i) {
+    if (args[i] == "skip-virtuals-in-implementations") {
+      // TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
+      options_.check_virtuals_in_implementations = false;
+    } else if (args[i] == "check-base-classes") {
+      // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
+      options_.check_base_classes = true;
+    } 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-enum-last-value") {
+      // TODO(tsepez): Enable this by default once http://crbug.com/356815
+      // and http://crbug.com/356816 are fixed.
+      options_.check_enum_last_value = true;
+    } else {
+      parsed = false;
+      llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
+    }
+  }
+
+  return parsed;
+}
+
+}  // namespace chrome_checker
+
+static FrontendPluginRegistry::Add<chrome_checker::FindBadConstructsAction> X(
+    "find-bad-constructs",
+    "Finds bad C++ constructs");
diff --git a/tools/clang/plugins/FindBadConstructsAction.h b/tools/clang/plugins/FindBadConstructsAction.h
new file mode 100644
index 0000000..887d339
--- /dev/null
+++ b/tools/clang/plugins/FindBadConstructsAction.h
@@ -0,0 +1,32 @@
+// 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.
+
+#ifndef TOOLS_CLANG_PLUGINS_FINDBADCONSTRUCTIONS_ACTION_H_
+#define TOOLS_CLANG_PLUGINS_FINDBADCONSTRUCTIONS_ACTION_H_
+
+#include "clang/Frontend/FrontendAction.h"
+
+#include "Options.h"
+
+namespace chrome_checker {
+
+class FindBadConstructsAction : public clang::PluginASTAction {
+ public:
+  FindBadConstructsAction();
+
+ protected:
+  // Overridden from PluginASTAction:
+  virtual clang::ASTConsumer* CreateASTConsumer(
+      clang::CompilerInstance& instance,
+      llvm::StringRef ref);
+  virtual bool ParseArgs(const clang::CompilerInstance& instance,
+                         const std::vector<std::string>& args);
+
+ private:
+  Options options_;
+};
+
+}  // namespace chrome_checker
+
+#endif  // TOOLS_CLANG_PLUGINS_FINDBADCONSTRUCTIONS_ACTION_H_
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
new file mode 100644
index 0000000..c4219a1
--- /dev/null
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -0,0 +1,687 @@
+// 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.
+
+#include "FindBadConstructsConsumer.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+
+namespace chrome_checker {
+
+namespace {
+
+const char kMethodRequiresOverride[] =
+    "[chromium-style] Overriding method must be marked with OVERRIDE.";
+const char kMethodRequiresVirtual[] =
+    "[chromium-style] Overriding method must have \"virtual\" keyword.";
+const char kNoExplicitDtor[] =
+    "[chromium-style] Classes that are ref-counted should have explicit "
+    "destructors that are declared protected or private.";
+const char kPublicDtor[] =
+    "[chromium-style] Classes that are ref-counted should have "
+    "destructors that are declared protected or private.";
+const char kProtectedNonVirtualDtor[] =
+    "[chromium-style] Classes that are ref-counted and have non-private "
+    "destructors should declare their destructor virtual.";
+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[] =
+    "[chromium-style] No explicit destructor for %0 defined";
+const char kNotePublicDtor[] =
+    "[chromium-style] Public destructor declared here";
+const char kNoteProtectedNonVirtualDtor[] =
+    "[chromium-style] Protected non-virtual destructor declared here";
+
+bool TypeHasNonTrivialDtor(const Type* type) {
+  if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
+    return !cxx_r->hasTrivialDestructor();
+
+  return false;
+}
+
+// Returns the underlying Type for |type| by expanding typedefs and removing
+// any namespace qualifiers. This is similar to desugaring, except that for
+// ElaboratedTypes, desugar will unwrap too much.
+const Type* UnwrapType(const Type* type) {
+  if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
+    return UnwrapType(elaborated->getNamedType().getTypePtr());
+  if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
+    return UnwrapType(typedefed->desugar().getTypePtr());
+  return type;
+}
+
+}  // namespace
+
+FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
+                                                     const Options& options)
+    : ChromeClassTester(instance), options_(options) {
+  // Register warning/error messages.
+  diag_method_requires_override_ =
+      diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride);
+  diag_method_requires_virtual_ =
+      diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresVirtual);
+  diag_no_explicit_dtor_ =
+      diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor);
+  diag_public_dtor_ =
+      diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor);
+  diag_protected_non_virtual_dtor_ =
+      diagnostic().getCustomDiagID(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(DiagnosticsEngine::Note, kNoteInheritance);
+  diag_note_implicit_dtor_ =
+      diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteImplicitDtor);
+  diag_note_public_dtor_ =
+      diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNotePublicDtor);
+  diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
+      DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
+}
+
+void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
+                                                 CXXRecordDecl* record) {
+  bool implementation_file = InImplementationFile(record_location);
+
+  if (!implementation_file) {
+    // Only check for "heavy" constructors/destructors in header files;
+    // within implementation files, there is no performance cost.
+    CheckCtorDtorWeight(record_location, record);
+  }
+
+  if (!implementation_file || options_.check_virtuals_in_implementations) {
+    bool warn_on_inline_bodies = !implementation_file;
+
+    // Check that all virtual methods are marked accordingly with both
+    // virtual and OVERRIDE.
+    CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
+  }
+
+  CheckRefCountedDtors(record_location, record);
+
+  if (options_.check_weak_ptr_factory_order)
+    CheckWeakPtrFactoryMembers(record_location, record);
+}
+
+void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location,
+                                                EnumDecl* enum_decl) {
+  if (!options_.check_enum_last_value)
+    return;
+
+  bool got_one = false;
+  bool is_signed = false;
+  llvm::APSInt max_so_far;
+  EnumDecl::enumerator_iterator iter;
+  for (iter = enum_decl->enumerator_begin();
+       iter != enum_decl->enumerator_end();
+       ++iter) {
+    llvm::APSInt current_value = iter->getInitVal();
+    if (!got_one) {
+      max_so_far = current_value;
+      is_signed = current_value.isSigned();
+      got_one = true;
+    } else {
+      if (is_signed != current_value.isSigned()) {
+        // This only happens in some cases when compiling C (not C++) files,
+        // so it is OK to bail out here.
+        return;
+      }
+      if (current_value > max_so_far)
+        max_so_far = current_value;
+    }
+  }
+  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_);
+    }
+  }
+}
+
+void FindBadConstructsConsumer::CheckCtorDtorWeight(
+    SourceLocation record_location,
+    CXXRecordDecl* record) {
+  // We don't handle anonymous structs. If this record doesn't have a
+  // name, it's of the form:
+  //
+  // struct {
+  //   ...
+  // } name_;
+  if (record->getIdentifier() == NULL)
+    return;
+
+  // Count the number of templated base classes as a feature of whether the
+  // destructor can be inlined.
+  int templated_base_classes = 0;
+  for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
+       it != record->bases_end();
+       ++it) {
+    if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
+        TypeLoc::TemplateSpecialization) {
+      ++templated_base_classes;
+    }
+  }
+
+  // Count the number of trivial and non-trivial member variables.
+  int trivial_member = 0;
+  int non_trivial_member = 0;
+  int templated_non_trivial_member = 0;
+  for (RecordDecl::field_iterator it = record->field_begin();
+       it != record->field_end();
+       ++it) {
+    CountType(it->getType().getTypePtr(),
+              &trivial_member,
+              &non_trivial_member,
+              &templated_non_trivial_member);
+  }
+
+  // Check to see if we need to ban inlined/synthesized constructors. Note
+  // that the cutoffs here are kind of arbitrary. Scores over 10 break.
+  int dtor_score = 0;
+  // Deriving from a templated base class shouldn't be enough to trigger
+  // the ctor warning, but if you do *anything* else, it should.
+  //
+  // TODO(erg): This is motivated by templated base classes that don't have
+  // any data members. Somehow detect when templated base classes have data
+  // members and treat them differently.
+  dtor_score += templated_base_classes * 9;
+  // Instantiating a template is an insta-hit.
+  dtor_score += templated_non_trivial_member * 10;
+  // The fourth normal class member should trigger the warning.
+  dtor_score += non_trivial_member * 3;
+
+  int ctor_score = dtor_score;
+  // You should be able to have 9 ints before we warn you.
+  ctor_score += trivial_member;
+
+  if (ctor_score >= 10) {
+    if (!record->hasUserDeclaredConstructor()) {
+      emitWarning(record_location,
+                  "Complex class/struct needs an explicit out-of-line "
+                  "constructor.");
+    } else {
+      // Iterate across all the constructors in this file and yell if we
+      // find one that tries to be inline.
+      for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
+           it != record->ctor_end();
+           ++it) {
+        if (it->hasInlineBody()) {
+          if (it->isCopyConstructor() &&
+              !record->hasUserDeclaredCopyConstructor()) {
+            emitWarning(record_location,
+                        "Complex class/struct needs an explicit out-of-line "
+                        "copy constructor.");
+          } else {
+            emitWarning(it->getInnerLocStart(),
+                        "Complex constructor has an inlined body.");
+          }
+        }
+      }
+    }
+  }
+
+  // The destructor side is equivalent except that we don't check for
+  // trivial members; 20 ints don't need a destructor.
+  if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
+    if (!record->hasUserDeclaredDestructor()) {
+      emitWarning(record_location,
+                  "Complex class/struct needs an explicit out-of-line "
+                  "destructor.");
+    } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
+      if (dtor->hasInlineBody()) {
+        emitWarning(dtor->getInnerLocStart(),
+                    "Complex destructor has an inline body.");
+      }
+    }
+  }
+}
+
+void FindBadConstructsConsumer::CheckVirtualMethod(const CXXMethodDecl* method,
+                                                   bool warn_on_inline_bodies) {
+  if (!method->isVirtual())
+    return;
+
+  if (!method->isVirtualAsWritten()) {
+    SourceLocation loc = method->getTypeSpecStartLoc();
+    if (isa<CXXDestructorDecl>(method))
+      loc = method->getInnerLocStart();
+    SourceManager& manager = instance().getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    SourceLocation spelling_loc = manager.getSpellingLoc(loc);
+    diagnostic().Report(full_loc, diag_method_requires_virtual_)
+        << FixItHint::CreateInsertion(spelling_loc, "virtual ");
+  }
+
+  // Virtual methods should not have inline definitions beyond "{}". This
+  // only matters for header files.
+  if (warn_on_inline_bodies && method->hasBody() && method->hasInlineBody()) {
+    if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
+      if (cs->size()) {
+        emitWarning(cs->getLBracLoc(),
+                    "virtual methods with non-empty bodies shouldn't be "
+                    "declared inline.");
+      }
+    }
+  }
+}
+
+bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) {
+  return GetNamespace(record).find("testing") != std::string::npos;
+}
+
+bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
+    const CXXMethodDecl* method) {
+  if (InBannedNamespace(method))
+    return true;
+  for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
+       i != method->end_overridden_methods();
+       ++i) {
+    const CXXMethodDecl* overridden = *i;
+    if (IsMethodInBannedOrTestingNamespace(overridden) ||
+        InTestingNamespace(overridden)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void FindBadConstructsConsumer::CheckOverriddenMethod(
+    const CXXMethodDecl* method) {
+  if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
+    return;
+
+  if (isa<CXXDestructorDecl>(method) || method->isPure())
+    return;
+
+  if (IsMethodInBannedOrTestingNamespace(method))
+    return;
+
+  SourceManager& manager = instance().getSourceManager();
+  SourceRange type_info_range =
+      method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
+  FullSourceLoc loc(type_info_range.getBegin(), manager);
+
+  // Build the FixIt insertion point after the end of the method definition,
+  // including any const-qualifiers and attributes, and before the opening
+  // of the l-curly-brace (if inline) or the semi-color (if a declaration).
+  SourceLocation spelling_end =
+      manager.getSpellingLoc(type_info_range.getEnd());
+  if (spelling_end.isValid()) {
+    SourceLocation token_end =
+        Lexer::getLocForEndOfToken(spelling_end, 0, manager, LangOptions());
+    diagnostic().Report(token_end, diag_method_requires_override_)
+        << FixItHint::CreateInsertion(token_end, " OVERRIDE");
+  } else {
+    diagnostic().Report(loc, diag_method_requires_override_);
+  }
+}
+
+// Makes sure there is a "virtual" keyword on virtual methods.
+//
+// Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
+// trick to get around that. If a class has member variables whose types are
+// in the "testing" namespace (which is how gmock works behind the scenes),
+// there's a really high chance we won't care about these errors
+void FindBadConstructsConsumer::CheckVirtualMethods(
+    SourceLocation record_location,
+    CXXRecordDecl* record,
+    bool warn_on_inline_bodies) {
+  for (CXXRecordDecl::field_iterator it = record->field_begin();
+       it != record->field_end();
+       ++it) {
+    CXXRecordDecl* record_type = it->getTypeSourceInfo()
+                                     ->getTypeLoc()
+                                     .getTypePtr()
+                                     ->getAsCXXRecordDecl();
+    if (record_type) {
+      if (InTestingNamespace(record_type)) {
+        return;
+      }
+    }
+  }
+
+  for (CXXRecordDecl::method_iterator it = record->method_begin();
+       it != record->method_end();
+       ++it) {
+    if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
+      // Ignore constructors and assignment operators.
+    } else if (isa<CXXDestructorDecl>(*it) &&
+               !record->hasUserDeclaredDestructor()) {
+      // Ignore non-user-declared destructors.
+    } else {
+      CheckVirtualMethod(*it, warn_on_inline_bodies);
+      CheckOverriddenMethod(*it);
+    }
+  }
+}
+
+void FindBadConstructsConsumer::CountType(const Type* type,
+                                          int* trivial_member,
+                                          int* non_trivial_member,
+                                          int* templated_non_trivial_member) {
+  switch (type->getTypeClass()) {
+    case Type::Record: {
+      // Simplifying; the whole class isn't trivial if the dtor is, but
+      // we use this as a signal about complexity.
+      if (TypeHasNonTrivialDtor(type))
+        (*trivial_member)++;
+      else
+        (*non_trivial_member)++;
+      break;
+    }
+    case Type::TemplateSpecialization: {
+      TemplateName name =
+          dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
+      bool whitelisted_template = false;
+
+      // HACK: I'm at a loss about how to get the syntax checker to get
+      // whether a template is exterened or not. For the first pass here,
+      // just do retarded string comparisons.
+      if (TemplateDecl* decl = name.getAsTemplateDecl()) {
+        std::string base_name = decl->getNameAsString();
+        if (base_name == "basic_string")
+          whitelisted_template = true;
+      }
+
+      if (whitelisted_template)
+        (*non_trivial_member)++;
+      else
+        (*templated_non_trivial_member)++;
+      break;
+    }
+    case Type::Elaborated: {
+      CountType(dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
+                trivial_member,
+                non_trivial_member,
+                templated_non_trivial_member);
+      break;
+    }
+    case Type::Typedef: {
+      while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
+        type = TT->getDecl()->getUnderlyingType().getTypePtr();
+      }
+      CountType(type,
+                trivial_member,
+                non_trivial_member,
+                templated_non_trivial_member);
+      break;
+    }
+    default: {
+      // Stupid assumption: anything we see that isn't the above is one of
+      // the 20 integer types.
+      (*trivial_member)++;
+      break;
+    }
+  }
+}
+
+// Check |record| for issues that are problematic for ref-counted types.
+// Note that |record| may not be a ref-counted type, but a base class for
+// a type that is.
+// If there are issues, update |loc| with the SourceLocation of the issue
+// and returns appropriately, or returns None if there are no issues.
+FindBadConstructsConsumer::RefcountIssue
+FindBadConstructsConsumer::CheckRecordForRefcountIssue(
+    const CXXRecordDecl* record,
+    SourceLocation& loc) {
+  if (!record->hasUserDeclaredDestructor()) {
+    loc = record->getLocation();
+    return ImplicitDestructor;
+  }
+
+  if (CXXDestructorDecl* dtor = record->getDestructor()) {
+    if (dtor->getAccess() == AS_public) {
+      loc = dtor->getInnerLocStart();
+      return PublicDestructor;
+    }
+  }
+
+  return None;
+}
+
+// Adds either a warning or error, based on the current handling of
+// -Werror.
+DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() {
+  return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
+                                            : DiagnosticsEngine::Warning;
+}
+
+// Returns true if |base| specifies one of the Chromium reference counted
+// classes (base::RefCounted / base::RefCountedThreadSafe).
+bool FindBadConstructsConsumer::IsRefCountedCallback(
+    const CXXBaseSpecifier* base,
+    CXXBasePath& path,
+    void* user_data) {
+  FindBadConstructsConsumer* self =
+      static_cast<FindBadConstructsConsumer*>(user_data);
+
+  const TemplateSpecializationType* base_type =
+      dyn_cast<TemplateSpecializationType>(
+          UnwrapType(base->getType().getTypePtr()));
+  if (!base_type) {
+    // Base-most definition is not a template, so this cannot derive from
+    // base::RefCounted. However, it may still be possible to use with a
+    // scoped_refptr<> and support ref-counting, so this is not a perfect
+    // guarantee of safety.
+    return false;
+  }
+
+  TemplateName name = base_type->getTemplateName();
+  if (TemplateDecl* decl = name.getAsTemplateDecl()) {
+    std::string base_name = decl->getNameAsString();
+
+    // Check for both base::RefCounted and base::RefCountedThreadSafe.
+    if (base_name.compare(0, 10, "RefCounted") == 0 &&
+        self->GetNamespace(decl) == "base") {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+// Returns true if |base| specifies a class that has a public destructor,
+// either explicitly or implicitly.
+bool FindBadConstructsConsumer::HasPublicDtorCallback(
+    const CXXBaseSpecifier* base,
+    CXXBasePath& path,
+    void* user_data) {
+  // Only examine paths that have public inheritance, as they are the
+  // only ones which will result in the destructor potentially being
+  // exposed. This check is largely redundant, as Chromium code should be
+  // exclusively using public inheritance.
+  if (path.Access != AS_public)
+    return false;
+
+  CXXRecordDecl* record =
+      dyn_cast<CXXRecordDecl>(base->getType()->getAs<RecordType>()->getDecl());
+  SourceLocation unused;
+  return None != CheckRecordForRefcountIssue(record, unused);
+}
+
+// Outputs a C++ inheritance chain as a diagnostic aid.
+void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath& path) {
+  for (CXXBasePath::const_iterator it = path.begin(); it != path.end(); ++it) {
+    diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
+        << it->Class << it->Base->getType();
+  }
+}
+
+unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue) {
+  switch (issue) {
+    case ImplicitDestructor:
+      return diag_no_explicit_dtor_;
+    case PublicDestructor:
+      return diag_public_dtor_;
+    case None:
+      assert(false && "Do not call DiagnosticForIssue with issue None");
+      return 0;
+  }
+  assert(false);
+  return 0;
+}
+
+// Check |record| to determine if it has any problematic refcounting
+// issues and, if so, print them as warnings/errors based on the current
+// value of getErrorLevel().
+//
+// If |record| is a C++ class, and if it inherits from one of the Chromium
+// ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
+// ensure that there are no public destructors in the class hierarchy. This
+// is to guard against accidentally stack-allocating a RefCounted class or
+// sticking it in a non-ref-counted container (like scoped_ptr<>).
+void FindBadConstructsConsumer::CheckRefCountedDtors(
+    SourceLocation record_location,
+    CXXRecordDecl* record) {
+  // Skip anonymous structs.
+  if (record->getIdentifier() == NULL)
+    return;
+
+  // Determine if the current type is even ref-counted.
+  CXXBasePaths refcounted_path;
+  if (!record->lookupInBases(&FindBadConstructsConsumer::IsRefCountedCallback,
+                             this,
+                             refcounted_path)) {
+    return;  // Class does not derive from a ref-counted base class.
+  }
+
+  // Easy check: Check to see if the current type is problematic.
+  SourceLocation loc;
+  RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
+  if (issue != None) {
+    diagnostic().Report(loc, DiagnosticForIssue(issue));
+    PrintInheritanceChain(refcounted_path.front());
+    return;
+  }
+  if (CXXDestructorDecl* dtor =
+          refcounted_path.begin()->back().Class->getDestructor()) {
+    if (dtor->getAccess() == AS_protected && !dtor->isVirtual()) {
+      loc = dtor->getInnerLocStart();
+      diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
+      return;
+    }
+  }
+
+  // Long check: Check all possible base classes for problematic
+  // destructors. This checks for situations involving multiple
+  // inheritance, where the ref-counted class may be implementing an
+  // interface that has a public or implicit destructor.
+  //
+  // struct SomeInterface {
+  //   virtual void DoFoo();
+  // };
+  //
+  // struct RefCountedInterface
+  //    : public base::RefCounted<RefCountedInterface>,
+  //      public SomeInterface {
+  //  private:
+  //   friend class base::Refcounted<RefCountedInterface>;
+  //   virtual ~RefCountedInterface() {}
+  // };
+  //
+  // While RefCountedInterface is "safe", in that its destructor is
+  // private, it's possible to do the following "unsafe" code:
+  //   scoped_refptr<RefCountedInterface> some_class(
+  //       new RefCountedInterface);
+  //   // Calls SomeInterface::~SomeInterface(), which is unsafe.
+  //   delete static_cast<SomeInterface*>(some_class.get());
+  if (!options_.check_base_classes)
+    return;
+
+  // Find all public destructors. This will record the class hierarchy
+  // that leads to the public destructor in |dtor_paths|.
+  CXXBasePaths dtor_paths;
+  if (!record->lookupInBases(&FindBadConstructsConsumer::HasPublicDtorCallback,
+                             this,
+                             dtor_paths)) {
+    return;
+  }
+
+  for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
+       it != dtor_paths.end();
+       ++it) {
+    // The record with the problem will always be the last record
+    // in the path, since it is the record that stopped the search.
+    const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
+        it->back().Base->getType()->getAs<RecordType>()->getDecl());
+
+    issue = CheckRecordForRefcountIssue(problem_record, loc);
+
+    if (issue == ImplicitDestructor) {
+      diagnostic().Report(record_location, diag_no_explicit_dtor_);
+      PrintInheritanceChain(refcounted_path.front());
+      diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
+      PrintInheritanceChain(*it);
+    } else if (issue == PublicDestructor) {
+      diagnostic().Report(record_location, diag_public_dtor_);
+      PrintInheritanceChain(refcounted_path.front());
+      diagnostic().Report(loc, diag_note_public_dtor_);
+      PrintInheritanceChain(*it);
+    }
+  }
+}
+
+// Check for any problems with WeakPtrFactory class members. This currently
+// only checks that any WeakPtrFactory<T> member of T appears as the last
+// data member in T. We could consider checking for bad uses of
+// WeakPtrFactory to refer to other data members, but that would require
+// looking at the initializer list in constructors to see what the factory
+// points to.
+// Note, if we later add other unrelated checks of data members, we should
+// consider collapsing them in to one loop to avoid iterating over the data
+// members more than once.
+void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
+    SourceLocation record_location,
+    CXXRecordDecl* record) {
+  // Skip anonymous structs.
+  if (record->getIdentifier() == NULL)
+    return;
+
+  // Iterate through members of the class.
+  RecordDecl::field_iterator iter(record->field_begin()),
+      the_end(record->field_end());
+  SourceLocation weak_ptr_factory_location;  // Invalid initially.
+  for (; iter != the_end; ++iter) {
+    // If we enter the loop but have already seen a matching WeakPtrFactory,
+    // it means there is at least one member after the factory.
+    if (weak_ptr_factory_location.isValid()) {
+      diagnostic().Report(weak_ptr_factory_location,
+                          diag_weak_ptr_factory_order_);
+    }
+    const TemplateSpecializationType* template_spec_type =
+        iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
+    if (template_spec_type) {
+      const TemplateDecl* template_decl =
+          template_spec_type->getTemplateName().getAsTemplateDecl();
+      if (template_decl && template_spec_type->getNumArgs() >= 1) {
+        if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
+            GetNamespace(template_decl) == "base") {
+          const TemplateArgument& arg = template_spec_type->getArg(0);
+          if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
+              record->getTypeForDecl()->getAsCXXRecordDecl()) {
+            weak_ptr_factory_location = iter->getLocation();
+          }
+        }
+      }
+    }
+  }
+}
+
+}  // namespace chrome_checker
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.h b/tools/clang/plugins/FindBadConstructsConsumer.h
new file mode 100644
index 0000000..4e51193
--- /dev/null
+++ b/tools/clang/plugins/FindBadConstructsConsumer.h
@@ -0,0 +1,100 @@
+// 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.
+
+// This file defines a bunch of recurring problems in the Chromium C++ code.
+//
+// Checks that are implemented:
+// - Constructors/Destructors should not be inlined if they are of a complex
+//   class type.
+// - Missing "virtual" keywords on methods that should be virtual.
+// - Non-annotated overriding virtual methods.
+// - Virtual methods with nonempty implementations in their headers.
+// - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
+//   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/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceManager.h"
+
+#include "ChromeClassTester.h"
+#include "Options.h"
+
+namespace chrome_checker {
+
+// Searches for constructs that we know we don't want in the Chromium code base.
+class FindBadConstructsConsumer : public ChromeClassTester {
+ public:
+  FindBadConstructsConsumer(clang::CompilerInstance& instance,
+                            const Options& options);
+
+  // ChromeClassTester overrides:
+  virtual void CheckChromeClass(clang::SourceLocation record_location,
+                                clang::CXXRecordDecl* record);
+  virtual void CheckChromeEnum(clang::SourceLocation enum_location,
+                               clang::EnumDecl* enum_decl);
+
+ private:
+  // The type of problematic ref-counting pattern that was encountered.
+  enum RefcountIssue { None, ImplicitDestructor, PublicDestructor };
+
+  void CheckCtorDtorWeight(clang::SourceLocation record_location,
+                           clang::CXXRecordDecl* record);
+
+  void CheckVirtualMethod(const clang::CXXMethodDecl* method,
+                          bool warn_on_inline_bodies);
+
+  bool InTestingNamespace(const clang::Decl* record);
+  bool IsMethodInBannedOrTestingNamespace(const clang::CXXMethodDecl* method);
+
+  void CheckOverriddenMethod(const clang::CXXMethodDecl* method);
+  void CheckVirtualMethods(clang::SourceLocation record_location,
+                           clang::CXXRecordDecl* record,
+                           bool warn_on_inline_bodies);
+
+  void CountType(const clang::Type* type,
+                 int* trivial_member,
+                 int* non_trivial_member,
+                 int* templated_non_trivial_member);
+
+  static RefcountIssue CheckRecordForRefcountIssue(
+      const clang::CXXRecordDecl* record,
+      clang::SourceLocation& loc);
+  clang::DiagnosticsEngine::Level getErrorLevel();
+  static bool IsRefCountedCallback(const clang::CXXBaseSpecifier* base,
+                                   clang::CXXBasePath& path,
+                                   void* user_data);
+  static bool HasPublicDtorCallback(const clang::CXXBaseSpecifier* base,
+                                    clang::CXXBasePath& path,
+                                    void* user_data);
+  void PrintInheritanceChain(const clang::CXXBasePath& path);
+  unsigned DiagnosticForIssue(RefcountIssue issue);
+  void CheckRefCountedDtors(clang::SourceLocation record_location,
+                            clang::CXXRecordDecl* record);
+
+  void CheckWeakPtrFactoryMembers(clang::SourceLocation record_location,
+                                  clang::CXXRecordDecl* record);
+
+  const Options options_;
+
+  unsigned diag_method_requires_override_;
+  unsigned diag_method_requires_virtual_;
+  unsigned diag_no_explicit_dtor_;
+  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_;
+  unsigned diag_note_protected_non_virtual_dtor_;
+};
+
+}  // namespace chrome_checker
diff --git a/tools/clang/plugins/Options.h b/tools/clang/plugins/Options.h
new file mode 100644
index 0000000..bb50124
--- /dev/null
+++ b/tools/clang/plugins/Options.h
@@ -0,0 +1,25 @@
+// 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.
+
+#ifndef TOOLS_CLANG_PLUGINS_OPTIONS_H_
+#define TOOLS_CLANG_PLUGINS_OPTIONS_H_
+
+namespace chrome_checker {
+
+struct Options {
+  Options()
+      : check_base_classes(false),
+        check_virtuals_in_implementations(true),
+        check_weak_ptr_factory_order(false),
+        check_enum_last_value(false) {}
+
+  bool check_base_classes;
+  bool check_virtuals_in_implementations;
+  bool check_weak_ptr_factory_order;
+  bool check_enum_last_value;
+};
+
+}  // namespace chrome_checker
+
+#endif  // TOOLS_CLANG_PLUGINS_OPTIONS_H_