Remove unneeded check-weak-ptr-factory-order flag
[chromium-blink-merge.git] / tools / clang / plugins / FindBadConstructsConsumer.cpp
blobaffac561cca23e831963a94321a3681e2652784c
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file.
5 #include "FindBadConstructsConsumer.h"
7 #include "clang/Frontend/CompilerInstance.h"
8 #include "clang/AST/Attr.h"
9 #include "clang/Lex/Lexer.h"
10 #include "llvm/Support/raw_ostream.h"
12 using namespace clang;
14 namespace chrome_checker {
16 namespace {
18 const char kMethodRequiresOverride[] =
19 "[chromium-style] Overriding method must be marked with 'override' or "
20 "'final'.";
21 const char kRedundantVirtualSpecifier[] =
22 "[chromium-style] %0 is redundant; %1 implies %0.";
23 // http://llvm.org/bugs/show_bug.cgi?id=21051 has been filed to make this a
24 // Clang warning.
25 const char kBaseMethodVirtualAndFinal[] =
26 "[chromium-style] The virtual method does not override anything and is "
27 "final; consider making it non-virtual.";
28 const char kNoExplicitDtor[] =
29 "[chromium-style] Classes that are ref-counted should have explicit "
30 "destructors that are declared protected or private.";
31 const char kPublicDtor[] =
32 "[chromium-style] Classes that are ref-counted should have "
33 "destructors that are declared protected or private.";
34 const char kProtectedNonVirtualDtor[] =
35 "[chromium-style] Classes that are ref-counted and have non-private "
36 "destructors should declare their destructor virtual.";
37 const char kWeakPtrFactoryOrder[] =
38 "[chromium-style] WeakPtrFactory members which refer to their outer class "
39 "must be the last member in the outer class definition.";
40 const char kBadLastEnumValue[] =
41 "[chromium-style] _LAST/Last constants of enum types must have the maximal "
42 "value for any constant of that type.";
43 const char kNoteInheritance[] = "[chromium-style] %0 inherits from %1 here";
44 const char kNoteImplicitDtor[] =
45 "[chromium-style] No explicit destructor for %0 defined";
46 const char kNotePublicDtor[] =
47 "[chromium-style] Public destructor declared here";
48 const char kNoteProtectedNonVirtualDtor[] =
49 "[chromium-style] Protected non-virtual destructor declared here";
51 bool TypeHasNonTrivialDtor(const Type* type) {
52 if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
53 return !cxx_r->hasTrivialDestructor();
55 return false;
58 // Returns the underlying Type for |type| by expanding typedefs and removing
59 // any namespace qualifiers. This is similar to desugaring, except that for
60 // ElaboratedTypes, desugar will unwrap too much.
61 const Type* UnwrapType(const Type* type) {
62 if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
63 return UnwrapType(elaborated->getNamedType().getTypePtr());
64 if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
65 return UnwrapType(typedefed->desugar().getTypePtr());
66 return type;
69 bool IsGtestTestFixture(const CXXRecordDecl* decl) {
70 return decl->getQualifiedNameAsString() == "testing::Test";
73 FixItHint FixItRemovalForVirtual(const SourceManager& manager,
74 const CXXMethodDecl* method) {
75 // Unfortunately, there doesn't seem to be a good way to determine the
76 // location of the 'virtual' keyword. It's available in Declarator, but that
77 // isn't accessible from the AST. So instead, make an educated guess that the
78 // first token is probably the virtual keyword. Strictly speaking, this
79 // doesn't have to be true, but it probably will be.
80 // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
81 SourceRange range(method->getLocStart());
82 // Get the spelling loc just in case it was expanded from a macro.
83 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
84 // Sanity check that the text looks like virtual.
85 StringRef text = clang::Lexer::getSourceText(
86 CharSourceRange::getTokenRange(spelling_range), manager, LangOptions());
87 if (text.trim() != "virtual")
88 return FixItHint();
89 return FixItHint::CreateRemoval(range);
92 } // namespace
94 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
95 const Options& options)
96 : ChromeClassTester(instance), options_(options) {
97 // Messages for virtual method specifiers.
98 diag_method_requires_override_ =
99 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride);
100 diag_redundant_virtual_specifier_ =
101 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier);
102 diag_base_method_virtual_and_final_ =
103 diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal);
105 // Messages for destructors.
106 diag_no_explicit_dtor_ =
107 diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor);
108 diag_public_dtor_ =
109 diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor);
110 diag_protected_non_virtual_dtor_ =
111 diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor);
113 // Miscellaneous messages.
114 diag_weak_ptr_factory_order_ =
115 diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder);
116 diag_bad_enum_last_value_ =
117 diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue);
119 // Registers notes to make it easier to interpret warnings.
120 diag_note_inheritance_ =
121 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteInheritance);
122 diag_note_implicit_dtor_ =
123 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteImplicitDtor);
124 diag_note_public_dtor_ =
125 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNotePublicDtor);
126 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
127 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
130 bool FindBadConstructsConsumer::VisitDecl(clang::Decl* decl) {
131 clang::TagDecl* tag_decl = dyn_cast<clang::TagDecl>(decl);
132 if (tag_decl && tag_decl->isCompleteDefinition())
133 CheckTag(tag_decl);
134 return true;
137 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
138 CXXRecordDecl* record) {
139 bool implementation_file = InImplementationFile(record_location);
141 if (!implementation_file) {
142 // Only check for "heavy" constructors/destructors in header files;
143 // within implementation files, there is no performance cost.
144 CheckCtorDtorWeight(record_location, record);
147 bool warn_on_inline_bodies = !implementation_file;
149 // Check that all virtual methods are annotated with override or final.
150 CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
152 CheckRefCountedDtors(record_location, record);
154 CheckWeakPtrFactoryMembers(record_location, record);
157 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location,
158 EnumDecl* enum_decl) {
159 if (!options_.check_enum_last_value)
160 return;
162 bool got_one = false;
163 bool is_signed = false;
164 llvm::APSInt max_so_far;
165 EnumDecl::enumerator_iterator iter;
166 for (iter = enum_decl->enumerator_begin();
167 iter != enum_decl->enumerator_end();
168 ++iter) {
169 llvm::APSInt current_value = iter->getInitVal();
170 if (!got_one) {
171 max_so_far = current_value;
172 is_signed = current_value.isSigned();
173 got_one = true;
174 } else {
175 if (is_signed != current_value.isSigned()) {
176 // This only happens in some cases when compiling C (not C++) files,
177 // so it is OK to bail out here.
178 return;
180 if (current_value > max_so_far)
181 max_so_far = current_value;
184 for (iter = enum_decl->enumerator_begin();
185 iter != enum_decl->enumerator_end();
186 ++iter) {
187 std::string name = iter->getNameAsString();
188 if (((name.size() > 4 && name.compare(name.size() - 4, 4, "Last") == 0) ||
189 (name.size() > 5 && name.compare(name.size() - 5, 5, "_LAST") == 0)) &&
190 iter->getInitVal() < max_so_far) {
191 diagnostic().Report(iter->getLocation(), diag_bad_enum_last_value_);
196 void FindBadConstructsConsumer::CheckCtorDtorWeight(
197 SourceLocation record_location,
198 CXXRecordDecl* record) {
199 // We don't handle anonymous structs. If this record doesn't have a
200 // name, it's of the form:
202 // struct {
203 // ...
204 // } name_;
205 if (record->getIdentifier() == NULL)
206 return;
208 // Count the number of templated base classes as a feature of whether the
209 // destructor can be inlined.
210 int templated_base_classes = 0;
211 for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
212 it != record->bases_end();
213 ++it) {
214 if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
215 TypeLoc::TemplateSpecialization) {
216 ++templated_base_classes;
220 // Count the number of trivial and non-trivial member variables.
221 int trivial_member = 0;
222 int non_trivial_member = 0;
223 int templated_non_trivial_member = 0;
224 for (RecordDecl::field_iterator it = record->field_begin();
225 it != record->field_end();
226 ++it) {
227 CountType(it->getType().getTypePtr(),
228 &trivial_member,
229 &non_trivial_member,
230 &templated_non_trivial_member);
233 // Check to see if we need to ban inlined/synthesized constructors. Note
234 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
235 int dtor_score = 0;
236 // Deriving from a templated base class shouldn't be enough to trigger
237 // the ctor warning, but if you do *anything* else, it should.
239 // TODO(erg): This is motivated by templated base classes that don't have
240 // any data members. Somehow detect when templated base classes have data
241 // members and treat them differently.
242 dtor_score += templated_base_classes * 9;
243 // Instantiating a template is an insta-hit.
244 dtor_score += templated_non_trivial_member * 10;
245 // The fourth normal class member should trigger the warning.
246 dtor_score += non_trivial_member * 3;
248 int ctor_score = dtor_score;
249 // You should be able to have 9 ints before we warn you.
250 ctor_score += trivial_member;
252 if (ctor_score >= 10) {
253 if (!record->hasUserDeclaredConstructor()) {
254 emitWarning(record_location,
255 "Complex class/struct needs an explicit out-of-line "
256 "constructor.");
257 } else {
258 // Iterate across all the constructors in this file and yell if we
259 // find one that tries to be inline.
260 for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
261 it != record->ctor_end();
262 ++it) {
263 if (it->hasInlineBody()) {
264 if (it->isCopyConstructor() &&
265 !record->hasUserDeclaredCopyConstructor()) {
266 emitWarning(record_location,
267 "Complex class/struct needs an explicit out-of-line "
268 "copy constructor.");
269 } else {
270 emitWarning(it->getInnerLocStart(),
271 "Complex constructor has an inlined body.");
278 // The destructor side is equivalent except that we don't check for
279 // trivial members; 20 ints don't need a destructor.
280 if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
281 if (!record->hasUserDeclaredDestructor()) {
282 emitWarning(record_location,
283 "Complex class/struct needs an explicit out-of-line "
284 "destructor.");
285 } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
286 if (dtor->hasInlineBody()) {
287 emitWarning(dtor->getInnerLocStart(),
288 "Complex destructor has an inline body.");
294 bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) {
295 return GetNamespace(record).find("testing") != std::string::npos;
298 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
299 const CXXMethodDecl* method) {
300 if (InBannedNamespace(method))
301 return true;
302 for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
303 i != method->end_overridden_methods();
304 ++i) {
305 const CXXMethodDecl* overridden = *i;
306 if (IsMethodInBannedOrTestingNamespace(overridden) ||
307 // Provide an exception for ::testing::Test. gtest itself uses some
308 // magic to try to make sure SetUp()/TearDown() aren't capitalized
309 // incorrectly, but having the plugin enforce override is also nice.
310 (InTestingNamespace(overridden) &&
311 (!options_.strict_virtual_specifiers ||
312 !IsGtestTestFixture(overridden->getParent())))) {
313 return true;
317 return false;
320 // Checks that virtual methods are correctly annotated, and have no body in a
321 // header file.
322 void FindBadConstructsConsumer::CheckVirtualMethods(
323 SourceLocation record_location,
324 CXXRecordDecl* record,
325 bool warn_on_inline_bodies) {
326 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
327 // trick to get around that. If a class has member variables whose types are
328 // in the "testing" namespace (which is how gmock works behind the scenes),
329 // there's a really high chance we won't care about these errors
330 for (CXXRecordDecl::field_iterator it = record->field_begin();
331 it != record->field_end();
332 ++it) {
333 CXXRecordDecl* record_type = it->getTypeSourceInfo()
334 ->getTypeLoc()
335 .getTypePtr()
336 ->getAsCXXRecordDecl();
337 if (record_type) {
338 if (InTestingNamespace(record_type)) {
339 return;
344 for (CXXRecordDecl::method_iterator it = record->method_begin();
345 it != record->method_end();
346 ++it) {
347 if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
348 // Ignore constructors and assignment operators.
349 } else if (isa<CXXDestructorDecl>(*it) &&
350 !record->hasUserDeclaredDestructor()) {
351 // Ignore non-user-declared destructors.
352 } else if (!it->isVirtual()) {
353 continue;
354 } else {
355 CheckVirtualSpecifiers(*it);
356 if (warn_on_inline_bodies)
357 CheckVirtualBodies(*it);
362 // Makes sure that virtual methods use the most appropriate specifier. If a
363 // virtual method overrides a method from a base class, only the override
364 // specifier should be used. If the method should not be overridden by derived
365 // classes, only the final specifier should be used.
366 void FindBadConstructsConsumer::CheckVirtualSpecifiers(
367 const CXXMethodDecl* method) {
368 bool is_override = method->size_overridden_methods() > 0;
369 bool has_virtual = method->isVirtualAsWritten();
370 OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
371 FinalAttr* final_attr = method->getAttr<FinalAttr>();
373 if (method->isPure() && !options_.strict_virtual_specifiers)
374 return;
376 if (IsMethodInBannedOrTestingNamespace(method))
377 return;
379 if (isa<CXXDestructorDecl>(method) && !options_.strict_virtual_specifiers)
380 return;
382 SourceManager& manager = instance().getSourceManager();
384 // Complain if a method is annotated virtual && (override || final).
385 if (has_virtual && (override_attr || final_attr) &&
386 options_.strict_virtual_specifiers) {
387 diagnostic().Report(method->getLocStart(),
388 diag_redundant_virtual_specifier_)
389 << "'virtual'"
390 << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
391 << FixItRemovalForVirtual(manager, method);
394 // Complain if a method is an override and is not annotated with override or
395 // final.
396 if (is_override && !override_attr && !final_attr) {
397 SourceRange type_info_range =
398 method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
399 FullSourceLoc loc(type_info_range.getBegin(), manager);
401 // Build the FixIt insertion point after the end of the method definition,
402 // including any const-qualifiers and attributes, and before the opening
403 // of the l-curly-brace (if inline) or the semi-color (if a declaration).
404 SourceLocation spelling_end =
405 manager.getSpellingLoc(type_info_range.getEnd());
406 if (spelling_end.isValid()) {
407 SourceLocation token_end =
408 Lexer::getLocForEndOfToken(spelling_end, 0, manager, LangOptions());
409 diagnostic().Report(token_end, diag_method_requires_override_)
410 << FixItHint::CreateInsertion(token_end, " override");
411 } else {
412 diagnostic().Report(loc, diag_method_requires_override_);
416 if (final_attr && override_attr && options_.strict_virtual_specifiers) {
417 diagnostic().Report(override_attr->getLocation(),
418 diag_redundant_virtual_specifier_)
419 << override_attr << final_attr
420 << FixItHint::CreateRemoval(override_attr->getRange());
423 if (final_attr && !is_override && options_.strict_virtual_specifiers) {
424 diagnostic().Report(method->getLocStart(),
425 diag_base_method_virtual_and_final_)
426 << FixItRemovalForVirtual(manager, method)
427 << FixItHint::CreateRemoval(final_attr->getRange());
431 void FindBadConstructsConsumer::CheckVirtualBodies(
432 const CXXMethodDecl* method) {
433 // Virtual methods should not have inline definitions beyond "{}". This
434 // only matters for header files.
435 if (method->hasBody() && method->hasInlineBody()) {
436 if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
437 if (cs->size()) {
438 emitWarning(cs->getLBracLoc(),
439 "virtual methods with non-empty bodies shouldn't be "
440 "declared inline.");
446 void FindBadConstructsConsumer::CountType(const Type* type,
447 int* trivial_member,
448 int* non_trivial_member,
449 int* templated_non_trivial_member) {
450 switch (type->getTypeClass()) {
451 case Type::Record: {
452 // Simplifying; the whole class isn't trivial if the dtor is, but
453 // we use this as a signal about complexity.
454 if (TypeHasNonTrivialDtor(type))
455 (*trivial_member)++;
456 else
457 (*non_trivial_member)++;
458 break;
460 case Type::TemplateSpecialization: {
461 TemplateName name =
462 dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
463 bool whitelisted_template = false;
465 // HACK: I'm at a loss about how to get the syntax checker to get
466 // whether a template is exterened or not. For the first pass here,
467 // just do retarded string comparisons.
468 if (TemplateDecl* decl = name.getAsTemplateDecl()) {
469 std::string base_name = decl->getNameAsString();
470 if (base_name == "basic_string")
471 whitelisted_template = true;
474 if (whitelisted_template)
475 (*non_trivial_member)++;
476 else
477 (*templated_non_trivial_member)++;
478 break;
480 case Type::Elaborated: {
481 CountType(dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
482 trivial_member,
483 non_trivial_member,
484 templated_non_trivial_member);
485 break;
487 case Type::Typedef: {
488 while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
489 type = TT->getDecl()->getUnderlyingType().getTypePtr();
491 CountType(type,
492 trivial_member,
493 non_trivial_member,
494 templated_non_trivial_member);
495 break;
497 default: {
498 // Stupid assumption: anything we see that isn't the above is one of
499 // the 20 integer types.
500 (*trivial_member)++;
501 break;
506 // Check |record| for issues that are problematic for ref-counted types.
507 // Note that |record| may not be a ref-counted type, but a base class for
508 // a type that is.
509 // If there are issues, update |loc| with the SourceLocation of the issue
510 // and returns appropriately, or returns None if there are no issues.
511 FindBadConstructsConsumer::RefcountIssue
512 FindBadConstructsConsumer::CheckRecordForRefcountIssue(
513 const CXXRecordDecl* record,
514 SourceLocation& loc) {
515 if (!record->hasUserDeclaredDestructor()) {
516 loc = record->getLocation();
517 return ImplicitDestructor;
520 if (CXXDestructorDecl* dtor = record->getDestructor()) {
521 if (dtor->getAccess() == AS_public) {
522 loc = dtor->getInnerLocStart();
523 return PublicDestructor;
527 return None;
530 // Adds either a warning or error, based on the current handling of
531 // -Werror.
532 DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() {
533 return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
534 : DiagnosticsEngine::Warning;
537 // Returns true if |base| specifies one of the Chromium reference counted
538 // classes (base::RefCounted / base::RefCountedThreadSafe).
539 bool FindBadConstructsConsumer::IsRefCountedCallback(
540 const CXXBaseSpecifier* base,
541 CXXBasePath& path,
542 void* user_data) {
543 FindBadConstructsConsumer* self =
544 static_cast<FindBadConstructsConsumer*>(user_data);
546 const TemplateSpecializationType* base_type =
547 dyn_cast<TemplateSpecializationType>(
548 UnwrapType(base->getType().getTypePtr()));
549 if (!base_type) {
550 // Base-most definition is not a template, so this cannot derive from
551 // base::RefCounted. However, it may still be possible to use with a
552 // scoped_refptr<> and support ref-counting, so this is not a perfect
553 // guarantee of safety.
554 return false;
557 TemplateName name = base_type->getTemplateName();
558 if (TemplateDecl* decl = name.getAsTemplateDecl()) {
559 std::string base_name = decl->getNameAsString();
561 // Check for both base::RefCounted and base::RefCountedThreadSafe.
562 if (base_name.compare(0, 10, "RefCounted") == 0 &&
563 self->GetNamespace(decl) == "base") {
564 return true;
568 return false;
571 // Returns true if |base| specifies a class that has a public destructor,
572 // either explicitly or implicitly.
573 bool FindBadConstructsConsumer::HasPublicDtorCallback(
574 const CXXBaseSpecifier* base,
575 CXXBasePath& path,
576 void* user_data) {
577 // Only examine paths that have public inheritance, as they are the
578 // only ones which will result in the destructor potentially being
579 // exposed. This check is largely redundant, as Chromium code should be
580 // exclusively using public inheritance.
581 if (path.Access != AS_public)
582 return false;
584 CXXRecordDecl* record =
585 dyn_cast<CXXRecordDecl>(base->getType()->getAs<RecordType>()->getDecl());
586 SourceLocation unused;
587 return None != CheckRecordForRefcountIssue(record, unused);
590 // Outputs a C++ inheritance chain as a diagnostic aid.
591 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath& path) {
592 for (CXXBasePath::const_iterator it = path.begin(); it != path.end(); ++it) {
593 diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
594 << it->Class << it->Base->getType();
598 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue) {
599 switch (issue) {
600 case ImplicitDestructor:
601 return diag_no_explicit_dtor_;
602 case PublicDestructor:
603 return diag_public_dtor_;
604 case None:
605 assert(false && "Do not call DiagnosticForIssue with issue None");
606 return 0;
608 assert(false);
609 return 0;
612 // Check |record| to determine if it has any problematic refcounting
613 // issues and, if so, print them as warnings/errors based on the current
614 // value of getErrorLevel().
616 // If |record| is a C++ class, and if it inherits from one of the Chromium
617 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
618 // ensure that there are no public destructors in the class hierarchy. This
619 // is to guard against accidentally stack-allocating a RefCounted class or
620 // sticking it in a non-ref-counted container (like scoped_ptr<>).
621 void FindBadConstructsConsumer::CheckRefCountedDtors(
622 SourceLocation record_location,
623 CXXRecordDecl* record) {
624 // Skip anonymous structs.
625 if (record->getIdentifier() == NULL)
626 return;
628 // Determine if the current type is even ref-counted.
629 CXXBasePaths refcounted_path;
630 if (!record->lookupInBases(&FindBadConstructsConsumer::IsRefCountedCallback,
631 this,
632 refcounted_path)) {
633 return; // Class does not derive from a ref-counted base class.
636 // Easy check: Check to see if the current type is problematic.
637 SourceLocation loc;
638 RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
639 if (issue != None) {
640 diagnostic().Report(loc, DiagnosticForIssue(issue));
641 PrintInheritanceChain(refcounted_path.front());
642 return;
644 if (CXXDestructorDecl* dtor =
645 refcounted_path.begin()->back().Class->getDestructor()) {
646 if (dtor->getAccess() == AS_protected && !dtor->isVirtual()) {
647 loc = dtor->getInnerLocStart();
648 diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
649 return;
653 // Long check: Check all possible base classes for problematic
654 // destructors. This checks for situations involving multiple
655 // inheritance, where the ref-counted class may be implementing an
656 // interface that has a public or implicit destructor.
658 // struct SomeInterface {
659 // virtual void DoFoo();
660 // };
662 // struct RefCountedInterface
663 // : public base::RefCounted<RefCountedInterface>,
664 // public SomeInterface {
665 // private:
666 // friend class base::Refcounted<RefCountedInterface>;
667 // virtual ~RefCountedInterface() {}
668 // };
670 // While RefCountedInterface is "safe", in that its destructor is
671 // private, it's possible to do the following "unsafe" code:
672 // scoped_refptr<RefCountedInterface> some_class(
673 // new RefCountedInterface);
674 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
675 // delete static_cast<SomeInterface*>(some_class.get());
676 if (!options_.check_base_classes)
677 return;
679 // Find all public destructors. This will record the class hierarchy
680 // that leads to the public destructor in |dtor_paths|.
681 CXXBasePaths dtor_paths;
682 if (!record->lookupInBases(&FindBadConstructsConsumer::HasPublicDtorCallback,
683 this,
684 dtor_paths)) {
685 return;
688 for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
689 it != dtor_paths.end();
690 ++it) {
691 // The record with the problem will always be the last record
692 // in the path, since it is the record that stopped the search.
693 const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
694 it->back().Base->getType()->getAs<RecordType>()->getDecl());
696 issue = CheckRecordForRefcountIssue(problem_record, loc);
698 if (issue == ImplicitDestructor) {
699 diagnostic().Report(record_location, diag_no_explicit_dtor_);
700 PrintInheritanceChain(refcounted_path.front());
701 diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
702 PrintInheritanceChain(*it);
703 } else if (issue == PublicDestructor) {
704 diagnostic().Report(record_location, diag_public_dtor_);
705 PrintInheritanceChain(refcounted_path.front());
706 diagnostic().Report(loc, diag_note_public_dtor_);
707 PrintInheritanceChain(*it);
712 // Check for any problems with WeakPtrFactory class members. This currently
713 // only checks that any WeakPtrFactory<T> member of T appears as the last
714 // data member in T. We could consider checking for bad uses of
715 // WeakPtrFactory to refer to other data members, but that would require
716 // looking at the initializer list in constructors to see what the factory
717 // points to.
718 // Note, if we later add other unrelated checks of data members, we should
719 // consider collapsing them in to one loop to avoid iterating over the data
720 // members more than once.
721 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
722 SourceLocation record_location,
723 CXXRecordDecl* record) {
724 // Skip anonymous structs.
725 if (record->getIdentifier() == NULL)
726 return;
728 // Iterate through members of the class.
729 RecordDecl::field_iterator iter(record->field_begin()),
730 the_end(record->field_end());
731 SourceLocation weak_ptr_factory_location; // Invalid initially.
732 for (; iter != the_end; ++iter) {
733 const TemplateSpecializationType* template_spec_type =
734 iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
735 bool param_is_weak_ptr_factory_to_self = false;
736 if (template_spec_type) {
737 const TemplateDecl* template_decl =
738 template_spec_type->getTemplateName().getAsTemplateDecl();
739 if (template_decl && template_spec_type->getNumArgs() == 1) {
740 if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
741 GetNamespace(template_decl) == "base") {
742 // Only consider WeakPtrFactory members which are specialized for the
743 // owning class.
744 const TemplateArgument& arg = template_spec_type->getArg(0);
745 if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
746 record->getTypeForDecl()->getAsCXXRecordDecl()) {
747 if (!weak_ptr_factory_location.isValid()) {
748 // Save the first matching WeakPtrFactory member for the
749 // diagnostic.
750 weak_ptr_factory_location = iter->getLocation();
752 param_is_weak_ptr_factory_to_self = true;
757 // If we've already seen a WeakPtrFactory<OwningType> and this param is not
758 // one of those, it means there is at least one member after a factory.
759 if (weak_ptr_factory_location.isValid() &&
760 !param_is_weak_ptr_factory_to_self) {
761 diagnostic().Report(weak_ptr_factory_location,
762 diag_weak_ptr_factory_order_);
767 } // namespace chrome_checker