1031 lines
40 KiB
C++
1031 lines
40 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.
|
|
|
|
#include "FindBadConstructsConsumer.h"
|
|
|
|
#include "clang/Frontend/CompilerInstance.h"
|
|
#include "clang/AST/Attr.h"
|
|
#include "clang/Lex/Lexer.h"
|
|
#include "clang/Sema/Sema.h"
|
|
#include "llvm/Support/raw_ostream.h"
|
|
|
|
using namespace clang;
|
|
|
|
namespace chrome_checker {
|
|
|
|
namespace {
|
|
|
|
const char kMethodRequiresOverride[] =
|
|
"[chromium-style] Overriding method must be marked with 'override' or "
|
|
"'final'.";
|
|
const char kRedundantVirtualSpecifier[] =
|
|
"[chromium-style] %0 is redundant; %1 implies %0.";
|
|
// http://llvm.org/bugs/show_bug.cgi?id=21051 has been filed to make this a
|
|
// Clang warning.
|
|
const char kBaseMethodVirtualAndFinal[] =
|
|
"[chromium-style] The virtual method does not override anything and is "
|
|
"final; consider making it non-virtual.";
|
|
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 kAutoDeducedToAPointerType[] =
|
|
"[chromium-style] auto variable type must not deduce to a raw pointer "
|
|
"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";
|
|
|
|
// 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;
|
|
}
|
|
|
|
bool IsGtestTestFixture(const CXXRecordDecl* decl) {
|
|
return decl->getQualifiedNameAsString() == "testing::Test";
|
|
}
|
|
|
|
// Generates a fixit hint to remove the 'virtual' keyword.
|
|
// Unfortunately, there doesn't seem to be a good way to determine the source
|
|
// location of the 'virtual' keyword. It's available in Declarator, but that
|
|
// isn't accessible from the AST. So instead, make an educated guess that the
|
|
// first token is probably the virtual keyword. Strictly speaking, this doesn't
|
|
// have to be true, but it probably will be.
|
|
// TODO(dcheng): Add a warning to force virtual to always appear first ;-)
|
|
FixItHint FixItRemovalForVirtual(const SourceManager& manager,
|
|
const LangOptions& lang_opts,
|
|
const CXXMethodDecl* method) {
|
|
SourceRange range(method->getLocStart());
|
|
// Get the spelling loc just in case it was expanded from a macro.
|
|
SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
|
|
// Sanity check that the text looks like virtual.
|
|
StringRef text = clang::Lexer::getSourceText(
|
|
CharSourceRange::getTokenRange(spelling_range), manager, lang_opts);
|
|
if (text.trim() != "virtual")
|
|
return FixItHint();
|
|
return FixItHint::CreateRemoval(range);
|
|
}
|
|
|
|
bool IsPodOrTemplateType(const CXXRecordDecl& record) {
|
|
return record.isPOD() ||
|
|
record.getDescribedClassTemplate() ||
|
|
record.getTemplateSpecializationKind() ||
|
|
record.isDependentType();
|
|
}
|
|
|
|
// Use a local RAV implementation to simply collect all FunctionDecls marked for
|
|
// late template parsing. This happens with the flag -fdelayed-template-parsing,
|
|
// which is on by default in MSVC-compatible mode.
|
|
std::set<FunctionDecl*> GetLateParsedFunctionDecls(TranslationUnitDecl* decl) {
|
|
struct Visitor : public RecursiveASTVisitor<Visitor> {
|
|
bool VisitFunctionDecl(FunctionDecl* function_decl) {
|
|
if (function_decl->isLateTemplateParsed())
|
|
late_parsed_decls.insert(function_decl);
|
|
return true;
|
|
}
|
|
|
|
std::set<FunctionDecl*> late_parsed_decls;
|
|
} v;
|
|
v.TraverseDecl(decl);
|
|
return v.late_parsed_decls;
|
|
}
|
|
|
|
std::string GetAutoReplacementTypeAsString(QualType type) {
|
|
QualType non_reference_type = type.getNonReferenceType();
|
|
if (!non_reference_type->isPointerType())
|
|
return "auto";
|
|
|
|
std::string result =
|
|
GetAutoReplacementTypeAsString(non_reference_type->getPointeeType());
|
|
result += "*";
|
|
if (non_reference_type.isLocalConstQualified())
|
|
result += " const";
|
|
if (non_reference_type.isLocalVolatileQualified())
|
|
result += " volatile";
|
|
if (type->isReferenceType() && !non_reference_type.isLocalConstQualified()) {
|
|
if (type->isLValueReferenceType())
|
|
result += "&";
|
|
else if (type->isRValueReferenceType())
|
|
result += "&&";
|
|
}
|
|
return result;
|
|
}
|
|
|
|
} // namespace
|
|
|
|
FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
|
|
const Options& options)
|
|
: ChromeClassTester(instance, options) {
|
|
if (options.check_ipc) {
|
|
ipc_visitor_.reset(new CheckIPCVisitor(instance));
|
|
}
|
|
|
|
// Messages for virtual method specifiers.
|
|
diag_method_requires_override_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride);
|
|
diag_redundant_virtual_specifier_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier);
|
|
diag_base_method_virtual_and_final_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal);
|
|
|
|
// Messages for destructors.
|
|
diag_no_explicit_dtor_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor);
|
|
diag_public_dtor_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor);
|
|
diag_protected_non_virtual_dtor_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor);
|
|
|
|
// Miscellaneous messages.
|
|
diag_weak_ptr_factory_order_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder);
|
|
diag_bad_enum_last_value_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue);
|
|
diag_auto_deduced_to_a_pointer_type_ =
|
|
diagnostic().getCustomDiagID(getErrorLevel(), kAutoDeducedToAPointerType);
|
|
|
|
// Registers notes to make it easier to interpret warnings.
|
|
diag_note_inheritance_ =
|
|
diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteInheritance);
|
|
diag_note_implicit_dtor_ =
|
|
diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteImplicitDtor);
|
|
diag_note_public_dtor_ =
|
|
diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNotePublicDtor);
|
|
diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
|
|
DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
|
|
}
|
|
|
|
void FindBadConstructsConsumer::Traverse(ASTContext& context) {
|
|
if (ipc_visitor_) {
|
|
ipc_visitor_->set_context(&context);
|
|
ParseFunctionTemplates(context.getTranslationUnitDecl());
|
|
}
|
|
RecursiveASTVisitor::TraverseDecl(context.getTranslationUnitDecl());
|
|
if (ipc_visitor_) ipc_visitor_->set_context(nullptr);
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::TraverseDecl(Decl* decl) {
|
|
if (ipc_visitor_) ipc_visitor_->BeginDecl(decl);
|
|
bool result = RecursiveASTVisitor::TraverseDecl(decl);
|
|
if (ipc_visitor_) ipc_visitor_->EndDecl();
|
|
return result;
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::VisitTagDecl(clang::TagDecl* tag_decl) {
|
|
if (tag_decl->isCompleteDefinition())
|
|
CheckTag(tag_decl);
|
|
return true;
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::VisitTemplateSpecializationType(
|
|
TemplateSpecializationType* spec) {
|
|
if (ipc_visitor_) ipc_visitor_->VisitTemplateSpecializationType(spec);
|
|
return true;
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::VisitCallExpr(CallExpr* call_expr) {
|
|
if (ipc_visitor_) ipc_visitor_->VisitCallExpr(call_expr);
|
|
return true;
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::VisitVarDecl(clang::VarDecl* var_decl) {
|
|
CheckVarDecl(var_decl);
|
|
return true;
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
|
|
CXXRecordDecl* record) {
|
|
bool implementation_file = InImplementationFile(record_location);
|
|
|
|
if (!implementation_file) {
|
|
// Only check for "heavy" constructors/destructors in header files;
|
|
// within implementation files, there is no performance cost.
|
|
|
|
// If this is a POD or a class template or a type dependent on a
|
|
// templated class, assume there's no ctor/dtor/virtual method
|
|
// optimization that we should do.
|
|
if (!IsPodOrTemplateType(*record))
|
|
CheckCtorDtorWeight(record_location, record);
|
|
}
|
|
|
|
bool warn_on_inline_bodies = !implementation_file;
|
|
// Check that all virtual methods are annotated with override or final.
|
|
// Note this could also apply to templates, but for some reason Clang
|
|
// does not always see the "override", so we get false positives.
|
|
// See http://llvm.org/bugs/show_bug.cgi?id=18440 and
|
|
// http://llvm.org/bugs/show_bug.cgi?id=21942
|
|
if (!IsPodOrTemplateType(*record))
|
|
CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
|
|
|
|
CheckRefCountedDtors(record_location, record);
|
|
|
|
CheckWeakPtrFactoryMembers(record_location, record);
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location,
|
|
EnumDecl* enum_decl) {
|
|
if (!options_.check_enum_last_value)
|
|
return;
|
|
|
|
bool got_one = false;
|
|
bool is_signed = false;
|
|
llvm::APSInt max_so_far;
|
|
EnumDecl::enumerator_iterator iter;
|
|
for (iter = enum_decl->enumerator_begin();
|
|
iter != enum_decl->enumerator_end();
|
|
++iter) {
|
|
llvm::APSInt current_value = iter->getInitVal();
|
|
if (!got_one) {
|
|
max_so_far = current_value;
|
|
is_signed = current_value.isSigned();
|
|
got_one = true;
|
|
} else {
|
|
if (is_signed != current_value.isSigned()) {
|
|
// This only happens in some cases when compiling C (not C++) files,
|
|
// so it is OK to bail out here.
|
|
return;
|
|
}
|
|
if (current_value > max_so_far)
|
|
max_so_far = current_value;
|
|
}
|
|
}
|
|
for (iter = enum_decl->enumerator_begin();
|
|
iter != enum_decl->enumerator_end();
|
|
++iter) {
|
|
std::string name = iter->getNameAsString();
|
|
if (((name.size() > 4 && name.compare(name.size() - 4, 4, "Last") == 0) ||
|
|
(name.size() > 5 && name.compare(name.size() - 5, 5, "_LAST") == 0)) &&
|
|
iter->getInitVal() < max_so_far) {
|
|
diagnostic().Report(iter->getLocation(), diag_bad_enum_last_value_);
|
|
}
|
|
}
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CheckCtorDtorWeight(
|
|
SourceLocation record_location,
|
|
CXXRecordDecl* record) {
|
|
// We don't handle anonymous structs. If this record doesn't have a
|
|
// name, it's of the form:
|
|
//
|
|
// struct {
|
|
// ...
|
|
// } name_;
|
|
if (record->getIdentifier() == NULL)
|
|
return;
|
|
|
|
// We don't handle unions.
|
|
if (record->isUnion())
|
|
return;
|
|
|
|
// Skip records that derive from ignored base classes.
|
|
if (HasIgnoredBases(record))
|
|
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) {
|
|
// The current check is buggy. An implicit copy constructor does not
|
|
// have an inline body, so this check never fires for classes with a
|
|
// user-declared out-of-line constructor.
|
|
if (it->hasInlineBody()) {
|
|
if (it->isCopyConstructor() &&
|
|
!record->hasUserDeclaredCopyConstructor()) {
|
|
// In general, implicit constructors are generated on demand. But
|
|
// in the Windows component build, dllexport causes instantiation of
|
|
// the copy constructor which means that this fires on many more
|
|
// classes. For now, suppress this on dllexported classes.
|
|
// (This does mean that windows component builds will not emit this
|
|
// warning in some cases where it is emitted in other configs, but
|
|
// that's the better tradeoff at this point).
|
|
// TODO(dcheng): With the RecursiveASTVisitor, these warnings might
|
|
// be emitted on other platforms too, reevaluate if we want to keep
|
|
// surpressing this then http://crbug.com/467288
|
|
if (!record->hasAttr<DLLExportAttr>())
|
|
emitWarning(record_location,
|
|
"Complex class/struct needs an explicit out-of-line "
|
|
"copy constructor.");
|
|
} else {
|
|
// See the comment in the previous branch about copy constructors.
|
|
// This does the same for implicit move constructors.
|
|
bool is_likely_compiler_generated_dllexport_move_ctor =
|
|
it->isMoveConstructor() &&
|
|
!record->hasUserDeclaredMoveConstructor() &&
|
|
record->hasAttr<DLLExportAttr>();
|
|
if (!is_likely_compiler_generated_dllexport_move_ctor)
|
|
emitWarning(it->getInnerLocStart(),
|
|
"Complex constructor has an inlined body.");
|
|
}
|
|
} else if (it->isInlined() && !it->isInlineSpecified() &&
|
|
!it->isDeleted() && (!it->isCopyOrMoveConstructor() ||
|
|
it->isExplicitlyDefaulted())) {
|
|
// isInlined() is a more reliable check than hasInlineBody(), but
|
|
// unfortunately, it results in warnings for implicit copy/move
|
|
// constructors in the previously mentioned situation. To preserve
|
|
// compatibility with existing Chromium code, only warn if it's an
|
|
// explicitly defaulted copy or move constructor.
|
|
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->isInlined() && !dtor->isInlineSpecified() &&
|
|
!dtor->isDeleted()) {
|
|
emitWarning(dtor->getInnerLocStart(),
|
|
"Complex destructor has an inline body.");
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) {
|
|
return GetNamespace(record).find("testing") != std::string::npos;
|
|
}
|
|
|
|
bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
|
|
const CXXMethodDecl* method) {
|
|
if (InBannedNamespace(method))
|
|
return true;
|
|
for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
|
|
i != method->end_overridden_methods();
|
|
++i) {
|
|
const CXXMethodDecl* overridden = *i;
|
|
if (IsMethodInBannedOrTestingNamespace(overridden) ||
|
|
// Provide an exception for ::testing::Test. gtest itself uses some
|
|
// magic to try to make sure SetUp()/TearDown() aren't capitalized
|
|
// incorrectly, but having the plugin enforce override is also nice.
|
|
(InTestingNamespace(overridden) &&
|
|
!IsGtestTestFixture(overridden->getParent()))) {
|
|
return true;
|
|
}
|
|
}
|
|
|
|
return false;
|
|
}
|
|
|
|
SuppressibleDiagnosticBuilder
|
|
FindBadConstructsConsumer::ReportIfSpellingLocNotIgnored(
|
|
SourceLocation loc,
|
|
unsigned diagnostic_id) {
|
|
return SuppressibleDiagnosticBuilder(
|
|
&diagnostic(), loc, diagnostic_id,
|
|
InBannedDirectory(instance().getSourceManager().getSpellingLoc(loc)));
|
|
}
|
|
|
|
// Checks that virtual methods are correctly annotated, and have no body in a
|
|
// header file.
|
|
void FindBadConstructsConsumer::CheckVirtualMethods(
|
|
SourceLocation record_location,
|
|
CXXRecordDecl* record,
|
|
bool warn_on_inline_bodies) {
|
|
// 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
|
|
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 if (!it->isVirtual()) {
|
|
continue;
|
|
} else {
|
|
CheckVirtualSpecifiers(*it);
|
|
if (warn_on_inline_bodies)
|
|
CheckVirtualBodies(*it);
|
|
}
|
|
}
|
|
}
|
|
|
|
// Makes sure that virtual methods use the most appropriate specifier. If a
|
|
// virtual method overrides a method from a base class, only the override
|
|
// specifier should be used. If the method should not be overridden by derived
|
|
// classes, only the final specifier should be used.
|
|
void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
|
const CXXMethodDecl* method) {
|
|
bool is_override = method->size_overridden_methods() > 0;
|
|
bool has_virtual = method->isVirtualAsWritten();
|
|
OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
|
|
FinalAttr* final_attr = method->getAttr<FinalAttr>();
|
|
|
|
if (IsMethodInBannedOrTestingNamespace(method))
|
|
return;
|
|
|
|
SourceManager& manager = instance().getSourceManager();
|
|
const LangOptions& lang_opts = instance().getLangOpts();
|
|
|
|
// Complain if a method is annotated virtual && (override || final).
|
|
if (has_virtual && (override_attr || final_attr)) {
|
|
// ... but only if virtual does not originate in a macro from a banned file.
|
|
// Note this is just an educated guess: the assumption here is that any
|
|
// macro for declaring methods will probably be at the start of the method's
|
|
// source range.
|
|
ReportIfSpellingLocNotIgnored(method->getLocStart(),
|
|
diag_redundant_virtual_specifier_)
|
|
<< "'virtual'"
|
|
<< (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
|
|
<< FixItRemovalForVirtual(manager, lang_opts, method);
|
|
}
|
|
|
|
// Complain if a method is an override and is not annotated with override or
|
|
// final.
|
|
if (is_override && !override_attr && !final_attr) {
|
|
SourceRange range = method->getSourceRange();
|
|
SourceLocation loc;
|
|
if (method->hasInlineBody()) {
|
|
loc = method->getBody()->getSourceRange().getBegin();
|
|
} else {
|
|
loc = Lexer::getLocForEndOfToken(manager.getSpellingLoc(range.getEnd()),
|
|
0, manager, lang_opts);
|
|
// The original code used the ending source loc of TypeSourceInfo's
|
|
// TypeLoc. Unfortunately, this breaks down in the presence of attributes.
|
|
// Attributes often appear at the end of a TypeLoc, e.g.
|
|
// virtual ULONG __stdcall AddRef()
|
|
// has a TypeSourceInfo that looks something like:
|
|
// ULONG AddRef() __attribute(stdcall)
|
|
// so a fix-it insertion would be generated to insert 'override' after
|
|
// __stdcall in the code as written.
|
|
// While using the spelling loc of the CXXMethodDecl fixes attribute
|
|
// handling, it breaks handling of "= 0" and similar constructs.. To work
|
|
// around this, scan backwards in the source text for a '=' or ')' token
|
|
// and adjust the location as needed...
|
|
for (SourceLocation l = loc.getLocWithOffset(-1);
|
|
l != manager.getLocForStartOfFile(manager.getFileID(loc));
|
|
l = l.getLocWithOffset(-1)) {
|
|
l = Lexer::GetBeginningOfToken(l, manager, lang_opts);
|
|
Token token;
|
|
// getRawToken() returns *true* on failure. In that case, just give up
|
|
// and don't bother generating a possibly incorrect fix-it.
|
|
if (Lexer::getRawToken(l, token, manager, lang_opts, true)) {
|
|
loc = SourceLocation();
|
|
break;
|
|
}
|
|
if (token.is(tok::r_paren)) {
|
|
break;
|
|
} else if (token.is(tok::equal)) {
|
|
loc = l;
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
// Again, only emit the warning if it doesn't originate from a macro in
|
|
// a system header.
|
|
if (loc.isValid()) {
|
|
ReportIfSpellingLocNotIgnored(loc, diag_method_requires_override_)
|
|
<< FixItHint::CreateInsertion(loc, " override");
|
|
} else {
|
|
ReportIfSpellingLocNotIgnored(range.getBegin(),
|
|
diag_method_requires_override_);
|
|
}
|
|
}
|
|
|
|
if (final_attr && override_attr) {
|
|
ReportIfSpellingLocNotIgnored(override_attr->getLocation(),
|
|
diag_redundant_virtual_specifier_)
|
|
<< override_attr << final_attr
|
|
<< FixItHint::CreateRemoval(override_attr->getRange());
|
|
}
|
|
|
|
if (final_attr && !is_override) {
|
|
ReportIfSpellingLocNotIgnored(method->getLocStart(),
|
|
diag_base_method_virtual_and_final_)
|
|
<< FixItRemovalForVirtual(manager, lang_opts, method)
|
|
<< FixItHint::CreateRemoval(final_attr->getRange());
|
|
}
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CheckVirtualBodies(
|
|
const CXXMethodDecl* method) {
|
|
// Virtual methods should not have inline definitions beyond "{}". This
|
|
// only matters for header files.
|
|
if (method->hasBody() && method->hasInlineBody()) {
|
|
if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
|
|
if (cs->size()) {
|
|
SourceLocation loc = cs->getLBracLoc();
|
|
// CR_BEGIN_MSG_MAP_EX and BEGIN_SAFE_MSG_MAP_EX try to be compatible
|
|
// to BEGIN_MSG_MAP(_EX). So even though they are in chrome code,
|
|
// we can't easily fix them, so explicitly whitelist them here.
|
|
bool emit = true;
|
|
if (loc.isMacroID()) {
|
|
SourceManager& manager = instance().getSourceManager();
|
|
if (InBannedDirectory(manager.getSpellingLoc(loc)))
|
|
emit = false;
|
|
else {
|
|
StringRef name = Lexer::getImmediateMacroName(
|
|
loc, manager, instance().getLangOpts());
|
|
if (name == "CR_BEGIN_MSG_MAP_EX" ||
|
|
name == "BEGIN_SAFE_MSG_MAP_EX")
|
|
emit = false;
|
|
}
|
|
}
|
|
if (emit)
|
|
emitWarning(loc,
|
|
"virtual methods with non-empty bodies shouldn't be "
|
|
"declared inline.");
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CountType(const Type* type,
|
|
int* trivial_member,
|
|
int* non_trivial_member,
|
|
int* templated_non_trivial_member) {
|
|
switch (type->getTypeClass()) {
|
|
case Type::Record: {
|
|
auto* record_decl = type->getAsCXXRecordDecl();
|
|
// Simplifying; the whole class isn't trivial if the dtor is, but
|
|
// we use this as a signal about complexity.
|
|
// Note that if a record doesn't have a definition, it doesn't matter how
|
|
// it's counted, since the translation unit will fail to build. In that
|
|
// case, just count it as a trivial member to avoid emitting warnings that
|
|
// might be spurious.
|
|
if (!record_decl->hasDefinition() || record_decl->hasTrivialDestructor())
|
|
(*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 externed or not. For the first pass here,
|
|
// just do simple 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)) {
|
|
if (auto* decl = TT->getDecl()) {
|
|
const std::string name = decl->getNameAsString();
|
|
auto* context = decl->getDeclContext();
|
|
if (name == "atomic_int" && context->isStdNamespace()) {
|
|
(*trivial_member)++;
|
|
return;
|
|
}
|
|
type = decl->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 a POD
|
|
// or reference type.
|
|
(*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
|
|
FindBadConstructsConsumer::RefcountIssue
|
|
FindBadConstructsConsumer::CheckRecordForRefcountIssue(
|
|
const CXXRecordDecl* record,
|
|
SourceLocation& loc) {
|
|
if (!record->hasUserDeclaredDestructor()) {
|
|
loc = record->getLocation();
|
|
return ImplicitDestructor;
|
|
}
|
|
|
|
if (CXXDestructorDecl* dtor = record->getDestructor()) {
|
|
if (dtor->getAccess() == AS_public) {
|
|
loc = dtor->getInnerLocStart();
|
|
return PublicDestructor;
|
|
}
|
|
}
|
|
|
|
return None;
|
|
}
|
|
|
|
// Returns true if |base| specifies one of the Chromium reference counted
|
|
// classes (base::RefCounted / base::RefCountedThreadSafe).
|
|
bool FindBadConstructsConsumer::IsRefCounted(
|
|
const CXXBaseSpecifier* base,
|
|
CXXBasePath& path) {
|
|
FindBadConstructsConsumer* self = this;
|
|
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 FindBadConstructsConsumer::HasPublicDtorCallback(
|
|
const CXXBaseSpecifier* base,
|
|
CXXBasePath& path,
|
|
void* user_data) {
|
|
// Only examine paths that have public inheritance, as they are the
|
|
// only ones which will result in the destructor potentially being
|
|
// exposed. This check is largely redundant, as Chromium code should be
|
|
// exclusively using public inheritance.
|
|
if (path.Access != AS_public)
|
|
return false;
|
|
|
|
CXXRecordDecl* record =
|
|
dyn_cast<CXXRecordDecl>(base->getType()->getAs<RecordType>()->getDecl());
|
|
SourceLocation unused;
|
|
return None != CheckRecordForRefcountIssue(record, unused);
|
|
}
|
|
|
|
// Outputs a C++ inheritance chain as a diagnostic aid.
|
|
void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath& path) {
|
|
for (CXXBasePath::const_iterator it = path.begin(); it != path.end(); ++it) {
|
|
diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
|
|
<< it->Class << it->Base->getType();
|
|
}
|
|
}
|
|
|
|
unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue) {
|
|
switch (issue) {
|
|
case ImplicitDestructor:
|
|
return diag_no_explicit_dtor_;
|
|
case PublicDestructor:
|
|
return diag_public_dtor_;
|
|
case None:
|
|
assert(false && "Do not call DiagnosticForIssue with issue None");
|
|
return 0;
|
|
}
|
|
assert(false);
|
|
return 0;
|
|
}
|
|
|
|
// Check |record| to determine if it has any problematic refcounting
|
|
// issues and, if so, print them as warnings/errors based on the current
|
|
// value of getErrorLevel().
|
|
//
|
|
// If |record| is a C++ class, and if it inherits from one of the Chromium
|
|
// ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
|
|
// ensure that there are no public destructors in the class hierarchy. This
|
|
// is to guard against accidentally stack-allocating a RefCounted class or
|
|
// sticking it in a non-ref-counted container (like std::unique_ptr<>).
|
|
void FindBadConstructsConsumer::CheckRefCountedDtors(
|
|
SourceLocation record_location,
|
|
CXXRecordDecl* record) {
|
|
// Skip anonymous structs.
|
|
if (record->getIdentifier() == NULL)
|
|
return;
|
|
|
|
// Determine if the current type is even ref-counted.
|
|
CXXBasePaths refcounted_path;
|
|
if (!record->lookupInBases(
|
|
[this](const CXXBaseSpecifier* base, CXXBasePath& path) {
|
|
return IsRefCounted(base, path);
|
|
},
|
|
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(
|
|
[](const CXXBaseSpecifier* base, CXXBasePath& path) {
|
|
// TODO(thakis): Inline HasPublicDtorCallback() after clang roll.
|
|
return HasPublicDtorCallback(base, path, nullptr);
|
|
},
|
|
dtor_paths)) {
|
|
return;
|
|
}
|
|
|
|
for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
|
|
it != dtor_paths.end();
|
|
++it) {
|
|
// The record with the problem will always be the last record
|
|
// in the path, since it is the record that stopped the search.
|
|
const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
|
|
it->back().Base->getType()->getAs<RecordType>()->getDecl());
|
|
|
|
issue = CheckRecordForRefcountIssue(problem_record, loc);
|
|
|
|
if (issue == ImplicitDestructor) {
|
|
diagnostic().Report(record_location, diag_no_explicit_dtor_);
|
|
PrintInheritanceChain(refcounted_path.front());
|
|
diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
|
|
PrintInheritanceChain(*it);
|
|
} else if (issue == PublicDestructor) {
|
|
diagnostic().Report(record_location, diag_public_dtor_);
|
|
PrintInheritanceChain(refcounted_path.front());
|
|
diagnostic().Report(loc, diag_note_public_dtor_);
|
|
PrintInheritanceChain(*it);
|
|
}
|
|
}
|
|
}
|
|
|
|
// Check for any problems with WeakPtrFactory class members. This currently
|
|
// only checks that any WeakPtrFactory<T> member of T appears as the last
|
|
// data member in T. We could consider checking for bad uses of
|
|
// WeakPtrFactory to refer to other data members, but that would require
|
|
// looking at the initializer list in constructors to see what the factory
|
|
// points to.
|
|
// Note, if we later add other unrelated checks of data members, we should
|
|
// consider collapsing them in to one loop to avoid iterating over the data
|
|
// members more than once.
|
|
void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
|
|
SourceLocation record_location,
|
|
CXXRecordDecl* record) {
|
|
// Skip anonymous structs.
|
|
if (record->getIdentifier() == NULL)
|
|
return;
|
|
|
|
// Iterate through members of the class.
|
|
RecordDecl::field_iterator iter(record->field_begin()),
|
|
the_end(record->field_end());
|
|
SourceLocation weak_ptr_factory_location; // Invalid initially.
|
|
for (; iter != the_end; ++iter) {
|
|
const TemplateSpecializationType* template_spec_type =
|
|
iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
|
|
bool param_is_weak_ptr_factory_to_self = false;
|
|
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") {
|
|
// Only consider WeakPtrFactory members which are specialized for the
|
|
// owning class.
|
|
const TemplateArgument& arg = template_spec_type->getArg(0);
|
|
if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
|
|
record->getTypeForDecl()->getAsCXXRecordDecl()) {
|
|
if (!weak_ptr_factory_location.isValid()) {
|
|
// Save the first matching WeakPtrFactory member for the
|
|
// diagnostic.
|
|
weak_ptr_factory_location = iter->getLocation();
|
|
}
|
|
param_is_weak_ptr_factory_to_self = true;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
// If we've already seen a WeakPtrFactory<OwningType> and this param is not
|
|
// one of those, it means there is at least one member after a factory.
|
|
if (weak_ptr_factory_location.isValid() &&
|
|
!param_is_weak_ptr_factory_to_self) {
|
|
diagnostic().Report(weak_ptr_factory_location,
|
|
diag_weak_ptr_factory_order_);
|
|
}
|
|
}
|
|
}
|
|
|
|
// Copied from BlinkGCPlugin, see crrev.com/1135333007
|
|
void FindBadConstructsConsumer::ParseFunctionTemplates(
|
|
TranslationUnitDecl* decl) {
|
|
if (!instance().getLangOpts().DelayedTemplateParsing)
|
|
return; // Nothing to do.
|
|
|
|
std::set<FunctionDecl*> late_parsed_decls = GetLateParsedFunctionDecls(decl);
|
|
clang::Sema& sema = instance().getSema();
|
|
|
|
for (const FunctionDecl* fd : late_parsed_decls) {
|
|
assert(fd->isLateTemplateParsed());
|
|
|
|
if (instance().getSourceManager().isInSystemHeader(
|
|
instance().getSourceManager().getSpellingLoc(fd->getLocation())))
|
|
continue;
|
|
|
|
// Parse and build AST for yet-uninstantiated template functions.
|
|
clang::LateParsedTemplate* lpt = sema.LateParsedTemplateMap[fd].get();
|
|
sema.LateTemplateParser(sema.OpaqueParser, *lpt);
|
|
}
|
|
}
|
|
|
|
void FindBadConstructsConsumer::CheckVarDecl(clang::VarDecl* var_decl) {
|
|
if (!options_.check_auto_raw_pointer)
|
|
return;
|
|
|
|
// Check whether auto deduces to a raw pointer.
|
|
QualType non_reference_type = var_decl->getType().getNonReferenceType();
|
|
// We might have a case where the type is written as auto*, but the actual
|
|
// type is deduced to be an int**. For that reason, keep going down the
|
|
// pointee type until we get an 'auto' or a non-pointer type.
|
|
for (;;) {
|
|
const clang::AutoType* auto_type =
|
|
non_reference_type->getAs<clang::AutoType>();
|
|
if (auto_type) {
|
|
if (auto_type->isDeduced()) {
|
|
QualType deduced_type = auto_type->getDeducedType();
|
|
if (!deduced_type.isNull() && deduced_type->isPointerType() &&
|
|
!deduced_type->isFunctionPointerType()) {
|
|
// Check if we should even be considering this type (note that there
|
|
// should be fewer auto types than banned namespace/directory types,
|
|
// so check this last.
|
|
if (!InBannedNamespace(var_decl) &&
|
|
!InBannedDirectory(var_decl->getLocStart())) {
|
|
// The range starts from |var_decl|'s loc start, which is the
|
|
// beginning of the full expression defining this |var_decl|. It
|
|
// ends, however, where this |var_decl|'s type loc ends, since
|
|
// that's the end of the type of |var_decl|.
|
|
// Note that the beginning source location of type loc omits cv
|
|
// qualifiers, which is why it's not a good candidate to use for the
|
|
// start of the range.
|
|
clang::SourceRange range(
|
|
var_decl->getLocStart(),
|
|
var_decl->getTypeSourceInfo()->getTypeLoc().getLocEnd());
|
|
ReportIfSpellingLocNotIgnored(range.getBegin(),
|
|
diag_auto_deduced_to_a_pointer_type_)
|
|
<< FixItHint::CreateReplacement(
|
|
range,
|
|
GetAutoReplacementTypeAsString(var_decl->getType()));
|
|
}
|
|
}
|
|
}
|
|
} else if (non_reference_type->isPointerType()) {
|
|
non_reference_type = non_reference_type->getPointeeType();
|
|
continue;
|
|
}
|
|
break;
|
|
}
|
|
}
|
|
|
|
} // namespace chrome_checker
|