Home | History | Annotate | Download | only in plugins
      1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
      2 // Use of this source code is governed by a BSD-style license that can be
      3 // found in the LICENSE file.
      4 
      5 // This file defines a bunch of recurring problems in the Chromium C++ code.
      6 //
      7 // Checks that are implemented:
      8 // - Constructors/Destructors should not be inlined if they are of a complex
      9 //   class type.
     10 // - Missing "virtual" keywords on methods that should be virtual.
     11 // - Non-annotated overriding virtual methods.
     12 // - Virtual methods with nonempty implementations in their headers.
     13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
     14 //   should have protected or private destructors.
     15 // - WeakPtrFactory members that refer to their outer class should be the last
     16 //   member.
     17 
     18 #include "clang/AST/ASTConsumer.h"
     19 #include "clang/AST/AST.h"
     20 #include "clang/AST/Attr.h"
     21 #include "clang/AST/CXXInheritance.h"
     22 #include "clang/AST/TypeLoc.h"
     23 #include "clang/Basic/SourceManager.h"
     24 #include "clang/Frontend/CompilerInstance.h"
     25 #include "clang/Frontend/FrontendPluginRegistry.h"
     26 #include "clang/Lex/Lexer.h"
     27 #include "llvm/Support/raw_ostream.h"
     28 
     29 #include "ChromeClassTester.h"
     30 
     31 using namespace clang;
     32 
     33 namespace {
     34 
     35 const char kMethodRequiresOverride[] =
     36     "[chromium-style] Overriding method must be marked with OVERRIDE.";
     37 const char kMethodRequiresVirtual[] =
     38     "[chromium-style] Overriding method must have \"virtual\" keyword.";
     39 const char kNoExplicitDtor[] =
     40     "[chromium-style] Classes that are ref-counted should have explicit "
     41     "destructors that are declared protected or private.";
     42 const char kPublicDtor[] =
     43     "[chromium-style] Classes that are ref-counted should have "
     44     "destructors that are declared protected or private.";
     45 const char kProtectedNonVirtualDtor[] =
     46     "[chromium-style] Classes that are ref-counted and have non-private "
     47     "destructors should declare their destructor virtual.";
     48 const char kWeakPtrFactoryOrder[] =
     49     "[chromium-style] WeakPtrFactory members which refer to their outer class "
     50     "must be the last member in the outer class definition.";
     51 const char kNoteInheritance[] =
     52     "[chromium-style] %0 inherits from %1 here";
     53 const char kNoteImplicitDtor[] =
     54     "[chromium-style] No explicit destructor for %0 defined";
     55 const char kNotePublicDtor[] =
     56     "[chromium-style] Public destructor declared here";
     57 const char kNoteProtectedNonVirtualDtor[] =
     58     "[chromium-style] Protected non-virtual destructor declared here";
     59 
     60 bool TypeHasNonTrivialDtor(const Type* type) {
     61   if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
     62     return cxx_r->hasTrivialDestructor();
     63 
     64   return false;
     65 }
     66 
     67 // Returns the underlying Type for |type| by expanding typedefs and removing
     68 // any namespace qualifiers. This is similar to desugaring, except that for
     69 // ElaboratedTypes, desugar will unwrap too much.
     70 const Type* UnwrapType(const Type* type) {
     71   if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
     72     return UnwrapType(elaborated->getNamedType().getTypePtr());
     73   if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
     74     return UnwrapType(typedefed->desugar().getTypePtr());
     75   return type;
     76 }
     77 
     78 struct FindBadConstructsOptions {
     79   FindBadConstructsOptions() : check_base_classes(false),
     80                                check_virtuals_in_implementations(true),
     81                                check_url_directory(false),
     82                                check_weak_ptr_factory_order(false) {
     83   }
     84   bool check_base_classes;
     85   bool check_virtuals_in_implementations;
     86   bool check_url_directory;
     87   bool check_weak_ptr_factory_order;
     88 };
     89 
     90 // Searches for constructs that we know we don't want in the Chromium code base.
     91 class FindBadConstructsConsumer : public ChromeClassTester {
     92  public:
     93   FindBadConstructsConsumer(CompilerInstance& instance,
     94                             const FindBadConstructsOptions& options)
     95       : ChromeClassTester(instance, options.check_url_directory),
     96         options_(options) {
     97     // Register warning/error messages.
     98     diag_method_requires_override_ = diagnostic().getCustomDiagID(
     99         getErrorLevel(), kMethodRequiresOverride);
    100     diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
    101         getErrorLevel(), kMethodRequiresVirtual);
    102     diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
    103         getErrorLevel(), kNoExplicitDtor);
    104     diag_public_dtor_ = diagnostic().getCustomDiagID(
    105         getErrorLevel(), kPublicDtor);
    106     diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
    107         getErrorLevel(), kProtectedNonVirtualDtor);
    108     diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
    109         getErrorLevel(), kWeakPtrFactoryOrder);
    110 
    111     // Registers notes to make it easier to interpret warnings.
    112     diag_note_inheritance_ = diagnostic().getCustomDiagID(
    113         DiagnosticsEngine::Note, kNoteInheritance);
    114     diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
    115         DiagnosticsEngine::Note, kNoteImplicitDtor);
    116     diag_note_public_dtor_ = diagnostic().getCustomDiagID(
    117         DiagnosticsEngine::Note, kNotePublicDtor);
    118     diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
    119         DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
    120   }
    121 
    122   virtual void CheckChromeClass(SourceLocation record_location,
    123                                 CXXRecordDecl* record) {
    124     bool implementation_file = InImplementationFile(record_location);
    125 
    126     if (!implementation_file) {
    127       // Only check for "heavy" constructors/destructors in header files;
    128       // within implementation files, there is no performance cost.
    129       CheckCtorDtorWeight(record_location, record);
    130     }
    131 
    132     if (!implementation_file || options_.check_virtuals_in_implementations) {
    133       bool warn_on_inline_bodies = !implementation_file;
    134 
    135       // Check that all virtual methods are marked accordingly with both
    136       // virtual and OVERRIDE.
    137       CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
    138     }
    139 
    140     CheckRefCountedDtors(record_location, record);
    141 
    142     if (options_.check_weak_ptr_factory_order)
    143       CheckWeakPtrFactoryMembers(record_location, record);
    144   }
    145 
    146  private:
    147   // The type of problematic ref-counting pattern that was encountered.
    148   enum RefcountIssue {
    149     None,
    150     ImplicitDestructor,
    151     PublicDestructor
    152   };
    153 
    154   FindBadConstructsOptions options_;
    155 
    156   unsigned diag_method_requires_override_;
    157   unsigned diag_method_requires_virtual_;
    158   unsigned diag_no_explicit_dtor_;
    159   unsigned diag_public_dtor_;
    160   unsigned diag_protected_non_virtual_dtor_;
    161   unsigned diag_weak_ptr_factory_order_;
    162   unsigned diag_note_inheritance_;
    163   unsigned diag_note_implicit_dtor_;
    164   unsigned diag_note_public_dtor_;
    165   unsigned diag_note_protected_non_virtual_dtor_;
    166 
    167   // Prints errors if the constructor/destructor weight is too heavy.
    168   void CheckCtorDtorWeight(SourceLocation record_location,
    169                            CXXRecordDecl* record) {
    170     // We don't handle anonymous structs. If this record doesn't have a
    171     // name, it's of the form:
    172     //
    173     // struct {
    174     //   ...
    175     // } name_;
    176     if (record->getIdentifier() == NULL)
    177       return;
    178 
    179     // Count the number of templated base classes as a feature of whether the
    180     // destructor can be inlined.
    181     int templated_base_classes = 0;
    182     for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
    183          it != record->bases_end(); ++it) {
    184       if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
    185           TypeLoc::TemplateSpecialization) {
    186         ++templated_base_classes;
    187       }
    188     }
    189 
    190     // Count the number of trivial and non-trivial member variables.
    191     int trivial_member = 0;
    192     int non_trivial_member = 0;
    193     int templated_non_trivial_member = 0;
    194     for (RecordDecl::field_iterator it = record->field_begin();
    195          it != record->field_end(); ++it) {
    196       CountType(it->getType().getTypePtr(),
    197                 &trivial_member,
    198                 &non_trivial_member,
    199                 &templated_non_trivial_member);
    200     }
    201 
    202     // Check to see if we need to ban inlined/synthesized constructors. Note
    203     // that the cutoffs here are kind of arbitrary. Scores over 10 break.
    204     int dtor_score = 0;
    205     // Deriving from a templated base class shouldn't be enough to trigger
    206     // the ctor warning, but if you do *anything* else, it should.
    207     //
    208     // TODO(erg): This is motivated by templated base classes that don't have
    209     // any data members. Somehow detect when templated base classes have data
    210     // members and treat them differently.
    211     dtor_score += templated_base_classes * 9;
    212     // Instantiating a template is an insta-hit.
    213     dtor_score += templated_non_trivial_member * 10;
    214     // The fourth normal class member should trigger the warning.
    215     dtor_score += non_trivial_member * 3;
    216 
    217     int ctor_score = dtor_score;
    218     // You should be able to have 9 ints before we warn you.
    219     ctor_score += trivial_member;
    220 
    221     if (ctor_score >= 10) {
    222       if (!record->hasUserDeclaredConstructor()) {
    223         emitWarning(record_location,
    224                     "Complex class/struct needs an explicit out-of-line "
    225                     "constructor.");
    226       } else {
    227         // Iterate across all the constructors in this file and yell if we
    228         // find one that tries to be inline.
    229         for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
    230              it != record->ctor_end(); ++it) {
    231           if (it->hasInlineBody()) {
    232             if (it->isCopyConstructor() &&
    233                 !record->hasUserDeclaredCopyConstructor()) {
    234               emitWarning(record_location,
    235                           "Complex class/struct needs an explicit out-of-line "
    236                           "copy constructor.");
    237             } else {
    238               emitWarning(it->getInnerLocStart(),
    239                           "Complex constructor has an inlined body.");
    240             }
    241           }
    242         }
    243       }
    244     }
    245 
    246     // The destructor side is equivalent except that we don't check for
    247     // trivial members; 20 ints don't need a destructor.
    248     if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
    249       if (!record->hasUserDeclaredDestructor()) {
    250         emitWarning(
    251             record_location,
    252             "Complex class/struct needs an explicit out-of-line "
    253             "destructor.");
    254       } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
    255         if (dtor->hasInlineBody()) {
    256           emitWarning(dtor->getInnerLocStart(),
    257                       "Complex destructor has an inline body.");
    258         }
    259       }
    260     }
    261   }
    262 
    263   void CheckVirtualMethod(const CXXMethodDecl* method,
    264                           bool warn_on_inline_bodies) {
    265     if (!method->isVirtual())
    266       return;
    267 
    268     if (!method->isVirtualAsWritten()) {
    269       SourceLocation loc = method->getTypeSpecStartLoc();
    270       if (isa<CXXDestructorDecl>(method))
    271         loc = method->getInnerLocStart();
    272       SourceManager& manager = instance().getSourceManager();
    273       FullSourceLoc full_loc(loc, manager);
    274       SourceLocation spelling_loc = manager.getSpellingLoc(loc);
    275       diagnostic().Report(full_loc, diag_method_requires_virtual_)
    276           << FixItHint::CreateInsertion(spelling_loc, "virtual ");
    277     }
    278 
    279     // Virtual methods should not have inline definitions beyond "{}". This
    280     // only matters for header files.
    281     if (warn_on_inline_bodies && method->hasBody() &&
    282         method->hasInlineBody()) {
    283       if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
    284         if (cs->size()) {
    285           emitWarning(
    286               cs->getLBracLoc(),
    287               "virtual methods with non-empty bodies shouldn't be "
    288               "declared inline.");
    289         }
    290       }
    291     }
    292   }
    293 
    294   bool InTestingNamespace(const Decl* record) {
    295     return GetNamespace(record).find("testing") != std::string::npos;
    296   }
    297 
    298   bool IsMethodInBannedOrTestingNamespace(const CXXMethodDecl* method) {
    299     if (InBannedNamespace(method))
    300       return true;
    301     for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
    302          i != method->end_overridden_methods();
    303          ++i) {
    304       const CXXMethodDecl* overridden = *i;
    305       if (IsMethodInBannedOrTestingNamespace(overridden) ||
    306           InTestingNamespace(overridden)) {
    307         return true;
    308       }
    309     }
    310 
    311     return false;
    312   }
    313 
    314   void CheckOverriddenMethod(const CXXMethodDecl* method) {
    315     if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
    316       return;
    317 
    318     if (isa<CXXDestructorDecl>(method) || method->isPure())
    319       return;
    320 
    321     if (IsMethodInBannedOrTestingNamespace(method))
    322       return;
    323 
    324     SourceManager& manager = instance().getSourceManager();
    325     SourceRange type_info_range =
    326         method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
    327     FullSourceLoc loc(type_info_range.getBegin(), manager);
    328 
    329     // Build the FixIt insertion point after the end of the method definition,
    330     // including any const-qualifiers and attributes, and before the opening
    331     // of the l-curly-brace (if inline) or the semi-color (if a declaration).
    332     SourceLocation spelling_end =
    333         manager.getSpellingLoc(type_info_range.getEnd());
    334     if (spelling_end.isValid()) {
    335       SourceLocation token_end = Lexer::getLocForEndOfToken(
    336           spelling_end, 0, manager, LangOptions());
    337       diagnostic().Report(token_end, diag_method_requires_override_)
    338           << FixItHint::CreateInsertion(token_end, " OVERRIDE");
    339     } else {
    340       diagnostic().Report(loc, diag_method_requires_override_);
    341     }
    342   }
    343 
    344   // Makes sure there is a "virtual" keyword on virtual methods.
    345   //
    346   // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
    347   // trick to get around that. If a class has member variables whose types are
    348   // in the "testing" namespace (which is how gmock works behind the scenes),
    349   // there's a really high chance we won't care about these errors
    350   void CheckVirtualMethods(SourceLocation record_location,
    351                            CXXRecordDecl* record,
    352                            bool warn_on_inline_bodies) {
    353     for (CXXRecordDecl::field_iterator it = record->field_begin();
    354          it != record->field_end(); ++it) {
    355       CXXRecordDecl* record_type =
    356           it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
    357           getAsCXXRecordDecl();
    358       if (record_type) {
    359         if (InTestingNamespace(record_type)) {
    360           return;
    361         }
    362       }
    363     }
    364 
    365     for (CXXRecordDecl::method_iterator it = record->method_begin();
    366          it != record->method_end(); ++it) {
    367       if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
    368         // Ignore constructors and assignment operators.
    369       } else if (isa<CXXDestructorDecl>(*it) &&
    370           !record->hasUserDeclaredDestructor()) {
    371         // Ignore non-user-declared destructors.
    372       } else {
    373         CheckVirtualMethod(*it, warn_on_inline_bodies);
    374         CheckOverriddenMethod(*it);
    375       }
    376     }
    377   }
    378 
    379   void CountType(const Type* type,
    380                  int* trivial_member,
    381                  int* non_trivial_member,
    382                  int* templated_non_trivial_member) {
    383     switch (type->getTypeClass()) {
    384       case Type::Record: {
    385         // Simplifying; the whole class isn't trivial if the dtor is, but
    386         // we use this as a signal about complexity.
    387         if (TypeHasNonTrivialDtor(type))
    388           (*trivial_member)++;
    389         else
    390           (*non_trivial_member)++;
    391         break;
    392       }
    393       case Type::TemplateSpecialization: {
    394         TemplateName name =
    395             dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
    396         bool whitelisted_template = false;
    397 
    398         // HACK: I'm at a loss about how to get the syntax checker to get
    399         // whether a template is exterened or not. For the first pass here,
    400         // just do retarded string comparisons.
    401         if (TemplateDecl* decl = name.getAsTemplateDecl()) {
    402           std::string base_name = decl->getNameAsString();
    403           if (base_name == "basic_string")
    404             whitelisted_template = true;
    405         }
    406 
    407         if (whitelisted_template)
    408           (*non_trivial_member)++;
    409         else
    410           (*templated_non_trivial_member)++;
    411         break;
    412       }
    413       case Type::Elaborated: {
    414         CountType(
    415             dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
    416             trivial_member, non_trivial_member, templated_non_trivial_member);
    417         break;
    418       }
    419       case Type::Typedef: {
    420         while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
    421           type = TT->getDecl()->getUnderlyingType().getTypePtr();
    422         }
    423         CountType(type, trivial_member, non_trivial_member,
    424                   templated_non_trivial_member);
    425         break;
    426       }
    427       default: {
    428         // Stupid assumption: anything we see that isn't the above is one of
    429         // the 20 integer types.
    430         (*trivial_member)++;
    431         break;
    432       }
    433     }
    434   }
    435 
    436   // Check |record| for issues that are problematic for ref-counted types.
    437   // Note that |record| may not be a ref-counted type, but a base class for
    438   // a type that is.
    439   // If there are issues, update |loc| with the SourceLocation of the issue
    440   // and returns appropriately, or returns None if there are no issues.
    441   static RefcountIssue CheckRecordForRefcountIssue(
    442       const CXXRecordDecl* record,
    443       SourceLocation &loc) {
    444     if (!record->hasUserDeclaredDestructor()) {
    445       loc = record->getLocation();
    446       return ImplicitDestructor;
    447     }
    448 
    449     if (CXXDestructorDecl* dtor = record->getDestructor()) {
    450       if (dtor->getAccess() == AS_public) {
    451         loc = dtor->getInnerLocStart();
    452         return PublicDestructor;
    453       }
    454     }
    455 
    456     return None;
    457   }
    458 
    459   // Adds either a warning or error, based on the current handling of
    460   // -Werror.
    461   DiagnosticsEngine::Level getErrorLevel() {
    462     return diagnostic().getWarningsAsErrors() ?
    463         DiagnosticsEngine::Error : DiagnosticsEngine::Warning;
    464   }
    465 
    466   // Returns true if |base| specifies one of the Chromium reference counted
    467   // classes (base::RefCounted / base::RefCountedThreadSafe).
    468   static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
    469                                    CXXBasePath& path,
    470                                    void* user_data) {
    471     FindBadConstructsConsumer* self =
    472         static_cast<FindBadConstructsConsumer*>(user_data);
    473 
    474     const TemplateSpecializationType* base_type =
    475         dyn_cast<TemplateSpecializationType>(
    476             UnwrapType(base->getType().getTypePtr()));
    477     if (!base_type) {
    478       // Base-most definition is not a template, so this cannot derive from
    479       // base::RefCounted. However, it may still be possible to use with a
    480       // scoped_refptr<> and support ref-counting, so this is not a perfect
    481       // guarantee of safety.
    482       return false;
    483     }
    484 
    485     TemplateName name = base_type->getTemplateName();
    486     if (TemplateDecl* decl = name.getAsTemplateDecl()) {
    487       std::string base_name = decl->getNameAsString();
    488 
    489       // Check for both base::RefCounted and base::RefCountedThreadSafe.
    490       if (base_name.compare(0, 10, "RefCounted") == 0 &&
    491           self->GetNamespace(decl) == "base") {
    492         return true;
    493       }
    494     }
    495 
    496     return false;
    497   }
    498 
    499   // Returns true if |base| specifies a class that has a public destructor,
    500   // either explicitly or implicitly.
    501   static bool HasPublicDtorCallback(const CXXBaseSpecifier* base,
    502                                     CXXBasePath& path,
    503                                     void* user_data) {
    504     // Only examine paths that have public inheritance, as they are the
    505     // only ones which will result in the destructor potentially being
    506     // exposed. This check is largely redundant, as Chromium code should be
    507     // exclusively using public inheritance.
    508     if (path.Access != AS_public)
    509       return false;
    510 
    511     CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(
    512         base->getType()->getAs<RecordType>()->getDecl());
    513     SourceLocation unused;
    514     return None != CheckRecordForRefcountIssue(record, unused);
    515   }
    516 
    517   // Outputs a C++ inheritance chain as a diagnostic aid.
    518   void PrintInheritanceChain(const CXXBasePath& path) {
    519     for (CXXBasePath::const_iterator it = path.begin(); it != path.end();
    520          ++it) {
    521       diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
    522           << it->Class << it->Base->getType();
    523     }
    524   }
    525 
    526   unsigned DiagnosticForIssue(RefcountIssue issue) {
    527     switch (issue) {
    528       case ImplicitDestructor:
    529         return diag_no_explicit_dtor_;
    530       case PublicDestructor:
    531         return diag_public_dtor_;
    532       case None:
    533         assert(false && "Do not call DiagnosticForIssue with issue None");
    534         return 0;
    535     }
    536     assert(false);
    537     return 0;
    538   }
    539 
    540   // Check |record| to determine if it has any problematic refcounting
    541   // issues and, if so, print them as warnings/errors based on the current
    542   // value of getErrorLevel().
    543   //
    544   // If |record| is a C++ class, and if it inherits from one of the Chromium
    545   // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
    546   // ensure that there are no public destructors in the class hierarchy. This
    547   // is to guard against accidentally stack-allocating a RefCounted class or
    548   // sticking it in a non-ref-counted container (like scoped_ptr<>).
    549   void CheckRefCountedDtors(SourceLocation record_location,
    550                             CXXRecordDecl* record) {
    551     // Skip anonymous structs.
    552     if (record->getIdentifier() == NULL)
    553       return;
    554 
    555     // Determine if the current type is even ref-counted.
    556     CXXBasePaths refcounted_path;
    557     if (!record->lookupInBases(
    558             &FindBadConstructsConsumer::IsRefCountedCallback, this,
    559             refcounted_path)) {
    560       return;  // Class does not derive from a ref-counted base class.
    561     }
    562 
    563     // Easy check: Check to see if the current type is problematic.
    564     SourceLocation loc;
    565     RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
    566     if (issue != None) {
    567       diagnostic().Report(loc, DiagnosticForIssue(issue));
    568       PrintInheritanceChain(refcounted_path.front());
    569       return;
    570     }
    571     if (CXXDestructorDecl* dtor =
    572         refcounted_path.begin()->back().Class->getDestructor()) {
    573       if (dtor->getAccess() == AS_protected &&
    574           !dtor->isVirtual()) {
    575         loc = dtor->getInnerLocStart();
    576         diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
    577         return;
    578       }
    579     }
    580 
    581     // Long check: Check all possible base classes for problematic
    582     // destructors. This checks for situations involving multiple
    583     // inheritance, where the ref-counted class may be implementing an
    584     // interface that has a public or implicit destructor.
    585     //
    586     // struct SomeInterface {
    587     //   virtual void DoFoo();
    588     // };
    589     //
    590     // struct RefCountedInterface
    591     //    : public base::RefCounted<RefCountedInterface>,
    592     //      public SomeInterface {
    593     //  private:
    594     //   friend class base::Refcounted<RefCountedInterface>;
    595     //   virtual ~RefCountedInterface() {}
    596     // };
    597     //
    598     // While RefCountedInterface is "safe", in that its destructor is
    599     // private, it's possible to do the following "unsafe" code:
    600     //   scoped_refptr<RefCountedInterface> some_class(
    601     //       new RefCountedInterface);
    602     //   // Calls SomeInterface::~SomeInterface(), which is unsafe.
    603     //   delete static_cast<SomeInterface*>(some_class.get());
    604     if (!options_.check_base_classes)
    605       return;
    606 
    607     // Find all public destructors. This will record the class hierarchy
    608     // that leads to the public destructor in |dtor_paths|.
    609     CXXBasePaths dtor_paths;
    610     if (!record->lookupInBases(
    611             &FindBadConstructsConsumer::HasPublicDtorCallback, this,
    612             dtor_paths)) {
    613       return;
    614     }
    615 
    616     for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
    617          it != dtor_paths.end(); ++it) {
    618       // The record with the problem will always be the last record
    619       // in the path, since it is the record that stopped the search.
    620       const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
    621           it->back().Base->getType()->getAs<RecordType>()->getDecl());
    622 
    623       issue = CheckRecordForRefcountIssue(problem_record, loc);
    624 
    625       if (issue == ImplicitDestructor) {
    626         diagnostic().Report(record_location, diag_no_explicit_dtor_);
    627         PrintInheritanceChain(refcounted_path.front());
    628         diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
    629         PrintInheritanceChain(*it);
    630       } else if (issue == PublicDestructor) {
    631         diagnostic().Report(record_location, diag_public_dtor_);
    632         PrintInheritanceChain(refcounted_path.front());
    633         diagnostic().Report(loc, diag_note_public_dtor_);
    634         PrintInheritanceChain(*it);
    635       }
    636     }
    637   }
    638 
    639   // Check for any problems with WeakPtrFactory class members. This currently
    640   // only checks that any WeakPtrFactory<T> member of T appears as the last
    641   // data member in T. We could consider checking for bad uses of
    642   // WeakPtrFactory to refer to other data members, but that would require
    643   // looking at the initializer list in constructors to see what the factory
    644   // points to.
    645   // Note, if we later add other unrelated checks of data members, we should
    646   // consider collapsing them in to one loop to avoid iterating over the data
    647   // members more than once.
    648   void CheckWeakPtrFactoryMembers(SourceLocation record_location,
    649                                   CXXRecordDecl* record) {
    650     // Skip anonymous structs.
    651     if (record->getIdentifier() == NULL)
    652       return;
    653 
    654     // Iterate through members of the class.
    655     RecordDecl::field_iterator iter(record->field_begin()),
    656                                the_end(record->field_end());
    657     SourceLocation weak_ptr_factory_location;  // Invalid initially.
    658     for (; iter != the_end; ++iter) {
    659       // If we enter the loop but have already seen a matching WeakPtrFactory,
    660       // it means there is at least one member after the factory.
    661       if (weak_ptr_factory_location.isValid()) {
    662         diagnostic().Report(weak_ptr_factory_location,
    663                             diag_weak_ptr_factory_order_);
    664       }
    665       const TemplateSpecializationType* template_spec_type =
    666           iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
    667       if (template_spec_type) {
    668         const TemplateDecl* template_decl =
    669             template_spec_type->getTemplateName().getAsTemplateDecl();
    670         if (template_decl && template_spec_type->getNumArgs() >= 1) {
    671           if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
    672               GetNamespace(template_decl) == "base") {
    673             const TemplateArgument& arg = template_spec_type->getArg(0);
    674             if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
    675                 record->getTypeForDecl()->getAsCXXRecordDecl()) {
    676               weak_ptr_factory_location = iter->getLocation();
    677             }
    678           }
    679         }
    680       }
    681     }
    682   }
    683 };
    684 
    685 
    686 class FindBadConstructsAction : public PluginASTAction {
    687  public:
    688   FindBadConstructsAction() {
    689   }
    690 
    691  protected:
    692   // Overridden from PluginASTAction:
    693   virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
    694                                          llvm::StringRef ref) {
    695     return new FindBadConstructsConsumer(instance, options_);
    696   }
    697 
    698   virtual bool ParseArgs(const CompilerInstance& instance,
    699                          const std::vector<std::string>& args) {
    700     bool parsed = true;
    701 
    702     for (size_t i = 0; i < args.size() && parsed; ++i) {
    703       if (args[i] == "skip-virtuals-in-implementations") {
    704         // TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
    705         options_.check_virtuals_in_implementations = false;
    706       } else if (args[i] == "check-base-classes") {
    707         // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
    708         options_.check_base_classes = true;
    709       } else if (args[i] == "check-url-directory") {
    710         // TODO(tfarina): Remove this once http://crbug.com/229660 is fixed.
    711         options_.check_url_directory = true;
    712       } else if (args[i] == "check-weak-ptr-factory-order") {
    713         // TODO(dmichael): Remove this once http://crbug.com/303818 is fixed.
    714         options_.check_weak_ptr_factory_order = true;
    715       } else {
    716         parsed = false;
    717         llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
    718       }
    719     }
    720 
    721     return parsed;
    722   }
    723 
    724  private:
    725   FindBadConstructsOptions options_;
    726 };
    727 
    728 }  // namespace
    729 
    730 static FrontendPluginRegistry::Add<FindBadConstructsAction>
    731 X("find-bad-constructs", "Finds bad C++ constructs");
    732