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 // Generates a fixit hint to remove the 'virtual' keyword.
74 // Unfortunately, there doesn't seem to be a good way to determine the source
75 // location of the 'virtual' keyword. It's available in Declarator, but that
76 // isn't accessible from the AST. So instead, make an educated guess that the
77 // first token is probably the virtual keyword. Strictly speaking, this doesn't
78 // have to be true, but it probably will be.
79 // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
80 FixItHint
FixItRemovalForVirtual(const SourceManager
& manager
,
81 const CXXMethodDecl
* method
) {
82 SourceRange
range(method
->getLocStart());
83 // Get the spelling loc just in case it was expanded from a macro.
84 SourceRange
spelling_range(manager
.getSpellingLoc(range
.getBegin()));
85 // Sanity check that the text looks like virtual.
86 StringRef text
= clang::Lexer::getSourceText(
87 CharSourceRange::getTokenRange(spelling_range
), manager
, LangOptions());
88 if (text
.trim() != "virtual")
90 return FixItHint::CreateRemoval(range
);
93 bool IsPodOrTemplateType(const CXXRecordDecl
& record
) {
94 return record
.isPOD() ||
95 record
.getDescribedClassTemplate() ||
96 record
.getTemplateSpecializationKind() ||
97 record
.isDependentType();
102 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance
& instance
,
103 const Options
& options
)
104 : ChromeClassTester(instance
), options_(options
) {
105 // Messages for virtual method specifiers.
106 diag_method_requires_override_
=
107 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride
);
108 diag_redundant_virtual_specifier_
=
109 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier
);
110 diag_base_method_virtual_and_final_
=
111 diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal
);
113 // Messages for destructors.
114 diag_no_explicit_dtor_
=
115 diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor
);
117 diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor
);
118 diag_protected_non_virtual_dtor_
=
119 diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor
);
121 // Miscellaneous messages.
122 diag_weak_ptr_factory_order_
=
123 diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder
);
124 diag_bad_enum_last_value_
=
125 diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue
);
127 // Registers notes to make it easier to interpret warnings.
128 diag_note_inheritance_
=
129 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteInheritance
);
130 diag_note_implicit_dtor_
=
131 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteImplicitDtor
);
132 diag_note_public_dtor_
=
133 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNotePublicDtor
);
134 diag_note_protected_non_virtual_dtor_
= diagnostic().getCustomDiagID(
135 DiagnosticsEngine::Note
, kNoteProtectedNonVirtualDtor
);
138 bool FindBadConstructsConsumer::VisitDecl(clang::Decl
* decl
) {
139 clang::TagDecl
* tag_decl
= dyn_cast
<clang::TagDecl
>(decl
);
140 if (tag_decl
&& tag_decl
->isCompleteDefinition())
145 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location
,
146 CXXRecordDecl
* record
) {
147 // By default, the clang checker doesn't check some types (templates, etc).
148 // That was only a mistake; once Chromium code passes these checks, we should
149 // remove the "check-templates" option and remove this code.
150 // See crbug.com/441916
151 if (!options_
.check_templates
&& IsPodOrTemplateType(*record
))
154 bool implementation_file
= InImplementationFile(record_location
);
156 if (!implementation_file
) {
157 // Only check for "heavy" constructors/destructors in header files;
158 // within implementation files, there is no performance cost.
160 // If this is a POD or a class template or a type dependent on a
161 // templated class, assume there's no ctor/dtor/virtual method
162 // optimization that we should do.
163 if (!IsPodOrTemplateType(*record
))
164 CheckCtorDtorWeight(record_location
, record
);
167 bool warn_on_inline_bodies
= !implementation_file
;
168 // Check that all virtual methods are annotated with override or final.
169 // Note this could also apply to templates, but for some reason Clang
170 // does not always see the "override", so we get false positives.
171 // See http://llvm.org/bugs/show_bug.cgi?id=18440 and
172 // http://llvm.org/bugs/show_bug.cgi?id=21942
173 if (!IsPodOrTemplateType(*record
))
174 CheckVirtualMethods(record_location
, record
, warn_on_inline_bodies
);
176 CheckRefCountedDtors(record_location
, record
);
178 CheckWeakPtrFactoryMembers(record_location
, record
);
181 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location
,
182 EnumDecl
* enum_decl
) {
183 if (!options_
.check_enum_last_value
)
186 bool got_one
= false;
187 bool is_signed
= false;
188 llvm::APSInt max_so_far
;
189 EnumDecl::enumerator_iterator iter
;
190 for (iter
= enum_decl
->enumerator_begin();
191 iter
!= enum_decl
->enumerator_end();
193 llvm::APSInt current_value
= iter
->getInitVal();
195 max_so_far
= current_value
;
196 is_signed
= current_value
.isSigned();
199 if (is_signed
!= current_value
.isSigned()) {
200 // This only happens in some cases when compiling C (not C++) files,
201 // so it is OK to bail out here.
204 if (current_value
> max_so_far
)
205 max_so_far
= current_value
;
208 for (iter
= enum_decl
->enumerator_begin();
209 iter
!= enum_decl
->enumerator_end();
211 std::string name
= iter
->getNameAsString();
212 if (((name
.size() > 4 && name
.compare(name
.size() - 4, 4, "Last") == 0) ||
213 (name
.size() > 5 && name
.compare(name
.size() - 5, 5, "_LAST") == 0)) &&
214 iter
->getInitVal() < max_so_far
) {
215 diagnostic().Report(iter
->getLocation(), diag_bad_enum_last_value_
);
220 void FindBadConstructsConsumer::CheckCtorDtorWeight(
221 SourceLocation record_location
,
222 CXXRecordDecl
* record
) {
223 // We don't handle anonymous structs. If this record doesn't have a
224 // name, it's of the form:
229 if (record
->getIdentifier() == NULL
)
232 // Count the number of templated base classes as a feature of whether the
233 // destructor can be inlined.
234 int templated_base_classes
= 0;
235 for (CXXRecordDecl::base_class_const_iterator it
= record
->bases_begin();
236 it
!= record
->bases_end();
238 if (it
->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
239 TypeLoc::TemplateSpecialization
) {
240 ++templated_base_classes
;
244 // Count the number of trivial and non-trivial member variables.
245 int trivial_member
= 0;
246 int non_trivial_member
= 0;
247 int templated_non_trivial_member
= 0;
248 for (RecordDecl::field_iterator it
= record
->field_begin();
249 it
!= record
->field_end();
251 CountType(it
->getType().getTypePtr(),
254 &templated_non_trivial_member
);
257 // Check to see if we need to ban inlined/synthesized constructors. Note
258 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
260 // Deriving from a templated base class shouldn't be enough to trigger
261 // the ctor warning, but if you do *anything* else, it should.
263 // TODO(erg): This is motivated by templated base classes that don't have
264 // any data members. Somehow detect when templated base classes have data
265 // members and treat them differently.
266 dtor_score
+= templated_base_classes
* 9;
267 // Instantiating a template is an insta-hit.
268 dtor_score
+= templated_non_trivial_member
* 10;
269 // The fourth normal class member should trigger the warning.
270 dtor_score
+= non_trivial_member
* 3;
272 int ctor_score
= dtor_score
;
273 // You should be able to have 9 ints before we warn you.
274 ctor_score
+= trivial_member
;
276 if (ctor_score
>= 10) {
277 if (!record
->hasUserDeclaredConstructor()) {
278 emitWarning(record_location
,
279 "Complex class/struct needs an explicit out-of-line "
282 // Iterate across all the constructors in this file and yell if we
283 // find one that tries to be inline.
284 for (CXXRecordDecl::ctor_iterator it
= record
->ctor_begin();
285 it
!= record
->ctor_end();
287 // The current check is buggy. An implicit copy constructor does not
288 // have an inline body, so this check never fires for classes with a
289 // user-declared out-of-line constructor.
290 if (it
->hasInlineBody()) {
291 if (it
->isCopyConstructor() &&
292 !record
->hasUserDeclaredCopyConstructor()) {
293 emitWarning(record_location
,
294 "Complex class/struct needs an explicit out-of-line "
295 "copy constructor.");
297 emitWarning(it
->getInnerLocStart(),
298 "Complex constructor has an inlined body.");
300 } else if (it
->isInlined() && !it
->isInlineSpecified() &&
301 !it
->isDeleted() && (!it
->isCopyOrMoveConstructor() ||
302 it
->isExplicitlyDefaulted())) {
303 // isInlined() is a more reliable check than hasInlineBody(), but
304 // unfortunately, it results in warnings for implicit copy/move
305 // constructors in the previously mentioned situation. To preserve
306 // compatibility with existing Chromium code, only warn if it's an
307 // explicitly defaulted copy or move constructor.
308 emitWarning(it
->getInnerLocStart(),
309 "Complex constructor has an inlined body.");
315 // The destructor side is equivalent except that we don't check for
316 // trivial members; 20 ints don't need a destructor.
317 if (dtor_score
>= 10 && !record
->hasTrivialDestructor()) {
318 if (!record
->hasUserDeclaredDestructor()) {
319 emitWarning(record_location
,
320 "Complex class/struct needs an explicit out-of-line "
322 } else if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
323 if (dtor
->isInlined() && !dtor
->isInlineSpecified() &&
324 !dtor
->isDeleted()) {
325 emitWarning(dtor
->getInnerLocStart(),
326 "Complex destructor has an inline body.");
332 bool FindBadConstructsConsumer::InTestingNamespace(const Decl
* record
) {
333 return GetNamespace(record
).find("testing") != std::string::npos
;
336 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
337 const CXXMethodDecl
* method
) {
338 if (InBannedNamespace(method
))
340 for (CXXMethodDecl::method_iterator i
= method
->begin_overridden_methods();
341 i
!= method
->end_overridden_methods();
343 const CXXMethodDecl
* overridden
= *i
;
344 if (IsMethodInBannedOrTestingNamespace(overridden
) ||
345 // Provide an exception for ::testing::Test. gtest itself uses some
346 // magic to try to make sure SetUp()/TearDown() aren't capitalized
347 // incorrectly, but having the plugin enforce override is also nice.
348 (InTestingNamespace(overridden
) &&
349 !IsGtestTestFixture(overridden
->getParent()))) {
357 // Checks that virtual methods are correctly annotated, and have no body in a
359 void FindBadConstructsConsumer::CheckVirtualMethods(
360 SourceLocation record_location
,
361 CXXRecordDecl
* record
,
362 bool warn_on_inline_bodies
) {
363 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
364 // trick to get around that. If a class has member variables whose types are
365 // in the "testing" namespace (which is how gmock works behind the scenes),
366 // there's a really high chance we won't care about these errors
367 for (CXXRecordDecl::field_iterator it
= record
->field_begin();
368 it
!= record
->field_end();
370 CXXRecordDecl
* record_type
= it
->getTypeSourceInfo()
373 ->getAsCXXRecordDecl();
375 if (InTestingNamespace(record_type
)) {
381 for (CXXRecordDecl::method_iterator it
= record
->method_begin();
382 it
!= record
->method_end();
384 if (it
->isCopyAssignmentOperator() || isa
<CXXConstructorDecl
>(*it
)) {
385 // Ignore constructors and assignment operators.
386 } else if (isa
<CXXDestructorDecl
>(*it
) &&
387 !record
->hasUserDeclaredDestructor()) {
388 // Ignore non-user-declared destructors.
389 } else if (!it
->isVirtual()) {
392 CheckVirtualSpecifiers(*it
);
393 if (warn_on_inline_bodies
)
394 CheckVirtualBodies(*it
);
399 // Makes sure that virtual methods use the most appropriate specifier. If a
400 // virtual method overrides a method from a base class, only the override
401 // specifier should be used. If the method should not be overridden by derived
402 // classes, only the final specifier should be used.
403 void FindBadConstructsConsumer::CheckVirtualSpecifiers(
404 const CXXMethodDecl
* method
) {
405 bool is_override
= method
->size_overridden_methods() > 0;
406 bool has_virtual
= method
->isVirtualAsWritten();
407 OverrideAttr
* override_attr
= method
->getAttr
<OverrideAttr
>();
408 FinalAttr
* final_attr
= method
->getAttr
<FinalAttr
>();
410 if (IsMethodInBannedOrTestingNamespace(method
))
413 SourceManager
& manager
= instance().getSourceManager();
415 // Complain if a method is annotated virtual && (override || final).
416 if (has_virtual
&& (override_attr
|| final_attr
)) {
417 // ... but only if virtual does not originate in a macro from a banned file.
418 // Note this is just an educated guess: the assumption here is that any
419 // macro for declaring methods will probably be at the start of the method's
421 if (!InBannedDirectory(manager
.getSpellingLoc(method
->getLocStart()))) {
422 diagnostic().Report(method
->getLocStart(),
423 diag_redundant_virtual_specifier_
)
425 << (override_attr
? static_cast<Attr
*>(override_attr
) : final_attr
)
426 << FixItRemovalForVirtual(manager
, method
);
430 // Complain if a method is an override and is not annotated with override or
432 if (is_override
&& !override_attr
&& !final_attr
) {
433 SourceRange range
= method
->getSourceRange();
435 if (method
->hasInlineBody()) {
436 loc
= method
->getBody()->getSourceRange().getBegin();
438 // TODO(dcheng): We should probably use ASTContext's LangOptions here.
439 LangOptions lang_options
;
440 loc
= Lexer::getLocForEndOfToken(
441 manager
.getSpellingLoc(range
.getEnd()), 0,
442 manager
, lang_options
);
443 // The original code used the ending source loc of TypeSourceInfo's
444 // TypeLoc. Unfortunately, this breaks down in the presence of attributes.
445 // Attributes often appear at the end of a TypeLoc, e.g.
446 // virtual ULONG __stdcall AddRef()
447 // has a TypeSourceInfo that looks something like:
448 // ULONG AddRef() __attribute(stdcall)
449 // so a fix-it insertion would be generated to insert 'override' after
450 // __stdcall in the code as written.
451 // While using the spelling loc of the CXXMethodDecl fixes attribute
452 // handling, it breaks handling of "= 0" and similar constructs.. To work
453 // around this, scan backwards in the source text for a '=' or ')' token
454 // and adjust the location as needed...
455 for (SourceLocation l
= loc
.getLocWithOffset(-1);
456 l
!= manager
.getLocForStartOfFile(manager
.getFileID(loc
));
457 l
= l
.getLocWithOffset(-1)) {
458 l
= Lexer::GetBeginningOfToken(l
, manager
, lang_options
);
460 // getRawToken() returns *true* on failure. In that case, just give up
461 // and don't bother generating a possibly incorrect fix-it.
462 if (Lexer::getRawToken(l
, token
, manager
, lang_options
, true)) {
463 loc
= SourceLocation();
466 if (token
.is(tok::r_paren
)) {
468 } else if (token
.is(tok::equal
)) {
475 diagnostic().Report(loc
, diag_method_requires_override_
)
476 << FixItHint::CreateInsertion(loc
, " override");
478 diagnostic().Report(range
.getBegin(), diag_method_requires_override_
);
482 if (final_attr
&& override_attr
) {
483 diagnostic().Report(override_attr
->getLocation(),
484 diag_redundant_virtual_specifier_
)
485 << override_attr
<< final_attr
486 << FixItHint::CreateRemoval(override_attr
->getRange());
489 if (final_attr
&& !is_override
) {
490 diagnostic().Report(method
->getLocStart(),
491 diag_base_method_virtual_and_final_
)
492 << FixItRemovalForVirtual(manager
, method
)
493 << FixItHint::CreateRemoval(final_attr
->getRange());
497 void FindBadConstructsConsumer::CheckVirtualBodies(
498 const CXXMethodDecl
* method
) {
499 // Virtual methods should not have inline definitions beyond "{}". This
500 // only matters for header files.
501 if (method
->hasBody() && method
->hasInlineBody()) {
502 if (CompoundStmt
* cs
= dyn_cast
<CompoundStmt
>(method
->getBody())) {
504 emitWarning(cs
->getLBracLoc(),
505 "virtual methods with non-empty bodies shouldn't be "
512 void FindBadConstructsConsumer::CountType(const Type
* type
,
514 int* non_trivial_member
,
515 int* templated_non_trivial_member
) {
516 switch (type
->getTypeClass()) {
518 // Simplifying; the whole class isn't trivial if the dtor is, but
519 // we use this as a signal about complexity.
520 if (TypeHasNonTrivialDtor(type
))
523 (*non_trivial_member
)++;
526 case Type::TemplateSpecialization
: {
528 dyn_cast
<TemplateSpecializationType
>(type
)->getTemplateName();
529 bool whitelisted_template
= false;
531 // HACK: I'm at a loss about how to get the syntax checker to get
532 // whether a template is exterened or not. For the first pass here,
533 // just do retarded string comparisons.
534 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
535 std::string base_name
= decl
->getNameAsString();
536 if (base_name
== "basic_string")
537 whitelisted_template
= true;
540 if (whitelisted_template
)
541 (*non_trivial_member
)++;
543 (*templated_non_trivial_member
)++;
546 case Type::Elaborated
: {
547 CountType(dyn_cast
<ElaboratedType
>(type
)->getNamedType().getTypePtr(),
550 templated_non_trivial_member
);
553 case Type::Typedef
: {
554 while (const TypedefType
* TT
= dyn_cast
<TypedefType
>(type
)) {
555 type
= TT
->getDecl()->getUnderlyingType().getTypePtr();
560 templated_non_trivial_member
);
564 // Stupid assumption: anything we see that isn't the above is one of
565 // the 20 integer types.
572 // Check |record| for issues that are problematic for ref-counted types.
573 // Note that |record| may not be a ref-counted type, but a base class for
575 // If there are issues, update |loc| with the SourceLocation of the issue
576 // and returns appropriately, or returns None if there are no issues.
577 FindBadConstructsConsumer::RefcountIssue
578 FindBadConstructsConsumer::CheckRecordForRefcountIssue(
579 const CXXRecordDecl
* record
,
580 SourceLocation
& loc
) {
581 if (!record
->hasUserDeclaredDestructor()) {
582 loc
= record
->getLocation();
583 return ImplicitDestructor
;
586 if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
587 if (dtor
->getAccess() == AS_public
) {
588 loc
= dtor
->getInnerLocStart();
589 return PublicDestructor
;
596 // Adds either a warning or error, based on the current handling of
598 DiagnosticsEngine::Level
FindBadConstructsConsumer::getErrorLevel() {
599 #if defined(LLVM_ON_WIN32)
600 // TODO(dcheng): Re-enable -Werror for these diagnostics on Windows once all
601 // the pre-existing warnings are cleaned up. https://crbug.com/467287
602 return DiagnosticsEngine::Warning
;
604 return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
605 : DiagnosticsEngine::Warning
;
609 // Returns true if |base| specifies one of the Chromium reference counted
610 // classes (base::RefCounted / base::RefCountedThreadSafe).
611 bool FindBadConstructsConsumer::IsRefCountedCallback(
612 const CXXBaseSpecifier
* base
,
615 FindBadConstructsConsumer
* self
=
616 static_cast<FindBadConstructsConsumer
*>(user_data
);
618 const TemplateSpecializationType
* base_type
=
619 dyn_cast
<TemplateSpecializationType
>(
620 UnwrapType(base
->getType().getTypePtr()));
622 // Base-most definition is not a template, so this cannot derive from
623 // base::RefCounted. However, it may still be possible to use with a
624 // scoped_refptr<> and support ref-counting, so this is not a perfect
625 // guarantee of safety.
629 TemplateName name
= base_type
->getTemplateName();
630 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
631 std::string base_name
= decl
->getNameAsString();
633 // Check for both base::RefCounted and base::RefCountedThreadSafe.
634 if (base_name
.compare(0, 10, "RefCounted") == 0 &&
635 self
->GetNamespace(decl
) == "base") {
643 // Returns true if |base| specifies a class that has a public destructor,
644 // either explicitly or implicitly.
645 bool FindBadConstructsConsumer::HasPublicDtorCallback(
646 const CXXBaseSpecifier
* base
,
649 // Only examine paths that have public inheritance, as they are the
650 // only ones which will result in the destructor potentially being
651 // exposed. This check is largely redundant, as Chromium code should be
652 // exclusively using public inheritance.
653 if (path
.Access
!= AS_public
)
656 CXXRecordDecl
* record
=
657 dyn_cast
<CXXRecordDecl
>(base
->getType()->getAs
<RecordType
>()->getDecl());
658 SourceLocation unused
;
659 return None
!= CheckRecordForRefcountIssue(record
, unused
);
662 // Outputs a C++ inheritance chain as a diagnostic aid.
663 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath
& path
) {
664 for (CXXBasePath::const_iterator it
= path
.begin(); it
!= path
.end(); ++it
) {
665 diagnostic().Report(it
->Base
->getLocStart(), diag_note_inheritance_
)
666 << it
->Class
<< it
->Base
->getType();
670 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue
) {
672 case ImplicitDestructor
:
673 return diag_no_explicit_dtor_
;
674 case PublicDestructor
:
675 return diag_public_dtor_
;
677 assert(false && "Do not call DiagnosticForIssue with issue None");
684 // Check |record| to determine if it has any problematic refcounting
685 // issues and, if so, print them as warnings/errors based on the current
686 // value of getErrorLevel().
688 // If |record| is a C++ class, and if it inherits from one of the Chromium
689 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
690 // ensure that there are no public destructors in the class hierarchy. This
691 // is to guard against accidentally stack-allocating a RefCounted class or
692 // sticking it in a non-ref-counted container (like scoped_ptr<>).
693 void FindBadConstructsConsumer::CheckRefCountedDtors(
694 SourceLocation record_location
,
695 CXXRecordDecl
* record
) {
696 // Skip anonymous structs.
697 if (record
->getIdentifier() == NULL
)
700 // Determine if the current type is even ref-counted.
701 CXXBasePaths refcounted_path
;
702 if (!record
->lookupInBases(&FindBadConstructsConsumer::IsRefCountedCallback
,
705 return; // Class does not derive from a ref-counted base class.
708 // Easy check: Check to see if the current type is problematic.
710 RefcountIssue issue
= CheckRecordForRefcountIssue(record
, loc
);
712 diagnostic().Report(loc
, DiagnosticForIssue(issue
));
713 PrintInheritanceChain(refcounted_path
.front());
716 if (CXXDestructorDecl
* dtor
=
717 refcounted_path
.begin()->back().Class
->getDestructor()) {
718 if (dtor
->getAccess() == AS_protected
&& !dtor
->isVirtual()) {
719 loc
= dtor
->getInnerLocStart();
720 diagnostic().Report(loc
, diag_protected_non_virtual_dtor_
);
725 // Long check: Check all possible base classes for problematic
726 // destructors. This checks for situations involving multiple
727 // inheritance, where the ref-counted class may be implementing an
728 // interface that has a public or implicit destructor.
730 // struct SomeInterface {
731 // virtual void DoFoo();
734 // struct RefCountedInterface
735 // : public base::RefCounted<RefCountedInterface>,
736 // public SomeInterface {
738 // friend class base::Refcounted<RefCountedInterface>;
739 // virtual ~RefCountedInterface() {}
742 // While RefCountedInterface is "safe", in that its destructor is
743 // private, it's possible to do the following "unsafe" code:
744 // scoped_refptr<RefCountedInterface> some_class(
745 // new RefCountedInterface);
746 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
747 // delete static_cast<SomeInterface*>(some_class.get());
748 if (!options_
.check_base_classes
)
751 // Find all public destructors. This will record the class hierarchy
752 // that leads to the public destructor in |dtor_paths|.
753 CXXBasePaths dtor_paths
;
754 if (!record
->lookupInBases(&FindBadConstructsConsumer::HasPublicDtorCallback
,
760 for (CXXBasePaths::const_paths_iterator it
= dtor_paths
.begin();
761 it
!= dtor_paths
.end();
763 // The record with the problem will always be the last record
764 // in the path, since it is the record that stopped the search.
765 const CXXRecordDecl
* problem_record
= dyn_cast
<CXXRecordDecl
>(
766 it
->back().Base
->getType()->getAs
<RecordType
>()->getDecl());
768 issue
= CheckRecordForRefcountIssue(problem_record
, loc
);
770 if (issue
== ImplicitDestructor
) {
771 diagnostic().Report(record_location
, diag_no_explicit_dtor_
);
772 PrintInheritanceChain(refcounted_path
.front());
773 diagnostic().Report(loc
, diag_note_implicit_dtor_
) << problem_record
;
774 PrintInheritanceChain(*it
);
775 } else if (issue
== PublicDestructor
) {
776 diagnostic().Report(record_location
, diag_public_dtor_
);
777 PrintInheritanceChain(refcounted_path
.front());
778 diagnostic().Report(loc
, diag_note_public_dtor_
);
779 PrintInheritanceChain(*it
);
784 // Check for any problems with WeakPtrFactory class members. This currently
785 // only checks that any WeakPtrFactory<T> member of T appears as the last
786 // data member in T. We could consider checking for bad uses of
787 // WeakPtrFactory to refer to other data members, but that would require
788 // looking at the initializer list in constructors to see what the factory
790 // Note, if we later add other unrelated checks of data members, we should
791 // consider collapsing them in to one loop to avoid iterating over the data
792 // members more than once.
793 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
794 SourceLocation record_location
,
795 CXXRecordDecl
* record
) {
796 // Skip anonymous structs.
797 if (record
->getIdentifier() == NULL
)
800 // Iterate through members of the class.
801 RecordDecl::field_iterator
iter(record
->field_begin()),
802 the_end(record
->field_end());
803 SourceLocation weak_ptr_factory_location
; // Invalid initially.
804 for (; iter
!= the_end
; ++iter
) {
805 const TemplateSpecializationType
* template_spec_type
=
806 iter
->getType().getTypePtr()->getAs
<TemplateSpecializationType
>();
807 bool param_is_weak_ptr_factory_to_self
= false;
808 if (template_spec_type
) {
809 const TemplateDecl
* template_decl
=
810 template_spec_type
->getTemplateName().getAsTemplateDecl();
811 if (template_decl
&& template_spec_type
->getNumArgs() == 1) {
812 if (template_decl
->getNameAsString().compare("WeakPtrFactory") == 0 &&
813 GetNamespace(template_decl
) == "base") {
814 // Only consider WeakPtrFactory members which are specialized for the
816 const TemplateArgument
& arg
= template_spec_type
->getArg(0);
817 if (arg
.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
818 record
->getTypeForDecl()->getAsCXXRecordDecl()) {
819 if (!weak_ptr_factory_location
.isValid()) {
820 // Save the first matching WeakPtrFactory member for the
822 weak_ptr_factory_location
= iter
->getLocation();
824 param_is_weak_ptr_factory_to_self
= true;
829 // If we've already seen a WeakPtrFactory<OwningType> and this param is not
830 // one of those, it means there is at least one member after a factory.
831 if (weak_ptr_factory_location
.isValid() &&
832 !param_is_weak_ptr_factory_to_self
) {
833 diagnostic().Report(weak_ptr_factory_location
,
834 diag_weak_ptr_factory_order_
);
839 } // namespace chrome_checker