michael@0: // Copyright (c) 2012 The Chromium Authors. All rights reserved.
michael@0: // Use of this source code is governed by a BSD-style license that can be
michael@0: // found in the LICENSE file.
michael@0: 
michael@0: // This file defines a bunch of recurring problems in the Chromium C++ code.
michael@0: //
michael@0: // Checks that are implemented:
michael@0: // - Constructors/Destructors should not be inlined if they are of a complex
michael@0: //   class type.
michael@0: // - Missing "virtual" keywords on methods that should be virtual.
michael@0: // - Non-annotated overriding virtual methods.
michael@0: // - Virtual methods with nonempty implementations in their headers.
michael@0: // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
michael@0: //   should have protected or private destructors.
michael@0: 
michael@0: #include "clang/Frontend/FrontendPluginRegistry.h"
michael@0: #include "clang/AST/ASTConsumer.h"
michael@0: #include "clang/AST/AST.h"
michael@0: #include "clang/AST/CXXInheritance.h"
michael@0: #include "clang/AST/TypeLoc.h"
michael@0: #include "clang/Basic/SourceManager.h"
michael@0: #include "clang/Frontend/CompilerInstance.h"
michael@0: #include "llvm/Support/raw_ostream.h"
michael@0: 
michael@0: #include "ChromeClassTester.h"
michael@0: 
michael@0: using namespace clang;
michael@0: 
michael@0: namespace {
michael@0: 
michael@0: bool TypeHasNonTrivialDtor(const Type* type) {
michael@0:   if (const CXXRecordDecl* cxx_r = type->getCXXRecordDeclForPointerType())
michael@0:     return cxx_r->hasTrivialDestructor();
michael@0: 
michael@0:   return false;
michael@0: }
michael@0: 
michael@0: // Returns the underlying Type for |type| by expanding typedefs and removing
michael@0: // any namespace qualifiers.
michael@0: const Type* UnwrapType(const Type* type) {
michael@0:   if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
michael@0:     return UnwrapType(elaborated->getNamedType().getTypePtr());
michael@0:   if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
michael@0:     return UnwrapType(typedefed->desugar().getTypePtr());
michael@0:   return type;
michael@0: }
michael@0: 
michael@0: // Searches for constructs that we know we don't want in the Chromium code base.
michael@0: class FindBadConstructsConsumer : public ChromeClassTester {
michael@0:  public:
michael@0:   FindBadConstructsConsumer(CompilerInstance& instance,
michael@0:                             bool check_refcounted_dtors,
michael@0:                             bool check_virtuals_in_implementations)
michael@0:       : ChromeClassTester(instance),
michael@0:         check_refcounted_dtors_(check_refcounted_dtors),
michael@0:         check_virtuals_in_implementations_(check_virtuals_in_implementations) {
michael@0:   }
michael@0: 
michael@0:   virtual void CheckChromeClass(SourceLocation record_location,
michael@0:                                 CXXRecordDecl* record) {
michael@0:     bool implementation_file = InImplementationFile(record_location);
michael@0: 
michael@0:     if (!implementation_file) {
michael@0:       // Only check for "heavy" constructors/destructors in header files;
michael@0:       // within implementation files, there is no performance cost.
michael@0:       CheckCtorDtorWeight(record_location, record);
michael@0:     }
michael@0: 
michael@0:     if (!implementation_file || check_virtuals_in_implementations_) {
michael@0:       bool warn_on_inline_bodies = !implementation_file;
michael@0: 
michael@0:       // Check that all virtual methods are marked accordingly with both
michael@0:       // virtual and OVERRIDE.
michael@0:       CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
michael@0:     }
michael@0: 
michael@0:     if (check_refcounted_dtors_)
michael@0:       CheckRefCountedDtors(record_location, record);
michael@0:   }
michael@0: 
michael@0:  private:
michael@0:   bool check_refcounted_dtors_;
michael@0:   bool check_virtuals_in_implementations_;
michael@0: 
michael@0:   // Returns true if |base| specifies one of the Chromium reference counted
michael@0:   // classes (base::RefCounted / base::RefCountedThreadSafe). |user_data| is
michael@0:   // ignored.
michael@0:   static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
michael@0:                                    CXXBasePath& path,
michael@0:                                    void* user_data) {
michael@0:     FindBadConstructsConsumer* self =
michael@0:         static_cast<FindBadConstructsConsumer*>(user_data);
michael@0: 
michael@0:     const TemplateSpecializationType* base_type =
michael@0:         dyn_cast<TemplateSpecializationType>(
michael@0:             UnwrapType(base->getType().getTypePtr()));
michael@0:     if (!base_type) {
michael@0:       // Base-most definition is not a template, so this cannot derive from
michael@0:       // base::RefCounted. However, it may still be possible to use with a
michael@0:       // scoped_refptr<> and support ref-counting, so this is not a perfect
michael@0:       // guarantee of safety.
michael@0:       return false;
michael@0:     }
michael@0: 
michael@0:     TemplateName name = base_type->getTemplateName();
michael@0:     if (TemplateDecl* decl = name.getAsTemplateDecl()) {
michael@0:       std::string base_name = decl->getNameAsString();
michael@0: 
michael@0:       // Check for both base::RefCounted and base::RefCountedThreadSafe.
michael@0:       if (base_name.compare(0, 10, "RefCounted") == 0 &&
michael@0:           self->GetNamespace(decl) == "base") {
michael@0:         return true;
michael@0:       }
michael@0:     }
michael@0:     return false;
michael@0:   }
michael@0: 
michael@0:   // Prints errors if the destructor of a RefCounted class is public.
michael@0:   void CheckRefCountedDtors(SourceLocation record_location,
michael@0:                             CXXRecordDecl* record) {
michael@0:     // Skip anonymous structs.
michael@0:     if (record->getIdentifier() == NULL)
michael@0:       return;
michael@0: 
michael@0:     CXXBasePaths paths;
michael@0:     if (!record->lookupInBases(
michael@0:             &FindBadConstructsConsumer::IsRefCountedCallback, this, paths)) {
michael@0:       return;  // Class does not derive from a ref-counted base class.
michael@0:     }
michael@0: 
michael@0:     if (!record->hasUserDeclaredDestructor()) {
michael@0:       emitWarning(
michael@0:           record_location,
michael@0:           "Classes that are ref-counted should have explicit "
michael@0:           "destructors that are protected or private.");
michael@0:     } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
michael@0:       if (dtor->getAccess() == AS_public) {
michael@0:         emitWarning(
michael@0:             dtor->getInnerLocStart(),
michael@0:             "Classes that are ref-counted should not have "
michael@0:             "public destructors.");
michael@0:       }
michael@0:     }
michael@0:   }
michael@0: 
michael@0:   // Prints errors if the constructor/destructor weight is too heavy.
michael@0:   void CheckCtorDtorWeight(SourceLocation record_location,
michael@0:                            CXXRecordDecl* record) {
michael@0:     // We don't handle anonymous structs. If this record doesn't have a
michael@0:     // name, it's of the form:
michael@0:     //
michael@0:     // struct {
michael@0:     //   ...
michael@0:     // } name_;
michael@0:     if (record->getIdentifier() == NULL)
michael@0:       return;
michael@0: 
michael@0:     // Count the number of templated base classes as a feature of whether the
michael@0:     // destructor can be inlined.
michael@0:     int templated_base_classes = 0;
michael@0:     for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
michael@0:          it != record->bases_end(); ++it) {
michael@0:       if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
michael@0:           TypeLoc::TemplateSpecialization) {
michael@0:         ++templated_base_classes;
michael@0:       }
michael@0:     }
michael@0: 
michael@0:     // Count the number of trivial and non-trivial member variables.
michael@0:     int trivial_member = 0;
michael@0:     int non_trivial_member = 0;
michael@0:     int templated_non_trivial_member = 0;
michael@0:     for (RecordDecl::field_iterator it = record->field_begin();
michael@0:          it != record->field_end(); ++it) {
michael@0:       CountType(it->getType().getTypePtr(),
michael@0:                 &trivial_member,
michael@0:                 &non_trivial_member,
michael@0:                 &templated_non_trivial_member);
michael@0:     }
michael@0: 
michael@0:     // Check to see if we need to ban inlined/synthesized constructors. Note
michael@0:     // that the cutoffs here are kind of arbitrary. Scores over 10 break.
michael@0:     int dtor_score = 0;
michael@0:     // Deriving from a templated base class shouldn't be enough to trigger
michael@0:     // the ctor warning, but if you do *anything* else, it should.
michael@0:     //
michael@0:     // TODO(erg): This is motivated by templated base classes that don't have
michael@0:     // any data members. Somehow detect when templated base classes have data
michael@0:     // members and treat them differently.
michael@0:     dtor_score += templated_base_classes * 9;
michael@0:     // Instantiating a template is an insta-hit.
michael@0:     dtor_score += templated_non_trivial_member * 10;
michael@0:     // The fourth normal class member should trigger the warning.
michael@0:     dtor_score += non_trivial_member * 3;
michael@0: 
michael@0:     int ctor_score = dtor_score;
michael@0:     // You should be able to have 9 ints before we warn you.
michael@0:     ctor_score += trivial_member;
michael@0: 
michael@0:     if (ctor_score >= 10) {
michael@0:       if (!record->hasUserDeclaredConstructor()) {
michael@0:         emitWarning(record_location,
michael@0:                     "Complex class/struct needs an explicit out-of-line "
michael@0:                     "constructor.");
michael@0:       } else {
michael@0:         // Iterate across all the constructors in this file and yell if we
michael@0:         // find one that tries to be inline.
michael@0:         for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
michael@0:              it != record->ctor_end(); ++it) {
michael@0:           if (it->hasInlineBody()) {
michael@0:             if (it->isCopyConstructor() &&
michael@0:                 !record->hasUserDeclaredCopyConstructor()) {
michael@0:               emitWarning(record_location,
michael@0:                           "Complex class/struct needs an explicit out-of-line "
michael@0:                           "copy constructor.");
michael@0:             } else {
michael@0:               emitWarning(it->getInnerLocStart(),
michael@0:                           "Complex constructor has an inlined body.");
michael@0:             }
michael@0:           }
michael@0:         }
michael@0:       }
michael@0:     }
michael@0: 
michael@0:     // The destructor side is equivalent except that we don't check for
michael@0:     // trivial members; 20 ints don't need a destructor.
michael@0:     if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
michael@0:       if (!record->hasUserDeclaredDestructor()) {
michael@0:         emitWarning(
michael@0:             record_location,
michael@0:             "Complex class/struct needs an explicit out-of-line "
michael@0:             "destructor.");
michael@0:       } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
michael@0:         if (dtor->hasInlineBody()) {
michael@0:           emitWarning(dtor->getInnerLocStart(),
michael@0:                       "Complex destructor has an inline body.");
michael@0:         }
michael@0:       }
michael@0:     }
michael@0:   }
michael@0: 
michael@0:   void CheckVirtualMethod(const CXXMethodDecl* method,
michael@0:                           bool warn_on_inline_bodies) {
michael@0:     if (!method->isVirtual())
michael@0:       return;
michael@0: 
michael@0:     if (!method->isVirtualAsWritten()) {
michael@0:       SourceLocation loc = method->getTypeSpecStartLoc();
michael@0:       if (isa<CXXDestructorDecl>(method))
michael@0:         loc = method->getInnerLocStart();
michael@0:       emitWarning(loc, "Overriding method must have \"virtual\" keyword.");
michael@0:     }
michael@0: 
michael@0:     // Virtual methods should not have inline definitions beyond "{}". This
michael@0:     // only matters for header files.
michael@0:     if (warn_on_inline_bodies && method->hasBody() &&
michael@0:         method->hasInlineBody()) {
michael@0:       if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
michael@0:         if (cs->size()) {
michael@0:           emitWarning(
michael@0:               cs->getLBracLoc(),
michael@0:               "virtual methods with non-empty bodies shouldn't be "
michael@0:               "declared inline.");
michael@0:         }
michael@0:       }
michael@0:     }
michael@0:   }
michael@0: 
michael@0:   bool InTestingNamespace(const Decl* record) {
michael@0:     return GetNamespace(record).find("testing") != std::string::npos;
michael@0:   }
michael@0: 
michael@0:   bool IsMethodInBannedNamespace(const CXXMethodDecl* method) {
michael@0:     if (InBannedNamespace(method))
michael@0:       return true;
michael@0:     for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
michael@0:          i != method->end_overridden_methods();
michael@0:          ++i) {
michael@0:       const CXXMethodDecl* overridden = *i;
michael@0:       if (IsMethodInBannedNamespace(overridden))
michael@0:         return true;
michael@0:     }
michael@0: 
michael@0:     return false;
michael@0:   }
michael@0: 
michael@0:   void CheckOverriddenMethod(const CXXMethodDecl* method) {
michael@0:     if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
michael@0:       return;
michael@0: 
michael@0:     if (isa<CXXDestructorDecl>(method) || method->isPure())
michael@0:       return;
michael@0: 
michael@0:     if (IsMethodInBannedNamespace(method))
michael@0:       return;
michael@0: 
michael@0:     SourceLocation loc = method->getTypeSpecStartLoc();
michael@0:     emitWarning(loc, "Overriding method must be marked with OVERRIDE.");
michael@0:   }
michael@0: 
michael@0:   // Makes sure there is a "virtual" keyword on virtual methods.
michael@0:   //
michael@0:   // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
michael@0:   // trick to get around that. If a class has member variables whose types are
michael@0:   // in the "testing" namespace (which is how gmock works behind the scenes),
michael@0:   // there's a really high chance we won't care about these errors
michael@0:   void CheckVirtualMethods(SourceLocation record_location,
michael@0:                            CXXRecordDecl* record,
michael@0:                            bool warn_on_inline_bodies) {
michael@0:     for (CXXRecordDecl::field_iterator it = record->field_begin();
michael@0:          it != record->field_end(); ++it) {
michael@0:       CXXRecordDecl* record_type =
michael@0:           it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
michael@0:           getAsCXXRecordDecl();
michael@0:       if (record_type) {
michael@0:         if (InTestingNamespace(record_type)) {
michael@0:           return;
michael@0:         }
michael@0:       }
michael@0:     }
michael@0: 
michael@0:     for (CXXRecordDecl::method_iterator it = record->method_begin();
michael@0:          it != record->method_end(); ++it) {
michael@0:       if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
michael@0:         // Ignore constructors and assignment operators.
michael@0:       } else if (isa<CXXDestructorDecl>(*it) &&
michael@0:           !record->hasUserDeclaredDestructor()) {
michael@0:         // Ignore non-user-declared destructors.
michael@0:       } else {
michael@0:         CheckVirtualMethod(*it, warn_on_inline_bodies);
michael@0:         CheckOverriddenMethod(*it);
michael@0:       }
michael@0:     }
michael@0:   }
michael@0: 
michael@0:   void CountType(const Type* type,
michael@0:                  int* trivial_member,
michael@0:                  int* non_trivial_member,
michael@0:                  int* templated_non_trivial_member) {
michael@0:     switch (type->getTypeClass()) {
michael@0:       case Type::Record: {
michael@0:         // Simplifying; the whole class isn't trivial if the dtor is, but
michael@0:         // we use this as a signal about complexity.
michael@0:         if (TypeHasNonTrivialDtor(type))
michael@0:           (*trivial_member)++;
michael@0:         else
michael@0:           (*non_trivial_member)++;
michael@0:         break;
michael@0:       }
michael@0:       case Type::TemplateSpecialization: {
michael@0:         TemplateName name =
michael@0:             dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
michael@0:         bool whitelisted_template = false;
michael@0: 
michael@0:         // HACK: I'm at a loss about how to get the syntax checker to get
michael@0:         // whether a template is exterened or not. For the first pass here,
michael@0:         // just do retarded string comparisons.
michael@0:         if (TemplateDecl* decl = name.getAsTemplateDecl()) {
michael@0:           std::string base_name = decl->getNameAsString();
michael@0:           if (base_name == "basic_string")
michael@0:             whitelisted_template = true;
michael@0:         }
michael@0: 
michael@0:         if (whitelisted_template)
michael@0:           (*non_trivial_member)++;
michael@0:         else
michael@0:           (*templated_non_trivial_member)++;
michael@0:         break;
michael@0:       }
michael@0:       case Type::Elaborated: {
michael@0:         CountType(
michael@0:             dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
michael@0:             trivial_member, non_trivial_member, templated_non_trivial_member);
michael@0:         break;
michael@0:       }
michael@0:       case Type::Typedef: {
michael@0:         while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
michael@0:           type = TT->getDecl()->getUnderlyingType().getTypePtr();
michael@0:         }
michael@0:         CountType(type, trivial_member, non_trivial_member,
michael@0:                   templated_non_trivial_member);
michael@0:         break;
michael@0:       }
michael@0:       default: {
michael@0:         // Stupid assumption: anything we see that isn't the above is one of
michael@0:         // the 20 integer types.
michael@0:         (*trivial_member)++;
michael@0:         break;
michael@0:       }
michael@0:     }
michael@0:   }
michael@0: };
michael@0: 
michael@0: class FindBadConstructsAction : public PluginASTAction {
michael@0:  public:
michael@0:   FindBadConstructsAction()
michael@0:       : check_refcounted_dtors_(true),
michael@0:         check_virtuals_in_implementations_(true) {
michael@0:   }
michael@0: 
michael@0:  protected:
michael@0:   // Overridden from PluginASTAction:
michael@0:   virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
michael@0:                                          llvm::StringRef ref) {
michael@0:     return new FindBadConstructsConsumer(
michael@0:         instance, check_refcounted_dtors_, check_virtuals_in_implementations_);
michael@0:   }
michael@0: 
michael@0:   virtual bool ParseArgs(const CompilerInstance& instance,
michael@0:                          const std::vector<std::string>& args) {
michael@0:     bool parsed = true;
michael@0: 
michael@0:     for (size_t i = 0; i < args.size() && parsed; ++i) {
michael@0:       if (args[i] == "skip-refcounted-dtors") {
michael@0:         check_refcounted_dtors_ = false;
michael@0:       } else if (args[i] == "skip-virtuals-in-implementations") {
michael@0:         check_virtuals_in_implementations_ = false;
michael@0:       } else {
michael@0:         parsed = false;
michael@0:         llvm::errs() << "Unknown argument: " << args[i] << "\n";
michael@0:       }
michael@0:     }
michael@0: 
michael@0:     return parsed;
michael@0:   }
michael@0: 
michael@0:  private:
michael@0:   bool check_refcounted_dtors_;
michael@0:   bool check_virtuals_in_implementations_;
michael@0: };
michael@0: 
michael@0: }  // namespace
michael@0: 
michael@0: static FrontendPluginRegistry::Add<FindBadConstructsAction>
michael@0: X("find-bad-constructs", "Finds bad C++ constructs");