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

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

Issue 862133002: Update from https://crrev.com/312398 (Closed) Base URL: git@github.com:domokit/mojo.git@master
Patch Set: Created 5 years, 11 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
Index: tools/clang/plugins/FindBadConstructsConsumer.cpp
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
index 4f281f1da0364df84b512ce544dd0c52c68dda1d..46e1df851b5358c2b31e1da669c8eaf0fec60314 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -89,6 +89,13 @@ FixItHint FixItRemovalForVirtual(const SourceManager& manager,
return FixItHint::CreateRemoval(range);
}
+bool IsPodOrTemplateType(const CXXRecordDecl& record) {
+ return record.isPOD() ||
+ record.getDescribedClassTemplate() ||
+ record.getTemplateSpecializationKind() ||
+ record.isDependentType();
+}
+
} // namespace
FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
@@ -136,23 +143,38 @@ bool FindBadConstructsConsumer::VisitDecl(clang::Decl* decl) {
void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
CXXRecordDecl* record) {
+ // By default, the clang checker doesn't check some types (templates, etc).
+ // That was only a mistake; once Chromium code passes these checks, we should
+ // remove the "check-templates" option and remove this code.
+ // See crbug.com/441916
+ if (!options_.check_templates && IsPodOrTemplateType(*record))
+ return;
+
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 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.
- CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
+ // 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);
- if (options_.check_weak_ptr_factory_order)
- CheckWeakPtrFactoryMembers(record_location, record);
+ CheckWeakPtrFactoryMembers(record_location, record);
}
void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location,

Powered by Google App Engine
This is Rietveld 408576698