| Index: tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| index 46e1df851b5358c2b31e1da669c8eaf0fec60314..fe868c01de6cab10389a46321e50f0f328bd0e0a 100644
|
| --- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| +++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
|
| @@ -283,6 +283,9 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
|
| 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()) {
|
| @@ -293,6 +296,15 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
|
| emitWarning(it->getInnerLocStart(),
|
| "Complex constructor has an inlined body.");
|
| }
|
| + } else if (it->isInlined() && (!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.");
|
| }
|
| }
|
| }
|
| @@ -306,7 +318,7 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
|
| "Complex class/struct needs an explicit out-of-line "
|
| "destructor.");
|
| } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
|
| - if (dtor->hasInlineBody()) {
|
| + if (dtor->isInlined()) {
|
| emitWarning(dtor->getInnerLocStart(),
|
| "Complex destructor has an inline body.");
|
| }
|
| @@ -331,8 +343,7 @@ bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
|
| // magic to try to make sure SetUp()/TearDown() aren't capitalized
|
| // incorrectly, but having the plugin enforce override is also nice.
|
| (InTestingNamespace(overridden) &&
|
| - (!options_.strict_virtual_specifiers ||
|
| - !IsGtestTestFixture(overridden->getParent())))) {
|
| + !IsGtestTestFixture(overridden->getParent()))) {
|
| return true;
|
| }
|
| }
|
| @@ -393,20 +404,13 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
|
| FinalAttr* final_attr = method->getAttr<FinalAttr>();
|
|
|
| - if (method->isPure() && !options_.strict_virtual_specifiers)
|
| - return;
|
| -
|
| if (IsMethodInBannedOrTestingNamespace(method))
|
| return;
|
|
|
| - if (isa<CXXDestructorDecl>(method) && !options_.strict_virtual_specifiers)
|
| - return;
|
| -
|
| SourceManager& manager = instance().getSourceManager();
|
|
|
| // Complain if a method is annotated virtual && (override || final).
|
| - if (has_virtual && (override_attr || final_attr) &&
|
| - options_.strict_virtual_specifiers) {
|
| + if (has_virtual && (override_attr || final_attr)) {
|
| diagnostic().Report(method->getLocStart(),
|
| diag_redundant_virtual_specifier_)
|
| << "'virtual'"
|
| @@ -436,14 +440,14 @@ void FindBadConstructsConsumer::CheckVirtualSpecifiers(
|
| }
|
| }
|
|
|
| - if (final_attr && override_attr && options_.strict_virtual_specifiers) {
|
| + if (final_attr && override_attr) {
|
| diagnostic().Report(override_attr->getLocation(),
|
| diag_redundant_virtual_specifier_)
|
| << override_attr << final_attr
|
| << FixItHint::CreateRemoval(override_attr->getRange());
|
| }
|
|
|
| - if (final_attr && !is_override && options_.strict_virtual_specifiers) {
|
| + if (final_attr && !is_override) {
|
| diagnostic().Report(method->getLocStart(),
|
| diag_base_method_virtual_and_final_)
|
| << FixItRemovalForVirtual(manager, method)
|
|
|