669 lines
25 KiB
C++
669 lines
25 KiB
C++
// 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.
|
|
|
|
#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 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;
|
|
}
|
|
|
|
// Searches for constructs that we know we don't want in the Chromium code base.
|
|
class FindBadConstructsConsumer : public ChromeClassTester {
|
|
public:
|
|
FindBadConstructsConsumer(CompilerInstance& instance,
|
|
bool check_base_classes,
|
|
bool check_virtuals_in_implementations,
|
|
bool check_url_directory)
|
|
: ChromeClassTester(instance, check_url_directory),
|
|
check_base_classes_(check_base_classes),
|
|
check_virtuals_in_implementations_(check_virtuals_in_implementations) {
|
|
// 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);
|
|
|
|
// 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 || 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);
|
|
}
|
|
|
|
private:
|
|
// The type of problematic ref-counting pattern that was encountered.
|
|
enum RefcountIssue {
|
|
None,
|
|
ImplicitDestructor,
|
|
PublicDestructor
|
|
};
|
|
|
|
bool check_base_classes_;
|
|
bool check_virtuals_in_implementations_;
|
|
|
|
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_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;
|
|
}
|
|
}
|
|
|
|
// 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 (!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);
|
|
}
|
|
}
|
|
}
|
|
};
|
|
|
|
class FindBadConstructsAction : public PluginASTAction {
|
|
public:
|
|
FindBadConstructsAction()
|
|
: check_base_classes_(false),
|
|
check_virtuals_in_implementations_(true),
|
|
check_url_directory_(false) {
|
|
}
|
|
|
|
protected:
|
|
// Overridden from PluginASTAction:
|
|
virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
|
|
llvm::StringRef ref) {
|
|
return new FindBadConstructsConsumer(
|
|
instance, check_base_classes_, check_virtuals_in_implementations_,
|
|
check_url_directory_);
|
|
}
|
|
|
|
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.
|
|
check_virtuals_in_implementations_ = false;
|
|
} else if (args[i] == "check-base-classes") {
|
|
// TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
|
|
check_base_classes_ = true;
|
|
} else if (args[i] == "check-url-directory") {
|
|
// TODO(tfarina): Remove this once http://crbug.com/229660 is fixed.
|
|
check_url_directory_ = true;
|
|
} else {
|
|
parsed = false;
|
|
llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
|
|
}
|
|
}
|
|
|
|
return parsed;
|
|
}
|
|
|
|
private:
|
|
bool check_base_classes_;
|
|
bool check_virtuals_in_implementations_;
|
|
bool check_url_directory_;
|
|
};
|
|
|
|
} // namespace
|
|
|
|
static FrontendPluginRegistry::Add<FindBadConstructsAction>
|
|
X("find-bad-constructs", "Finds bad C++ constructs");
|