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
{
18 const char kMethodRequiresOverride
[] =
19 "[chromium-style] Overriding method must be marked with 'override' or "
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
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();
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());
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")
89 return FixItHint::CreateRemoval(range
);
92 bool IsPodOrTemplateType(const CXXRecordDecl
& record
) {
93 return record
.isPOD() ||
94 record
.getDescribedClassTemplate() ||
95 record
.getTemplateSpecializationKind() ||
96 record
.isDependentType();
101 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance
& instance
,
102 const Options
& options
)
103 : ChromeClassTester(instance
), options_(options
) {
104 // Messages for virtual method specifiers.
105 diag_method_requires_override_
=
106 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride
);
107 diag_redundant_virtual_specifier_
=
108 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier
);
109 diag_base_method_virtual_and_final_
=
110 diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal
);
112 // Messages for destructors.
113 diag_no_explicit_dtor_
=
114 diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor
);
116 diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor
);
117 diag_protected_non_virtual_dtor_
=
118 diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor
);
120 // Miscellaneous messages.
121 diag_weak_ptr_factory_order_
=
122 diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder
);
123 diag_bad_enum_last_value_
=
124 diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue
);
126 // Registers notes to make it easier to interpret warnings.
127 diag_note_inheritance_
=
128 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteInheritance
);
129 diag_note_implicit_dtor_
=
130 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteImplicitDtor
);
131 diag_note_public_dtor_
=
132 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNotePublicDtor
);
133 diag_note_protected_non_virtual_dtor_
= diagnostic().getCustomDiagID(
134 DiagnosticsEngine::Note
, kNoteProtectedNonVirtualDtor
);
137 bool FindBadConstructsConsumer::VisitDecl(clang::Decl
* decl
) {
138 clang::TagDecl
* tag_decl
= dyn_cast
<clang::TagDecl
>(decl
);
139 if (tag_decl
&& tag_decl
->isCompleteDefinition())
144 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location
,
145 CXXRecordDecl
* record
) {
146 // By default, the clang checker doesn't check some types (templates, etc).
147 // That was only a mistake; once Chromium code passes these checks, we should
148 // remove the "check-templates" option and remove this code.
149 // See crbug.com/441916
150 if (!options_
.check_templates
&& IsPodOrTemplateType(*record
))
153 bool implementation_file
= InImplementationFile(record_location
);
155 if (!implementation_file
) {
156 // Only check for "heavy" constructors/destructors in header files;
157 // within implementation files, there is no performance cost.
159 // If this is a POD or a class template or a type dependent on a
160 // templated class, assume there's no ctor/dtor/virtual method
161 // optimization that we should do.
162 if (!IsPodOrTemplateType(*record
))
163 CheckCtorDtorWeight(record_location
, record
);
166 bool warn_on_inline_bodies
= !implementation_file
;
167 // Check that all virtual methods are annotated with override or final.
168 // Note this could also apply to templates, but for some reason Clang
169 // does not always see the "override", so we get false positives.
170 // See http://llvm.org/bugs/show_bug.cgi?id=18440 and
171 // http://llvm.org/bugs/show_bug.cgi?id=21942
172 if (!IsPodOrTemplateType(*record
))
173 CheckVirtualMethods(record_location
, record
, warn_on_inline_bodies
);
175 CheckRefCountedDtors(record_location
, record
);
177 CheckWeakPtrFactoryMembers(record_location
, record
);
180 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location
,
181 EnumDecl
* enum_decl
) {
182 if (!options_
.check_enum_last_value
)
185 bool got_one
= false;
186 bool is_signed
= false;
187 llvm::APSInt max_so_far
;
188 EnumDecl::enumerator_iterator iter
;
189 for (iter
= enum_decl
->enumerator_begin();
190 iter
!= enum_decl
->enumerator_end();
192 llvm::APSInt current_value
= iter
->getInitVal();
194 max_so_far
= current_value
;
195 is_signed
= current_value
.isSigned();
198 if (is_signed
!= current_value
.isSigned()) {
199 // This only happens in some cases when compiling C (not C++) files,
200 // so it is OK to bail out here.
203 if (current_value
> max_so_far
)
204 max_so_far
= current_value
;
207 for (iter
= enum_decl
->enumerator_begin();
208 iter
!= enum_decl
->enumerator_end();
210 std::string name
= iter
->getNameAsString();
211 if (((name
.size() > 4 && name
.compare(name
.size() - 4, 4, "Last") == 0) ||
212 (name
.size() > 5 && name
.compare(name
.size() - 5, 5, "_LAST") == 0)) &&
213 iter
->getInitVal() < max_so_far
) {
214 diagnostic().Report(iter
->getLocation(), diag_bad_enum_last_value_
);
219 void FindBadConstructsConsumer::CheckCtorDtorWeight(
220 SourceLocation record_location
,
221 CXXRecordDecl
* record
) {
222 // We don't handle anonymous structs. If this record doesn't have a
223 // name, it's of the form:
228 if (record
->getIdentifier() == NULL
)
231 // Count the number of templated base classes as a feature of whether the
232 // destructor can be inlined.
233 int templated_base_classes
= 0;
234 for (CXXRecordDecl::base_class_const_iterator it
= record
->bases_begin();
235 it
!= record
->bases_end();
237 if (it
->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
238 TypeLoc::TemplateSpecialization
) {
239 ++templated_base_classes
;
243 // Count the number of trivial and non-trivial member variables.
244 int trivial_member
= 0;
245 int non_trivial_member
= 0;
246 int templated_non_trivial_member
= 0;
247 for (RecordDecl::field_iterator it
= record
->field_begin();
248 it
!= record
->field_end();
250 CountType(it
->getType().getTypePtr(),
253 &templated_non_trivial_member
);
256 // Check to see if we need to ban inlined/synthesized constructors. Note
257 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
259 // Deriving from a templated base class shouldn't be enough to trigger
260 // the ctor warning, but if you do *anything* else, it should.
262 // TODO(erg): This is motivated by templated base classes that don't have
263 // any data members. Somehow detect when templated base classes have data
264 // members and treat them differently.
265 dtor_score
+= templated_base_classes
* 9;
266 // Instantiating a template is an insta-hit.
267 dtor_score
+= templated_non_trivial_member
* 10;
268 // The fourth normal class member should trigger the warning.
269 dtor_score
+= non_trivial_member
* 3;
271 int ctor_score
= dtor_score
;
272 // You should be able to have 9 ints before we warn you.
273 ctor_score
+= trivial_member
;
275 if (ctor_score
>= 10) {
276 if (!record
->hasUserDeclaredConstructor()) {
277 emitWarning(record_location
,
278 "Complex class/struct needs an explicit out-of-line "
281 // Iterate across all the constructors in this file and yell if we
282 // find one that tries to be inline.
283 for (CXXRecordDecl::ctor_iterator it
= record
->ctor_begin();
284 it
!= record
->ctor_end();
286 // The current check is buggy. An implicit copy constructor does not
287 // have an inline body, so this check never fires for classes with a
288 // user-declared out-of-line constructor.
289 if (it
->hasInlineBody()) {
290 if (it
->isCopyConstructor() &&
291 !record
->hasUserDeclaredCopyConstructor()) {
292 emitWarning(record_location
,
293 "Complex class/struct needs an explicit out-of-line "
294 "copy constructor.");
296 emitWarning(it
->getInnerLocStart(),
297 "Complex constructor has an inlined body.");
299 } else if (it
->isInlined() && !it
->isInlineSpecified() &&
300 !it
->isDeleted() && (!it
->isCopyOrMoveConstructor() ||
301 it
->isExplicitlyDefaulted())) {
302 // isInlined() is a more reliable check than hasInlineBody(), but
303 // unfortunately, it results in warnings for implicit copy/move
304 // constructors in the previously mentioned situation. To preserve
305 // compatibility with existing Chromium code, only warn if it's an
306 // explicitly defaulted copy or move constructor.
307 emitWarning(it
->getInnerLocStart(),
308 "Complex constructor has an inlined body.");
314 // The destructor side is equivalent except that we don't check for
315 // trivial members; 20 ints don't need a destructor.
316 if (dtor_score
>= 10 && !record
->hasTrivialDestructor()) {
317 if (!record
->hasUserDeclaredDestructor()) {
318 emitWarning(record_location
,
319 "Complex class/struct needs an explicit out-of-line "
321 } else if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
322 if (dtor
->isInlined() && !dtor
->isInlineSpecified() &&
323 !dtor
->isDeleted()) {
324 emitWarning(dtor
->getInnerLocStart(),
325 "Complex destructor has an inline body.");
331 bool FindBadConstructsConsumer::InTestingNamespace(const Decl
* record
) {
332 return GetNamespace(record
).find("testing") != std::string::npos
;
335 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
336 const CXXMethodDecl
* method
) {
337 if (InBannedNamespace(method
))
339 for (CXXMethodDecl::method_iterator i
= method
->begin_overridden_methods();
340 i
!= method
->end_overridden_methods();
342 const CXXMethodDecl
* overridden
= *i
;
343 if (IsMethodInBannedOrTestingNamespace(overridden
) ||
344 // Provide an exception for ::testing::Test. gtest itself uses some
345 // magic to try to make sure SetUp()/TearDown() aren't capitalized
346 // incorrectly, but having the plugin enforce override is also nice.
347 (InTestingNamespace(overridden
) &&
348 !IsGtestTestFixture(overridden
->getParent()))) {
356 // Checks that virtual methods are correctly annotated, and have no body in a
358 void FindBadConstructsConsumer::CheckVirtualMethods(
359 SourceLocation record_location
,
360 CXXRecordDecl
* record
,
361 bool warn_on_inline_bodies
) {
362 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
363 // trick to get around that. If a class has member variables whose types are
364 // in the "testing" namespace (which is how gmock works behind the scenes),
365 // there's a really high chance we won't care about these errors
366 for (CXXRecordDecl::field_iterator it
= record
->field_begin();
367 it
!= record
->field_end();
369 CXXRecordDecl
* record_type
= it
->getTypeSourceInfo()
372 ->getAsCXXRecordDecl();
374 if (InTestingNamespace(record_type
)) {
380 for (CXXRecordDecl::method_iterator it
= record
->method_begin();
381 it
!= record
->method_end();
383 if (it
->isCopyAssignmentOperator() || isa
<CXXConstructorDecl
>(*it
)) {
384 // Ignore constructors and assignment operators.
385 } else if (isa
<CXXDestructorDecl
>(*it
) &&
386 !record
->hasUserDeclaredDestructor()) {
387 // Ignore non-user-declared destructors.
388 } else if (!it
->isVirtual()) {
391 CheckVirtualSpecifiers(*it
);
392 if (warn_on_inline_bodies
)
393 CheckVirtualBodies(*it
);
398 // Makes sure that virtual methods use the most appropriate specifier. If a
399 // virtual method overrides a method from a base class, only the override
400 // specifier should be used. If the method should not be overridden by derived
401 // classes, only the final specifier should be used.
402 void FindBadConstructsConsumer::CheckVirtualSpecifiers(
403 const CXXMethodDecl
* method
) {
404 bool is_override
= method
->size_overridden_methods() > 0;
405 bool has_virtual
= method
->isVirtualAsWritten();
406 OverrideAttr
* override_attr
= method
->getAttr
<OverrideAttr
>();
407 FinalAttr
* final_attr
= method
->getAttr
<FinalAttr
>();
409 if (IsMethodInBannedOrTestingNamespace(method
))
412 SourceManager
& manager
= instance().getSourceManager();
414 // Complain if a method is annotated virtual && (override || final).
415 if (has_virtual
&& (override_attr
|| final_attr
)) {
416 diagnostic().Report(method
->getLocStart(),
417 diag_redundant_virtual_specifier_
)
419 << (override_attr
? static_cast<Attr
*>(override_attr
) : final_attr
)
420 << FixItRemovalForVirtual(manager
, method
);
423 // Complain if a method is an override and is not annotated with override or
425 if (is_override
&& !override_attr
&& !final_attr
) {
426 SourceRange type_info_range
=
427 method
->getTypeSourceInfo()->getTypeLoc().getSourceRange();
428 FullSourceLoc
loc(type_info_range
.getBegin(), manager
);
430 // Build the FixIt insertion point after the end of the method definition,
431 // including any const-qualifiers and attributes, and before the opening
432 // of the l-curly-brace (if inline) or the semi-color (if a declaration).
433 SourceLocation spelling_end
=
434 manager
.getSpellingLoc(type_info_range
.getEnd());
435 if (spelling_end
.isValid()) {
436 SourceLocation token_end
=
437 Lexer::getLocForEndOfToken(spelling_end
, 0, manager
, LangOptions());
438 diagnostic().Report(token_end
, diag_method_requires_override_
)
439 << FixItHint::CreateInsertion(token_end
, " override");
441 diagnostic().Report(loc
, diag_method_requires_override_
);
445 if (final_attr
&& override_attr
) {
446 diagnostic().Report(override_attr
->getLocation(),
447 diag_redundant_virtual_specifier_
)
448 << override_attr
<< final_attr
449 << FixItHint::CreateRemoval(override_attr
->getRange());
452 if (final_attr
&& !is_override
) {
453 diagnostic().Report(method
->getLocStart(),
454 diag_base_method_virtual_and_final_
)
455 << FixItRemovalForVirtual(manager
, method
)
456 << FixItHint::CreateRemoval(final_attr
->getRange());
460 void FindBadConstructsConsumer::CheckVirtualBodies(
461 const CXXMethodDecl
* method
) {
462 // Virtual methods should not have inline definitions beyond "{}". This
463 // only matters for header files.
464 if (method
->hasBody() && method
->hasInlineBody()) {
465 if (CompoundStmt
* cs
= dyn_cast
<CompoundStmt
>(method
->getBody())) {
467 emitWarning(cs
->getLBracLoc(),
468 "virtual methods with non-empty bodies shouldn't be "
475 void FindBadConstructsConsumer::CountType(const Type
* type
,
477 int* non_trivial_member
,
478 int* templated_non_trivial_member
) {
479 switch (type
->getTypeClass()) {
481 // Simplifying; the whole class isn't trivial if the dtor is, but
482 // we use this as a signal about complexity.
483 if (TypeHasNonTrivialDtor(type
))
486 (*non_trivial_member
)++;
489 case Type::TemplateSpecialization
: {
491 dyn_cast
<TemplateSpecializationType
>(type
)->getTemplateName();
492 bool whitelisted_template
= false;
494 // HACK: I'm at a loss about how to get the syntax checker to get
495 // whether a template is exterened or not. For the first pass here,
496 // just do retarded string comparisons.
497 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
498 std::string base_name
= decl
->getNameAsString();
499 if (base_name
== "basic_string")
500 whitelisted_template
= true;
503 if (whitelisted_template
)
504 (*non_trivial_member
)++;
506 (*templated_non_trivial_member
)++;
509 case Type::Elaborated
: {
510 CountType(dyn_cast
<ElaboratedType
>(type
)->getNamedType().getTypePtr(),
513 templated_non_trivial_member
);
516 case Type::Typedef
: {
517 while (const TypedefType
* TT
= dyn_cast
<TypedefType
>(type
)) {
518 type
= TT
->getDecl()->getUnderlyingType().getTypePtr();
523 templated_non_trivial_member
);
527 // Stupid assumption: anything we see that isn't the above is one of
528 // the 20 integer types.
535 // Check |record| for issues that are problematic for ref-counted types.
536 // Note that |record| may not be a ref-counted type, but a base class for
538 // If there are issues, update |loc| with the SourceLocation of the issue
539 // and returns appropriately, or returns None if there are no issues.
540 FindBadConstructsConsumer::RefcountIssue
541 FindBadConstructsConsumer::CheckRecordForRefcountIssue(
542 const CXXRecordDecl
* record
,
543 SourceLocation
& loc
) {
544 if (!record
->hasUserDeclaredDestructor()) {
545 loc
= record
->getLocation();
546 return ImplicitDestructor
;
549 if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
550 if (dtor
->getAccess() == AS_public
) {
551 loc
= dtor
->getInnerLocStart();
552 return PublicDestructor
;
559 // Adds either a warning or error, based on the current handling of
561 DiagnosticsEngine::Level
FindBadConstructsConsumer::getErrorLevel() {
562 return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
563 : DiagnosticsEngine::Warning
;
566 // Returns true if |base| specifies one of the Chromium reference counted
567 // classes (base::RefCounted / base::RefCountedThreadSafe).
568 bool FindBadConstructsConsumer::IsRefCountedCallback(
569 const CXXBaseSpecifier
* base
,
572 FindBadConstructsConsumer
* self
=
573 static_cast<FindBadConstructsConsumer
*>(user_data
);
575 const TemplateSpecializationType
* base_type
=
576 dyn_cast
<TemplateSpecializationType
>(
577 UnwrapType(base
->getType().getTypePtr()));
579 // Base-most definition is not a template, so this cannot derive from
580 // base::RefCounted. However, it may still be possible to use with a
581 // scoped_refptr<> and support ref-counting, so this is not a perfect
582 // guarantee of safety.
586 TemplateName name
= base_type
->getTemplateName();
587 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
588 std::string base_name
= decl
->getNameAsString();
590 // Check for both base::RefCounted and base::RefCountedThreadSafe.
591 if (base_name
.compare(0, 10, "RefCounted") == 0 &&
592 self
->GetNamespace(decl
) == "base") {
600 // Returns true if |base| specifies a class that has a public destructor,
601 // either explicitly or implicitly.
602 bool FindBadConstructsConsumer::HasPublicDtorCallback(
603 const CXXBaseSpecifier
* base
,
606 // Only examine paths that have public inheritance, as they are the
607 // only ones which will result in the destructor potentially being
608 // exposed. This check is largely redundant, as Chromium code should be
609 // exclusively using public inheritance.
610 if (path
.Access
!= AS_public
)
613 CXXRecordDecl
* record
=
614 dyn_cast
<CXXRecordDecl
>(base
->getType()->getAs
<RecordType
>()->getDecl());
615 SourceLocation unused
;
616 return None
!= CheckRecordForRefcountIssue(record
, unused
);
619 // Outputs a C++ inheritance chain as a diagnostic aid.
620 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath
& path
) {
621 for (CXXBasePath::const_iterator it
= path
.begin(); it
!= path
.end(); ++it
) {
622 diagnostic().Report(it
->Base
->getLocStart(), diag_note_inheritance_
)
623 << it
->Class
<< it
->Base
->getType();
627 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue
) {
629 case ImplicitDestructor
:
630 return diag_no_explicit_dtor_
;
631 case PublicDestructor
:
632 return diag_public_dtor_
;
634 assert(false && "Do not call DiagnosticForIssue with issue None");
641 // Check |record| to determine if it has any problematic refcounting
642 // issues and, if so, print them as warnings/errors based on the current
643 // value of getErrorLevel().
645 // If |record| is a C++ class, and if it inherits from one of the Chromium
646 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
647 // ensure that there are no public destructors in the class hierarchy. This
648 // is to guard against accidentally stack-allocating a RefCounted class or
649 // sticking it in a non-ref-counted container (like scoped_ptr<>).
650 void FindBadConstructsConsumer::CheckRefCountedDtors(
651 SourceLocation record_location
,
652 CXXRecordDecl
* record
) {
653 // Skip anonymous structs.
654 if (record
->getIdentifier() == NULL
)
657 // Determine if the current type is even ref-counted.
658 CXXBasePaths refcounted_path
;
659 if (!record
->lookupInBases(&FindBadConstructsConsumer::IsRefCountedCallback
,
662 return; // Class does not derive from a ref-counted base class.
665 // Easy check: Check to see if the current type is problematic.
667 RefcountIssue issue
= CheckRecordForRefcountIssue(record
, loc
);
669 diagnostic().Report(loc
, DiagnosticForIssue(issue
));
670 PrintInheritanceChain(refcounted_path
.front());
673 if (CXXDestructorDecl
* dtor
=
674 refcounted_path
.begin()->back().Class
->getDestructor()) {
675 if (dtor
->getAccess() == AS_protected
&& !dtor
->isVirtual()) {
676 loc
= dtor
->getInnerLocStart();
677 diagnostic().Report(loc
, diag_protected_non_virtual_dtor_
);
682 // Long check: Check all possible base classes for problematic
683 // destructors. This checks for situations involving multiple
684 // inheritance, where the ref-counted class may be implementing an
685 // interface that has a public or implicit destructor.
687 // struct SomeInterface {
688 // virtual void DoFoo();
691 // struct RefCountedInterface
692 // : public base::RefCounted<RefCountedInterface>,
693 // public SomeInterface {
695 // friend class base::Refcounted<RefCountedInterface>;
696 // virtual ~RefCountedInterface() {}
699 // While RefCountedInterface is "safe", in that its destructor is
700 // private, it's possible to do the following "unsafe" code:
701 // scoped_refptr<RefCountedInterface> some_class(
702 // new RefCountedInterface);
703 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
704 // delete static_cast<SomeInterface*>(some_class.get());
705 if (!options_
.check_base_classes
)
708 // Find all public destructors. This will record the class hierarchy
709 // that leads to the public destructor in |dtor_paths|.
710 CXXBasePaths dtor_paths
;
711 if (!record
->lookupInBases(&FindBadConstructsConsumer::HasPublicDtorCallback
,
717 for (CXXBasePaths::const_paths_iterator it
= dtor_paths
.begin();
718 it
!= dtor_paths
.end();
720 // The record with the problem will always be the last record
721 // in the path, since it is the record that stopped the search.
722 const CXXRecordDecl
* problem_record
= dyn_cast
<CXXRecordDecl
>(
723 it
->back().Base
->getType()->getAs
<RecordType
>()->getDecl());
725 issue
= CheckRecordForRefcountIssue(problem_record
, loc
);
727 if (issue
== ImplicitDestructor
) {
728 diagnostic().Report(record_location
, diag_no_explicit_dtor_
);
729 PrintInheritanceChain(refcounted_path
.front());
730 diagnostic().Report(loc
, diag_note_implicit_dtor_
) << problem_record
;
731 PrintInheritanceChain(*it
);
732 } else if (issue
== PublicDestructor
) {
733 diagnostic().Report(record_location
, diag_public_dtor_
);
734 PrintInheritanceChain(refcounted_path
.front());
735 diagnostic().Report(loc
, diag_note_public_dtor_
);
736 PrintInheritanceChain(*it
);
741 // Check for any problems with WeakPtrFactory class members. This currently
742 // only checks that any WeakPtrFactory<T> member of T appears as the last
743 // data member in T. We could consider checking for bad uses of
744 // WeakPtrFactory to refer to other data members, but that would require
745 // looking at the initializer list in constructors to see what the factory
747 // Note, if we later add other unrelated checks of data members, we should
748 // consider collapsing them in to one loop to avoid iterating over the data
749 // members more than once.
750 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
751 SourceLocation record_location
,
752 CXXRecordDecl
* record
) {
753 // Skip anonymous structs.
754 if (record
->getIdentifier() == NULL
)
757 // Iterate through members of the class.
758 RecordDecl::field_iterator
iter(record
->field_begin()),
759 the_end(record
->field_end());
760 SourceLocation weak_ptr_factory_location
; // Invalid initially.
761 for (; iter
!= the_end
; ++iter
) {
762 const TemplateSpecializationType
* template_spec_type
=
763 iter
->getType().getTypePtr()->getAs
<TemplateSpecializationType
>();
764 bool param_is_weak_ptr_factory_to_self
= false;
765 if (template_spec_type
) {
766 const TemplateDecl
* template_decl
=
767 template_spec_type
->getTemplateName().getAsTemplateDecl();
768 if (template_decl
&& template_spec_type
->getNumArgs() == 1) {
769 if (template_decl
->getNameAsString().compare("WeakPtrFactory") == 0 &&
770 GetNamespace(template_decl
) == "base") {
771 // Only consider WeakPtrFactory members which are specialized for the
773 const TemplateArgument
& arg
= template_spec_type
->getArg(0);
774 if (arg
.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
775 record
->getTypeForDecl()->getAsCXXRecordDecl()) {
776 if (!weak_ptr_factory_location
.isValid()) {
777 // Save the first matching WeakPtrFactory member for the
779 weak_ptr_factory_location
= iter
->getLocation();
781 param_is_weak_ptr_factory_to_self
= true;
786 // If we've already seen a WeakPtrFactory<OwningType> and this param is not
787 // one of those, it means there is at least one member after a factory.
788 if (weak_ptr_factory_location
.isValid() &&
789 !param_is_weak_ptr_factory_to_self
) {
790 diagnostic().Report(weak_ptr_factory_location
,
791 diag_weak_ptr_factory_order_
);
796 } // namespace chrome_checker