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) |