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
);
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
);
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())
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
)
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();
169 llvm::APSInt current_value
= iter
->getInitVal();
171 max_so_far
= current_value
;
172 is_signed
= current_value
.isSigned();
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.
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();
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:
205 if (record
->getIdentifier() == NULL
)
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();
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();
227 CountType(it
->getType().getTypePtr(),
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.
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 "
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();
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.");
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 "
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
))
302 for (CXXMethodDecl::method_iterator i
= method
->begin_overridden_methods();
303 i
!= method
->end_overridden_methods();
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())))) {
320 // Checks that virtual methods are correctly annotated, and have no body in a
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();
333 CXXRecordDecl
* record_type
= it
->getTypeSourceInfo()
336 ->getAsCXXRecordDecl();
338 if (InTestingNamespace(record_type
)) {
344 for (CXXRecordDecl::method_iterator it
= record
->method_begin();
345 it
!= record
->method_end();
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()) {
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
)
376 if (IsMethodInBannedOrTestingNamespace(method
))
379 if (isa
<CXXDestructorDecl
>(method
) && !options_
.strict_virtual_specifiers
)
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_
)
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
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");
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())) {
438 emitWarning(cs
->getLBracLoc(),
439 "virtual methods with non-empty bodies shouldn't be "
446 void FindBadConstructsConsumer::CountType(const Type
* type
,
448 int* non_trivial_member
,
449 int* templated_non_trivial_member
) {
450 switch (type
->getTypeClass()) {
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
))
457 (*non_trivial_member
)++;
460 case Type::TemplateSpecialization
: {
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
)++;
477 (*templated_non_trivial_member
)++;
480 case Type::Elaborated
: {
481 CountType(dyn_cast
<ElaboratedType
>(type
)->getNamedType().getTypePtr(),
484 templated_non_trivial_member
);
487 case Type::Typedef
: {
488 while (const TypedefType
* TT
= dyn_cast
<TypedefType
>(type
)) {
489 type
= TT
->getDecl()->getUnderlyingType().getTypePtr();
494 templated_non_trivial_member
);
498 // Stupid assumption: anything we see that isn't the above is one of
499 // the 20 integer types.
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
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
;
530 // Adds either a warning or error, based on the current handling of
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
,
543 FindBadConstructsConsumer
* self
=
544 static_cast<FindBadConstructsConsumer
*>(user_data
);
546 const TemplateSpecializationType
* base_type
=
547 dyn_cast
<TemplateSpecializationType
>(
548 UnwrapType(base
->getType().getTypePtr()));
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.
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") {
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
,
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
)
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
) {
600 case ImplicitDestructor
:
601 return diag_no_explicit_dtor_
;
602 case PublicDestructor
:
603 return diag_public_dtor_
;
605 assert(false && "Do not call DiagnosticForIssue with issue None");
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
)
628 // Determine if the current type is even ref-counted.
629 CXXBasePaths refcounted_path
;
630 if (!record
->lookupInBases(&FindBadConstructsConsumer::IsRefCountedCallback
,
633 return; // Class does not derive from a ref-counted base class.
636 // Easy check: Check to see if the current type is problematic.
638 RefcountIssue issue
= CheckRecordForRefcountIssue(record
, loc
);
640 diagnostic().Report(loc
, DiagnosticForIssue(issue
));
641 PrintInheritanceChain(refcounted_path
.front());
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_
);
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();
662 // struct RefCountedInterface
663 // : public base::RefCounted<RefCountedInterface>,
664 // public SomeInterface {
666 // friend class base::Refcounted<RefCountedInterface>;
667 // virtual ~RefCountedInterface() {}
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
)
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
,
688 for (CXXBasePaths::const_paths_iterator it
= dtor_paths
.begin();
689 it
!= dtor_paths
.end();
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
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
)
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
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
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