Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(352)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: tools/clang/plugins/FindBadConstructs.cpp

Issue 348543002: Chrome clang plugin cleanup part 1 of ?? (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix fat finger deletion Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tools/clang/plugins/FindBadConstructsAction.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/clang/plugins/FindBadConstructs.cpp
diff --git a/tools/clang/plugins/FindBadConstructs.cpp b/tools/clang/plugins/FindBadConstructs.cpp
deleted file mode 100644
index 4b74fbe1f70902ec8b014346d60f22f16be73438..0000000000000000000000000000000000000000
--- 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");
« no previous file with comments | « no previous file | tools/clang/plugins/FindBadConstructsAction.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698