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 LangOptions
& lang_opts
,
82 const CXXMethodDecl
* method
) {
83 SourceRange
range(method
->getLocStart());
84 // Get the spelling loc just in case it was expanded from a macro.
85 SourceRange
spelling_range(manager
.getSpellingLoc(range
.getBegin()));
86 // Sanity check that the text looks like virtual.
87 StringRef text
= clang::Lexer::getSourceText(
88 CharSourceRange::getTokenRange(spelling_range
), manager
, lang_opts
);
89 if (text
.trim() != "virtual")
91 return FixItHint::CreateRemoval(range
);
94 bool IsPodOrTemplateType(const CXXRecordDecl
& record
) {
95 return record
.isPOD() ||
96 record
.getDescribedClassTemplate() ||
97 record
.getTemplateSpecializationKind() ||
98 record
.isDependentType();
103 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance
& instance
,
104 const Options
& options
)
105 : ChromeClassTester(instance
, options
) {
106 // Messages for virtual method specifiers.
107 diag_method_requires_override_
=
108 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride
);
109 diag_redundant_virtual_specifier_
=
110 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier
);
111 diag_base_method_virtual_and_final_
=
112 diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal
);
114 // Messages for destructors.
115 diag_no_explicit_dtor_
=
116 diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor
);
118 diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor
);
119 diag_protected_non_virtual_dtor_
=
120 diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor
);
122 // Miscellaneous messages.
123 diag_weak_ptr_factory_order_
=
124 diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder
);
125 diag_bad_enum_last_value_
=
126 diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue
);
128 // Registers notes to make it easier to interpret warnings.
129 diag_note_inheritance_
=
130 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteInheritance
);
131 diag_note_implicit_dtor_
=
132 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNoteImplicitDtor
);
133 diag_note_public_dtor_
=
134 diagnostic().getCustomDiagID(DiagnosticsEngine::Note
, kNotePublicDtor
);
135 diag_note_protected_non_virtual_dtor_
= diagnostic().getCustomDiagID(
136 DiagnosticsEngine::Note
, kNoteProtectedNonVirtualDtor
);
139 bool FindBadConstructsConsumer::VisitDecl(clang::Decl
* decl
) {
140 clang::TagDecl
* tag_decl
= dyn_cast
<clang::TagDecl
>(decl
);
141 if (tag_decl
&& tag_decl
->isCompleteDefinition())
146 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location
,
147 CXXRecordDecl
* record
) {
148 // By default, the clang checker doesn't check some types (templates, etc).
149 // That was only a mistake; once Chromium code passes these checks, we should
150 // remove the "check-templates" option and remove this code.
151 // See crbug.com/441916
152 if (!options_
.check_templates
&& IsPodOrTemplateType(*record
))
155 bool implementation_file
= InImplementationFile(record_location
);
157 if (!implementation_file
) {
158 // Only check for "heavy" constructors/destructors in header files;
159 // within implementation files, there is no performance cost.
161 // If this is a POD or a class template or a type dependent on a
162 // templated class, assume there's no ctor/dtor/virtual method
163 // optimization that we should do.
164 if (!IsPodOrTemplateType(*record
))
165 CheckCtorDtorWeight(record_location
, record
);
168 bool warn_on_inline_bodies
= !implementation_file
;
169 // Check that all virtual methods are annotated with override or final.
170 // Note this could also apply to templates, but for some reason Clang
171 // does not always see the "override", so we get false positives.
172 // See http://llvm.org/bugs/show_bug.cgi?id=18440 and
173 // http://llvm.org/bugs/show_bug.cgi?id=21942
174 if (!IsPodOrTemplateType(*record
))
175 CheckVirtualMethods(record_location
, record
, warn_on_inline_bodies
);
177 CheckRefCountedDtors(record_location
, record
);
179 CheckWeakPtrFactoryMembers(record_location
, record
);
182 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location
,
183 EnumDecl
* enum_decl
) {
184 if (!options_
.check_enum_last_value
)
187 bool got_one
= false;
188 bool is_signed
= false;
189 llvm::APSInt max_so_far
;
190 EnumDecl::enumerator_iterator iter
;
191 for (iter
= enum_decl
->enumerator_begin();
192 iter
!= enum_decl
->enumerator_end();
194 llvm::APSInt current_value
= iter
->getInitVal();
196 max_so_far
= current_value
;
197 is_signed
= current_value
.isSigned();
200 if (is_signed
!= current_value
.isSigned()) {
201 // This only happens in some cases when compiling C (not C++) files,
202 // so it is OK to bail out here.
205 if (current_value
> max_so_far
)
206 max_so_far
= current_value
;
209 for (iter
= enum_decl
->enumerator_begin();
210 iter
!= enum_decl
->enumerator_end();
212 std::string name
= iter
->getNameAsString();
213 if (((name
.size() > 4 && name
.compare(name
.size() - 4, 4, "Last") == 0) ||
214 (name
.size() > 5 && name
.compare(name
.size() - 5, 5, "_LAST") == 0)) &&
215 iter
->getInitVal() < max_so_far
) {
216 diagnostic().Report(iter
->getLocation(), diag_bad_enum_last_value_
);
221 void FindBadConstructsConsumer::CheckCtorDtorWeight(
222 SourceLocation record_location
,
223 CXXRecordDecl
* record
) {
224 // We don't handle anonymous structs. If this record doesn't have a
225 // name, it's of the form:
230 if (record
->getIdentifier() == NULL
)
233 // Count the number of templated base classes as a feature of whether the
234 // destructor can be inlined.
235 int templated_base_classes
= 0;
236 for (CXXRecordDecl::base_class_const_iterator it
= record
->bases_begin();
237 it
!= record
->bases_end();
239 if (it
->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
240 TypeLoc::TemplateSpecialization
) {
241 ++templated_base_classes
;
245 // Count the number of trivial and non-trivial member variables.
246 int trivial_member
= 0;
247 int non_trivial_member
= 0;
248 int templated_non_trivial_member
= 0;
249 for (RecordDecl::field_iterator it
= record
->field_begin();
250 it
!= record
->field_end();
252 CountType(it
->getType().getTypePtr(),
255 &templated_non_trivial_member
);
258 // Check to see if we need to ban inlined/synthesized constructors. Note
259 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
261 // Deriving from a templated base class shouldn't be enough to trigger
262 // the ctor warning, but if you do *anything* else, it should.
264 // TODO(erg): This is motivated by templated base classes that don't have
265 // any data members. Somehow detect when templated base classes have data
266 // members and treat them differently.
267 dtor_score
+= templated_base_classes
* 9;
268 // Instantiating a template is an insta-hit.
269 dtor_score
+= templated_non_trivial_member
* 10;
270 // The fourth normal class member should trigger the warning.
271 dtor_score
+= non_trivial_member
* 3;
273 int ctor_score
= dtor_score
;
274 // You should be able to have 9 ints before we warn you.
275 ctor_score
+= trivial_member
;
277 if (ctor_score
>= 10) {
278 if (!record
->hasUserDeclaredConstructor()) {
279 emitWarning(record_location
,
280 "Complex class/struct needs an explicit out-of-line "
283 // Iterate across all the constructors in this file and yell if we
284 // find one that tries to be inline.
285 for (CXXRecordDecl::ctor_iterator it
= record
->ctor_begin();
286 it
!= record
->ctor_end();
288 // The current check is buggy. An implicit copy constructor does not
289 // have an inline body, so this check never fires for classes with a
290 // user-declared out-of-line constructor.
291 if (it
->hasInlineBody()) {
292 if (it
->isCopyConstructor() &&
293 !record
->hasUserDeclaredCopyConstructor()) {
294 // In general, implicit constructors are generated on demand. But
295 // in the Windows component build, dllexport causes instantiation of
296 // the copy constructor which means that this fires on many more
297 // classes. For now, suppress this on dllexported classes.
298 // (This does mean that windows component builds will not emit this
299 // warning in some cases where it is emitted in other configs, but
300 // that's the better tradeoff at this point).
301 // TODO(dcheng): With the RecursiveASTVisitor, these warnings might
302 // be emitted on other platforms too, reevaluate if we want to keep
303 // surpressing this then http://crbug.com/467288
304 if (!record
->hasAttr
<DLLExportAttr
>())
305 emitWarning(record_location
,
306 "Complex class/struct needs an explicit out-of-line "
307 "copy constructor.");
309 // See the comment in the previous branch about copy constructors.
310 // This does the same for implicit move constructors.
311 bool is_likely_compiler_generated_dllexport_move_ctor
=
312 it
->isMoveConstructor() &&
313 !record
->hasUserDeclaredMoveConstructor() &&
314 record
->hasAttr
<DLLExportAttr
>();
315 if (!is_likely_compiler_generated_dllexport_move_ctor
)
316 emitWarning(it
->getInnerLocStart(),
317 "Complex constructor has an inlined body.");
319 } else if (it
->isInlined() && !it
->isInlineSpecified() &&
320 !it
->isDeleted() && (!it
->isCopyOrMoveConstructor() ||
321 it
->isExplicitlyDefaulted())) {
322 // isInlined() is a more reliable check than hasInlineBody(), but
323 // unfortunately, it results in warnings for implicit copy/move
324 // constructors in the previously mentioned situation. To preserve
325 // compatibility with existing Chromium code, only warn if it's an
326 // explicitly defaulted copy or move constructor.
327 emitWarning(it
->getInnerLocStart(),
328 "Complex constructor has an inlined body.");
334 // The destructor side is equivalent except that we don't check for
335 // trivial members; 20 ints don't need a destructor.
336 if (dtor_score
>= 10 && !record
->hasTrivialDestructor()) {
337 if (!record
->hasUserDeclaredDestructor()) {
338 emitWarning(record_location
,
339 "Complex class/struct needs an explicit out-of-line "
341 } else if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
342 if (dtor
->isInlined() && !dtor
->isInlineSpecified() &&
343 !dtor
->isDeleted()) {
344 emitWarning(dtor
->getInnerLocStart(),
345 "Complex destructor has an inline body.");
351 bool FindBadConstructsConsumer::InTestingNamespace(const Decl
* record
) {
352 return GetNamespace(record
).find("testing") != std::string::npos
;
355 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
356 const CXXMethodDecl
* method
) {
357 if (InBannedNamespace(method
))
359 for (CXXMethodDecl::method_iterator i
= method
->begin_overridden_methods();
360 i
!= method
->end_overridden_methods();
362 const CXXMethodDecl
* overridden
= *i
;
363 if (IsMethodInBannedOrTestingNamespace(overridden
) ||
364 // Provide an exception for ::testing::Test. gtest itself uses some
365 // magic to try to make sure SetUp()/TearDown() aren't capitalized
366 // incorrectly, but having the plugin enforce override is also nice.
367 (InTestingNamespace(overridden
) &&
368 !IsGtestTestFixture(overridden
->getParent()))) {
376 SuppressibleDiagnosticBuilder
377 FindBadConstructsConsumer::ReportIfSpellingLocNotIgnored(
379 unsigned diagnostic_id
) {
380 return SuppressibleDiagnosticBuilder(
381 &diagnostic(), loc
, diagnostic_id
,
382 InBannedDirectory(instance().getSourceManager().getSpellingLoc(loc
)));
385 // Checks that virtual methods are correctly annotated, and have no body in a
387 void FindBadConstructsConsumer::CheckVirtualMethods(
388 SourceLocation record_location
,
389 CXXRecordDecl
* record
,
390 bool warn_on_inline_bodies
) {
391 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
392 // trick to get around that. If a class has member variables whose types are
393 // in the "testing" namespace (which is how gmock works behind the scenes),
394 // there's a really high chance we won't care about these errors
395 for (CXXRecordDecl::field_iterator it
= record
->field_begin();
396 it
!= record
->field_end();
398 CXXRecordDecl
* record_type
= it
->getTypeSourceInfo()
401 ->getAsCXXRecordDecl();
403 if (InTestingNamespace(record_type
)) {
409 for (CXXRecordDecl::method_iterator it
= record
->method_begin();
410 it
!= record
->method_end();
412 if (it
->isCopyAssignmentOperator() || isa
<CXXConstructorDecl
>(*it
)) {
413 // Ignore constructors and assignment operators.
414 } else if (isa
<CXXDestructorDecl
>(*it
) &&
415 !record
->hasUserDeclaredDestructor()) {
416 // Ignore non-user-declared destructors.
417 } else if (!it
->isVirtual()) {
420 CheckVirtualSpecifiers(*it
);
421 if (warn_on_inline_bodies
)
422 CheckVirtualBodies(*it
);
427 // Makes sure that virtual methods use the most appropriate specifier. If a
428 // virtual method overrides a method from a base class, only the override
429 // specifier should be used. If the method should not be overridden by derived
430 // classes, only the final specifier should be used.
431 void FindBadConstructsConsumer::CheckVirtualSpecifiers(
432 const CXXMethodDecl
* method
) {
433 bool is_override
= method
->size_overridden_methods() > 0;
434 bool has_virtual
= method
->isVirtualAsWritten();
435 OverrideAttr
* override_attr
= method
->getAttr
<OverrideAttr
>();
436 FinalAttr
* final_attr
= method
->getAttr
<FinalAttr
>();
438 if (IsMethodInBannedOrTestingNamespace(method
))
441 SourceManager
& manager
= instance().getSourceManager();
442 const LangOptions
& lang_opts
= instance().getLangOpts();
444 // Complain if a method is annotated virtual && (override || final).
445 if (has_virtual
&& (override_attr
|| final_attr
)) {
446 // ... but only if virtual does not originate in a macro from a banned file.
447 // Note this is just an educated guess: the assumption here is that any
448 // macro for declaring methods will probably be at the start of the method's
450 ReportIfSpellingLocNotIgnored(method
->getLocStart(),
451 diag_redundant_virtual_specifier_
)
453 << (override_attr
? static_cast<Attr
*>(override_attr
) : final_attr
)
454 << FixItRemovalForVirtual(manager
, lang_opts
, method
);
457 // Complain if a method is an override and is not annotated with override or
459 if (is_override
&& !override_attr
&& !final_attr
) {
460 SourceRange range
= method
->getSourceRange();
462 if (method
->hasInlineBody()) {
463 loc
= method
->getBody()->getSourceRange().getBegin();
465 loc
= Lexer::getLocForEndOfToken(manager
.getSpellingLoc(range
.getEnd()),
466 0, manager
, lang_opts
);
467 // The original code used the ending source loc of TypeSourceInfo's
468 // TypeLoc. Unfortunately, this breaks down in the presence of attributes.
469 // Attributes often appear at the end of a TypeLoc, e.g.
470 // virtual ULONG __stdcall AddRef()
471 // has a TypeSourceInfo that looks something like:
472 // ULONG AddRef() __attribute(stdcall)
473 // so a fix-it insertion would be generated to insert 'override' after
474 // __stdcall in the code as written.
475 // While using the spelling loc of the CXXMethodDecl fixes attribute
476 // handling, it breaks handling of "= 0" and similar constructs.. To work
477 // around this, scan backwards in the source text for a '=' or ')' token
478 // and adjust the location as needed...
479 for (SourceLocation l
= loc
.getLocWithOffset(-1);
480 l
!= manager
.getLocForStartOfFile(manager
.getFileID(loc
));
481 l
= l
.getLocWithOffset(-1)) {
482 l
= Lexer::GetBeginningOfToken(l
, manager
, lang_opts
);
484 // getRawToken() returns *true* on failure. In that case, just give up
485 // and don't bother generating a possibly incorrect fix-it.
486 if (Lexer::getRawToken(l
, token
, manager
, lang_opts
, true)) {
487 loc
= SourceLocation();
490 if (token
.is(tok::r_paren
)) {
492 } else if (token
.is(tok::equal
)) {
498 // Again, only emit the warning if it doesn't originate from a macro in
501 ReportIfSpellingLocNotIgnored(loc
, diag_method_requires_override_
)
502 << FixItHint::CreateInsertion(loc
, " override");
504 ReportIfSpellingLocNotIgnored(range
.getBegin(),
505 diag_method_requires_override_
);
509 if (final_attr
&& override_attr
) {
510 ReportIfSpellingLocNotIgnored(override_attr
->getLocation(),
511 diag_redundant_virtual_specifier_
)
512 << override_attr
<< final_attr
513 << FixItHint::CreateRemoval(override_attr
->getRange());
516 if (final_attr
&& !is_override
) {
517 ReportIfSpellingLocNotIgnored(method
->getLocStart(),
518 diag_base_method_virtual_and_final_
)
519 << FixItRemovalForVirtual(manager
, lang_opts
, method
)
520 << FixItHint::CreateRemoval(final_attr
->getRange());
524 void FindBadConstructsConsumer::CheckVirtualBodies(
525 const CXXMethodDecl
* method
) {
526 // Virtual methods should not have inline definitions beyond "{}". This
527 // only matters for header files.
528 if (method
->hasBody() && method
->hasInlineBody()) {
529 if (CompoundStmt
* cs
= dyn_cast
<CompoundStmt
>(method
->getBody())) {
531 SourceLocation loc
= cs
->getLBracLoc();
532 // CR_BEGIN_MSG_MAP_EX and BEGIN_SAFE_MSG_MAP_EX try to be compatible
533 // to BEGIN_MSG_MAP(_EX). So even though they are in chrome code,
534 // we can't easily fix them, so explicitly whitelist them here.
536 if (loc
.isMacroID()) {
537 SourceManager
& manager
= instance().getSourceManager();
538 if (InBannedDirectory(manager
.getSpellingLoc(loc
)))
541 StringRef name
= Lexer::getImmediateMacroName(
542 loc
, manager
, instance().getLangOpts());
543 if (name
== "CR_BEGIN_MSG_MAP_EX" ||
544 name
== "BEGIN_SAFE_MSG_MAP_EX")
550 "virtual methods with non-empty bodies shouldn't be "
557 void FindBadConstructsConsumer::CountType(const Type
* type
,
559 int* non_trivial_member
,
560 int* templated_non_trivial_member
) {
561 switch (type
->getTypeClass()) {
563 // Simplifying; the whole class isn't trivial if the dtor is, but
564 // we use this as a signal about complexity.
565 if (TypeHasNonTrivialDtor(type
))
568 (*non_trivial_member
)++;
571 case Type::TemplateSpecialization
: {
573 dyn_cast
<TemplateSpecializationType
>(type
)->getTemplateName();
574 bool whitelisted_template
= false;
576 // HACK: I'm at a loss about how to get the syntax checker to get
577 // whether a template is externed or not. For the first pass here,
578 // just do retarded string comparisons.
579 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
580 std::string base_name
= decl
->getNameAsString();
581 if (base_name
== "basic_string")
582 whitelisted_template
= true;
585 if (whitelisted_template
)
586 (*non_trivial_member
)++;
588 (*templated_non_trivial_member
)++;
591 case Type::Elaborated
: {
592 CountType(dyn_cast
<ElaboratedType
>(type
)->getNamedType().getTypePtr(),
595 templated_non_trivial_member
);
598 case Type::Typedef
: {
599 while (const TypedefType
* TT
= dyn_cast
<TypedefType
>(type
)) {
600 type
= TT
->getDecl()->getUnderlyingType().getTypePtr();
605 templated_non_trivial_member
);
609 // Stupid assumption: anything we see that isn't the above is one of
610 // the 20 integer types.
617 // Check |record| for issues that are problematic for ref-counted types.
618 // Note that |record| may not be a ref-counted type, but a base class for
620 // If there are issues, update |loc| with the SourceLocation of the issue
621 // and returns appropriately, or returns None if there are no issues.
623 FindBadConstructsConsumer::RefcountIssue
624 FindBadConstructsConsumer::CheckRecordForRefcountIssue(
625 const CXXRecordDecl
* record
,
626 SourceLocation
& loc
) {
627 if (!record
->hasUserDeclaredDestructor()) {
628 loc
= record
->getLocation();
629 return ImplicitDestructor
;
632 if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
633 if (dtor
->getAccess() == AS_public
) {
634 loc
= dtor
->getInnerLocStart();
635 return PublicDestructor
;
642 // Returns true if |base| specifies one of the Chromium reference counted
643 // classes (base::RefCounted / base::RefCountedThreadSafe).
644 bool FindBadConstructsConsumer::IsRefCounted(
645 const CXXBaseSpecifier
* base
,
647 FindBadConstructsConsumer
* self
= this;
648 const TemplateSpecializationType
* base_type
=
649 dyn_cast
<TemplateSpecializationType
>(
650 UnwrapType(base
->getType().getTypePtr()));
652 // Base-most definition is not a template, so this cannot derive from
653 // base::RefCounted. However, it may still be possible to use with a
654 // scoped_refptr<> and support ref-counting, so this is not a perfect
655 // guarantee of safety.
659 TemplateName name
= base_type
->getTemplateName();
660 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
661 std::string base_name
= decl
->getNameAsString();
663 // Check for both base::RefCounted and base::RefCountedThreadSafe.
664 if (base_name
.compare(0, 10, "RefCounted") == 0 &&
665 self
->GetNamespace(decl
) == "base") {
673 // Returns true if |base| specifies a class that has a public destructor,
674 // either explicitly or implicitly.
676 bool FindBadConstructsConsumer::HasPublicDtorCallback(
677 const CXXBaseSpecifier
* base
,
680 // Only examine paths that have public inheritance, as they are the
681 // only ones which will result in the destructor potentially being
682 // exposed. This check is largely redundant, as Chromium code should be
683 // exclusively using public inheritance.
684 if (path
.Access
!= AS_public
)
687 CXXRecordDecl
* record
=
688 dyn_cast
<CXXRecordDecl
>(base
->getType()->getAs
<RecordType
>()->getDecl());
689 SourceLocation unused
;
690 return None
!= CheckRecordForRefcountIssue(record
, unused
);
693 // Outputs a C++ inheritance chain as a diagnostic aid.
694 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath
& path
) {
695 for (CXXBasePath::const_iterator it
= path
.begin(); it
!= path
.end(); ++it
) {
696 diagnostic().Report(it
->Base
->getLocStart(), diag_note_inheritance_
)
697 << it
->Class
<< it
->Base
->getType();
701 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue
) {
703 case ImplicitDestructor
:
704 return diag_no_explicit_dtor_
;
705 case PublicDestructor
:
706 return diag_public_dtor_
;
708 assert(false && "Do not call DiagnosticForIssue with issue None");
715 // Check |record| to determine if it has any problematic refcounting
716 // issues and, if so, print them as warnings/errors based on the current
717 // value of getErrorLevel().
719 // If |record| is a C++ class, and if it inherits from one of the Chromium
720 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
721 // ensure that there are no public destructors in the class hierarchy. This
722 // is to guard against accidentally stack-allocating a RefCounted class or
723 // sticking it in a non-ref-counted container (like scoped_ptr<>).
724 void FindBadConstructsConsumer::CheckRefCountedDtors(
725 SourceLocation record_location
,
726 CXXRecordDecl
* record
) {
727 // Skip anonymous structs.
728 if (record
->getIdentifier() == NULL
)
731 // Determine if the current type is even ref-counted.
732 CXXBasePaths refcounted_path
;
733 if (!record
->lookupInBases(
734 [this](const CXXBaseSpecifier
* base
, CXXBasePath
& path
) {
735 return IsRefCounted(base
, path
);
738 return; // Class does not derive from a ref-counted base class.
741 // Easy check: Check to see if the current type is problematic.
743 RefcountIssue issue
= CheckRecordForRefcountIssue(record
, loc
);
745 diagnostic().Report(loc
, DiagnosticForIssue(issue
));
746 PrintInheritanceChain(refcounted_path
.front());
749 if (CXXDestructorDecl
* dtor
=
750 refcounted_path
.begin()->back().Class
->getDestructor()) {
751 if (dtor
->getAccess() == AS_protected
&& !dtor
->isVirtual()) {
752 loc
= dtor
->getInnerLocStart();
753 diagnostic().Report(loc
, diag_protected_non_virtual_dtor_
);
758 // Long check: Check all possible base classes for problematic
759 // destructors. This checks for situations involving multiple
760 // inheritance, where the ref-counted class may be implementing an
761 // interface that has a public or implicit destructor.
763 // struct SomeInterface {
764 // virtual void DoFoo();
767 // struct RefCountedInterface
768 // : public base::RefCounted<RefCountedInterface>,
769 // public SomeInterface {
771 // friend class base::Refcounted<RefCountedInterface>;
772 // virtual ~RefCountedInterface() {}
775 // While RefCountedInterface is "safe", in that its destructor is
776 // private, it's possible to do the following "unsafe" code:
777 // scoped_refptr<RefCountedInterface> some_class(
778 // new RefCountedInterface);
779 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
780 // delete static_cast<SomeInterface*>(some_class.get());
781 if (!options_
.check_base_classes
)
784 // Find all public destructors. This will record the class hierarchy
785 // that leads to the public destructor in |dtor_paths|.
786 CXXBasePaths dtor_paths
;
787 if (!record
->lookupInBases(
788 [](const CXXBaseSpecifier
* base
, CXXBasePath
& path
) {
789 // TODO(thakis): Inline HasPublicDtorCallback() after clang roll.
790 return HasPublicDtorCallback(base
, path
, nullptr);
796 for (CXXBasePaths::const_paths_iterator it
= dtor_paths
.begin();
797 it
!= dtor_paths
.end();
799 // The record with the problem will always be the last record
800 // in the path, since it is the record that stopped the search.
801 const CXXRecordDecl
* problem_record
= dyn_cast
<CXXRecordDecl
>(
802 it
->back().Base
->getType()->getAs
<RecordType
>()->getDecl());
804 issue
= CheckRecordForRefcountIssue(problem_record
, loc
);
806 if (issue
== ImplicitDestructor
) {
807 diagnostic().Report(record_location
, diag_no_explicit_dtor_
);
808 PrintInheritanceChain(refcounted_path
.front());
809 diagnostic().Report(loc
, diag_note_implicit_dtor_
) << problem_record
;
810 PrintInheritanceChain(*it
);
811 } else if (issue
== PublicDestructor
) {
812 diagnostic().Report(record_location
, diag_public_dtor_
);
813 PrintInheritanceChain(refcounted_path
.front());
814 diagnostic().Report(loc
, diag_note_public_dtor_
);
815 PrintInheritanceChain(*it
);
820 // Check for any problems with WeakPtrFactory class members. This currently
821 // only checks that any WeakPtrFactory<T> member of T appears as the last
822 // data member in T. We could consider checking for bad uses of
823 // WeakPtrFactory to refer to other data members, but that would require
824 // looking at the initializer list in constructors to see what the factory
826 // Note, if we later add other unrelated checks of data members, we should
827 // consider collapsing them in to one loop to avoid iterating over the data
828 // members more than once.
829 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
830 SourceLocation record_location
,
831 CXXRecordDecl
* record
) {
832 // Skip anonymous structs.
833 if (record
->getIdentifier() == NULL
)
836 // Iterate through members of the class.
837 RecordDecl::field_iterator
iter(record
->field_begin()),
838 the_end(record
->field_end());
839 SourceLocation weak_ptr_factory_location
; // Invalid initially.
840 for (; iter
!= the_end
; ++iter
) {
841 const TemplateSpecializationType
* template_spec_type
=
842 iter
->getType().getTypePtr()->getAs
<TemplateSpecializationType
>();
843 bool param_is_weak_ptr_factory_to_self
= false;
844 if (template_spec_type
) {
845 const TemplateDecl
* template_decl
=
846 template_spec_type
->getTemplateName().getAsTemplateDecl();
847 if (template_decl
&& template_spec_type
->getNumArgs() == 1) {
848 if (template_decl
->getNameAsString().compare("WeakPtrFactory") == 0 &&
849 GetNamespace(template_decl
) == "base") {
850 // Only consider WeakPtrFactory members which are specialized for the
852 const TemplateArgument
& arg
= template_spec_type
->getArg(0);
853 if (arg
.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
854 record
->getTypeForDecl()->getAsCXXRecordDecl()) {
855 if (!weak_ptr_factory_location
.isValid()) {
856 // Save the first matching WeakPtrFactory member for the
858 weak_ptr_factory_location
= iter
->getLocation();
860 param_is_weak_ptr_factory_to_self
= true;
865 // If we've already seen a WeakPtrFactory<OwningType> and this param is not
866 // one of those, it means there is at least one member after a factory.
867 if (weak_ptr_factory_location
.isValid() &&
868 !param_is_weak_ptr_factory_to_self
) {
869 diagnostic().Report(weak_ptr_factory_location
,
870 diag_weak_ptr_factory_order_
);
875 } // namespace chrome_checker