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 // This file defines a bunch of recurring problems in the Chromium C++ code.
7 // Checks that are implemented:
8 // - Constructors/Destructors should not be inlined if they are of a complex
10 // - Missing "virtual" keywords on methods that should be virtual.
11 // - Non-annotated overriding virtual methods.
12 // - Virtual methods with nonempty implementations in their headers.
13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
14 // should have protected or private destructors.
15 // - WeakPtrFactory members that refer to their outer class should be the last
17 // - Enum types with a xxxx_LAST or xxxxLast const actually have that constant
18 // have the maximal value for that type.
20 #include "clang/AST/ASTConsumer.h"
21 #include "clang/AST/AST.h"
22 #include "clang/AST/Attr.h"
23 #include "clang/AST/CXXInheritance.h"
24 #include "clang/AST/TypeLoc.h"
25 #include "clang/Basic/SourceManager.h"
26 #include "clang/Frontend/CompilerInstance.h"
27 #include "clang/Frontend/FrontendPluginRegistry.h"
28 #include "clang/Lex/Lexer.h"
29 #include "llvm/Support/raw_ostream.h"
31 #include "ChromeClassTester.h"
33 using namespace clang
;
37 const char kMethodRequiresOverride
[] =
38 "[chromium-style] Overriding method must be marked with OVERRIDE.";
39 const char kMethodRequiresVirtual
[] =
40 "[chromium-style] Overriding method must have \"virtual\" keyword.";
41 const char kNoExplicitDtor
[] =
42 "[chromium-style] Classes that are ref-counted should have explicit "
43 "destructors that are declared protected or private.";
44 const char kPublicDtor
[] =
45 "[chromium-style] Classes that are ref-counted should have "
46 "destructors that are declared protected or private.";
47 const char kProtectedNonVirtualDtor
[] =
48 "[chromium-style] Classes that are ref-counted and have non-private "
49 "destructors should declare their destructor virtual.";
50 const char kWeakPtrFactoryOrder
[] =
51 "[chromium-style] WeakPtrFactory members which refer to their outer class "
52 "must be the last member in the outer class definition.";
53 const char kBadLastEnumValue
[] =
54 "[chromium-style] _LAST/Last constants of enum types must have the maximal "
55 "value for any constant of that type.";
56 const char kNoteInheritance
[] =
57 "[chromium-style] %0 inherits from %1 here";
58 const char kNoteImplicitDtor
[] =
59 "[chromium-style] No explicit destructor for %0 defined";
60 const char kNotePublicDtor
[] =
61 "[chromium-style] Public destructor declared here";
62 const char kNoteProtectedNonVirtualDtor
[] =
63 "[chromium-style] Protected non-virtual destructor declared here";
65 bool TypeHasNonTrivialDtor(const Type
* type
) {
66 if (const CXXRecordDecl
* cxx_r
= type
->getPointeeCXXRecordDecl())
67 return !cxx_r
->hasTrivialDestructor();
72 // Returns the underlying Type for |type| by expanding typedefs and removing
73 // any namespace qualifiers. This is similar to desugaring, except that for
74 // ElaboratedTypes, desugar will unwrap too much.
75 const Type
* UnwrapType(const Type
* type
) {
76 if (const ElaboratedType
* elaborated
= dyn_cast
<ElaboratedType
>(type
))
77 return UnwrapType(elaborated
->getNamedType().getTypePtr());
78 if (const TypedefType
* typedefed
= dyn_cast
<TypedefType
>(type
))
79 return UnwrapType(typedefed
->desugar().getTypePtr());
83 struct FindBadConstructsOptions
{
84 FindBadConstructsOptions() : check_base_classes(false),
85 check_virtuals_in_implementations(true),
86 check_weak_ptr_factory_order(false),
87 check_enum_last_value(false) {
89 bool check_base_classes
;
90 bool check_virtuals_in_implementations
;
91 bool check_weak_ptr_factory_order
;
92 bool check_enum_last_value
;
95 // Searches for constructs that we know we don't want in the Chromium code base.
96 class FindBadConstructsConsumer
: public ChromeClassTester
{
98 FindBadConstructsConsumer(CompilerInstance
& instance
,
99 const FindBadConstructsOptions
& options
)
100 : ChromeClassTester(instance
),
102 // Register warning/error messages.
103 diag_method_requires_override_
= diagnostic().getCustomDiagID(
104 getErrorLevel(), kMethodRequiresOverride
);
105 diag_method_requires_virtual_
= diagnostic().getCustomDiagID(
106 getErrorLevel(), kMethodRequiresVirtual
);
107 diag_no_explicit_dtor_
= diagnostic().getCustomDiagID(
108 getErrorLevel(), kNoExplicitDtor
);
109 diag_public_dtor_
= diagnostic().getCustomDiagID(
110 getErrorLevel(), kPublicDtor
);
111 diag_protected_non_virtual_dtor_
= diagnostic().getCustomDiagID(
112 getErrorLevel(), kProtectedNonVirtualDtor
);
113 diag_weak_ptr_factory_order_
= diagnostic().getCustomDiagID(
114 getErrorLevel(), kWeakPtrFactoryOrder
);
115 diag_bad_enum_last_value_
= diagnostic().getCustomDiagID(
116 getErrorLevel(), kBadLastEnumValue
);
118 // Registers notes to make it easier to interpret warnings.
119 diag_note_inheritance_
= diagnostic().getCustomDiagID(
120 DiagnosticsEngine::Note
, kNoteInheritance
);
121 diag_note_implicit_dtor_
= diagnostic().getCustomDiagID(
122 DiagnosticsEngine::Note
, kNoteImplicitDtor
);
123 diag_note_public_dtor_
= diagnostic().getCustomDiagID(
124 DiagnosticsEngine::Note
, kNotePublicDtor
);
125 diag_note_protected_non_virtual_dtor_
= diagnostic().getCustomDiagID(
126 DiagnosticsEngine::Note
, kNoteProtectedNonVirtualDtor
);
129 virtual void CheckChromeClass(SourceLocation record_location
,
130 CXXRecordDecl
* record
) {
131 bool implementation_file
= InImplementationFile(record_location
);
133 if (!implementation_file
) {
134 // Only check for "heavy" constructors/destructors in header files;
135 // within implementation files, there is no performance cost.
136 CheckCtorDtorWeight(record_location
, record
);
139 if (!implementation_file
|| options_
.check_virtuals_in_implementations
) {
140 bool warn_on_inline_bodies
= !implementation_file
;
142 // Check that all virtual methods are marked accordingly with both
143 // virtual and OVERRIDE.
144 CheckVirtualMethods(record_location
, record
, warn_on_inline_bodies
);
147 CheckRefCountedDtors(record_location
, record
);
149 if (options_
.check_weak_ptr_factory_order
)
150 CheckWeakPtrFactoryMembers(record_location
, record
);
153 virtual void CheckChromeEnum(SourceLocation enum_location
,
154 EnumDecl
* enum_decl
) {
155 if (!options_
.check_enum_last_value
)
158 bool got_one
= false;
159 bool is_signed
= false;
160 llvm::APSInt max_so_far
;
161 EnumDecl::enumerator_iterator iter
;
162 for (iter
= enum_decl
->enumerator_begin();
163 iter
!= enum_decl
->enumerator_end(); ++iter
) {
164 llvm::APSInt current_value
= iter
->getInitVal();
166 max_so_far
= current_value
;
167 is_signed
= current_value
.isSigned();
170 if (is_signed
!= current_value
.isSigned()) {
171 // This only happens in some cases when compiling C (not C++) files,
172 // so it is OK to bail out here.
175 if (current_value
> max_so_far
)
176 max_so_far
= current_value
;
179 for (iter
= enum_decl
->enumerator_begin();
180 iter
!= enum_decl
->enumerator_end(); ++iter
) {
181 std::string name
= iter
->getNameAsString();
182 if (((name
.size() > 4 &&
183 name
.compare(name
.size() - 4, 4, "Last") == 0) ||
185 name
.compare(name
.size() - 5, 5, "_LAST") == 0)) &&
186 iter
->getInitVal() < max_so_far
) {
187 diagnostic().Report(iter
->getLocation(), diag_bad_enum_last_value_
);
193 // The type of problematic ref-counting pattern that was encountered.
200 FindBadConstructsOptions options_
;
202 unsigned diag_method_requires_override_
;
203 unsigned diag_method_requires_virtual_
;
204 unsigned diag_no_explicit_dtor_
;
205 unsigned diag_public_dtor_
;
206 unsigned diag_protected_non_virtual_dtor_
;
207 unsigned diag_weak_ptr_factory_order_
;
208 unsigned diag_bad_enum_last_value_
;
209 unsigned diag_note_inheritance_
;
210 unsigned diag_note_implicit_dtor_
;
211 unsigned diag_note_public_dtor_
;
212 unsigned diag_note_protected_non_virtual_dtor_
;
214 // Prints errors if the constructor/destructor weight is too heavy.
215 void CheckCtorDtorWeight(SourceLocation record_location
,
216 CXXRecordDecl
* record
) {
217 // We don't handle anonymous structs. If this record doesn't have a
218 // name, it's of the form:
223 if (record
->getIdentifier() == NULL
)
226 // Count the number of templated base classes as a feature of whether the
227 // destructor can be inlined.
228 int templated_base_classes
= 0;
229 for (CXXRecordDecl::base_class_const_iterator it
= record
->bases_begin();
230 it
!= record
->bases_end(); ++it
) {
231 if (it
->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
232 TypeLoc::TemplateSpecialization
) {
233 ++templated_base_classes
;
237 // Count the number of trivial and non-trivial member variables.
238 int trivial_member
= 0;
239 int non_trivial_member
= 0;
240 int templated_non_trivial_member
= 0;
241 for (RecordDecl::field_iterator it
= record
->field_begin();
242 it
!= record
->field_end(); ++it
) {
243 CountType(it
->getType().getTypePtr(),
246 &templated_non_trivial_member
);
249 // Check to see if we need to ban inlined/synthesized constructors. Note
250 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
252 // Deriving from a templated base class shouldn't be enough to trigger
253 // the ctor warning, but if you do *anything* else, it should.
255 // TODO(erg): This is motivated by templated base classes that don't have
256 // any data members. Somehow detect when templated base classes have data
257 // members and treat them differently.
258 dtor_score
+= templated_base_classes
* 9;
259 // Instantiating a template is an insta-hit.
260 dtor_score
+= templated_non_trivial_member
* 10;
261 // The fourth normal class member should trigger the warning.
262 dtor_score
+= non_trivial_member
* 3;
264 int ctor_score
= dtor_score
;
265 // You should be able to have 9 ints before we warn you.
266 ctor_score
+= trivial_member
;
268 if (ctor_score
>= 10) {
269 if (!record
->hasUserDeclaredConstructor()) {
270 emitWarning(record_location
,
271 "Complex class/struct needs an explicit out-of-line "
274 // Iterate across all the constructors in this file and yell if we
275 // find one that tries to be inline.
276 for (CXXRecordDecl::ctor_iterator it
= record
->ctor_begin();
277 it
!= record
->ctor_end(); ++it
) {
278 if (it
->hasInlineBody()) {
279 if (it
->isCopyConstructor() &&
280 !record
->hasUserDeclaredCopyConstructor()) {
281 emitWarning(record_location
,
282 "Complex class/struct needs an explicit out-of-line "
283 "copy constructor.");
285 emitWarning(it
->getInnerLocStart(),
286 "Complex constructor has an inlined body.");
293 // The destructor side is equivalent except that we don't check for
294 // trivial members; 20 ints don't need a destructor.
295 if (dtor_score
>= 10 && !record
->hasTrivialDestructor()) {
296 if (!record
->hasUserDeclaredDestructor()) {
299 "Complex class/struct needs an explicit out-of-line "
301 } else if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
302 if (dtor
->hasInlineBody()) {
303 emitWarning(dtor
->getInnerLocStart(),
304 "Complex destructor has an inline body.");
310 void CheckVirtualMethod(const CXXMethodDecl
* method
,
311 bool warn_on_inline_bodies
) {
312 if (!method
->isVirtual())
315 if (!method
->isVirtualAsWritten()) {
316 SourceLocation loc
= method
->getTypeSpecStartLoc();
317 if (isa
<CXXDestructorDecl
>(method
))
318 loc
= method
->getInnerLocStart();
319 SourceManager
& manager
= instance().getSourceManager();
320 FullSourceLoc
full_loc(loc
, manager
);
321 SourceLocation spelling_loc
= manager
.getSpellingLoc(loc
);
322 diagnostic().Report(full_loc
, diag_method_requires_virtual_
)
323 << FixItHint::CreateInsertion(spelling_loc
, "virtual ");
326 // Virtual methods should not have inline definitions beyond "{}". This
327 // only matters for header files.
328 if (warn_on_inline_bodies
&& method
->hasBody() &&
329 method
->hasInlineBody()) {
330 if (CompoundStmt
* cs
= dyn_cast
<CompoundStmt
>(method
->getBody())) {
334 "virtual methods with non-empty bodies shouldn't be "
341 bool InTestingNamespace(const Decl
* record
) {
342 return GetNamespace(record
).find("testing") != std::string::npos
;
345 bool IsMethodInBannedOrTestingNamespace(const CXXMethodDecl
* method
) {
346 if (InBannedNamespace(method
))
348 for (CXXMethodDecl::method_iterator i
= method
->begin_overridden_methods();
349 i
!= method
->end_overridden_methods();
351 const CXXMethodDecl
* overridden
= *i
;
352 if (IsMethodInBannedOrTestingNamespace(overridden
) ||
353 InTestingNamespace(overridden
)) {
361 void CheckOverriddenMethod(const CXXMethodDecl
* method
) {
362 if (!method
->size_overridden_methods() || method
->getAttr
<OverrideAttr
>())
365 if (isa
<CXXDestructorDecl
>(method
) || method
->isPure())
368 if (IsMethodInBannedOrTestingNamespace(method
))
371 SourceManager
& manager
= instance().getSourceManager();
372 SourceRange type_info_range
=
373 method
->getTypeSourceInfo()->getTypeLoc().getSourceRange();
374 FullSourceLoc
loc(type_info_range
.getBegin(), manager
);
376 // Build the FixIt insertion point after the end of the method definition,
377 // including any const-qualifiers and attributes, and before the opening
378 // of the l-curly-brace (if inline) or the semi-color (if a declaration).
379 SourceLocation spelling_end
=
380 manager
.getSpellingLoc(type_info_range
.getEnd());
381 if (spelling_end
.isValid()) {
382 SourceLocation token_end
= Lexer::getLocForEndOfToken(
383 spelling_end
, 0, manager
, LangOptions());
384 diagnostic().Report(token_end
, diag_method_requires_override_
)
385 << FixItHint::CreateInsertion(token_end
, " OVERRIDE");
387 diagnostic().Report(loc
, diag_method_requires_override_
);
391 // Makes sure there is a "virtual" keyword on virtual methods.
393 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
394 // trick to get around that. If a class has member variables whose types are
395 // in the "testing" namespace (which is how gmock works behind the scenes),
396 // there's a really high chance we won't care about these errors
397 void CheckVirtualMethods(SourceLocation record_location
,
398 CXXRecordDecl
* record
,
399 bool warn_on_inline_bodies
) {
400 for (CXXRecordDecl::field_iterator it
= record
->field_begin();
401 it
!= record
->field_end(); ++it
) {
402 CXXRecordDecl
* record_type
=
403 it
->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
404 getAsCXXRecordDecl();
406 if (InTestingNamespace(record_type
)) {
412 for (CXXRecordDecl::method_iterator it
= record
->method_begin();
413 it
!= record
->method_end(); ++it
) {
414 if (it
->isCopyAssignmentOperator() || isa
<CXXConstructorDecl
>(*it
)) {
415 // Ignore constructors and assignment operators.
416 } else if (isa
<CXXDestructorDecl
>(*it
) &&
417 !record
->hasUserDeclaredDestructor()) {
418 // Ignore non-user-declared destructors.
420 CheckVirtualMethod(*it
, warn_on_inline_bodies
);
421 CheckOverriddenMethod(*it
);
426 void CountType(const Type
* type
,
428 int* non_trivial_member
,
429 int* templated_non_trivial_member
) {
430 switch (type
->getTypeClass()) {
432 // Simplifying; the whole class isn't trivial if the dtor is, but
433 // we use this as a signal about complexity.
434 if (TypeHasNonTrivialDtor(type
))
437 (*non_trivial_member
)++;
440 case Type::TemplateSpecialization
: {
442 dyn_cast
<TemplateSpecializationType
>(type
)->getTemplateName();
443 bool whitelisted_template
= false;
445 // HACK: I'm at a loss about how to get the syntax checker to get
446 // whether a template is exterened or not. For the first pass here,
447 // just do retarded string comparisons.
448 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
449 std::string base_name
= decl
->getNameAsString();
450 if (base_name
== "basic_string")
451 whitelisted_template
= true;
454 if (whitelisted_template
)
455 (*non_trivial_member
)++;
457 (*templated_non_trivial_member
)++;
460 case Type::Elaborated
: {
462 dyn_cast
<ElaboratedType
>(type
)->getNamedType().getTypePtr(),
463 trivial_member
, non_trivial_member
, templated_non_trivial_member
);
466 case Type::Typedef
: {
467 while (const TypedefType
* TT
= dyn_cast
<TypedefType
>(type
)) {
468 type
= TT
->getDecl()->getUnderlyingType().getTypePtr();
470 CountType(type
, trivial_member
, non_trivial_member
,
471 templated_non_trivial_member
);
475 // Stupid assumption: anything we see that isn't the above is one of
476 // the 20 integer types.
483 // Check |record| for issues that are problematic for ref-counted types.
484 // Note that |record| may not be a ref-counted type, but a base class for
486 // If there are issues, update |loc| with the SourceLocation of the issue
487 // and returns appropriately, or returns None if there are no issues.
488 static RefcountIssue
CheckRecordForRefcountIssue(
489 const CXXRecordDecl
* record
,
490 SourceLocation
&loc
) {
491 if (!record
->hasUserDeclaredDestructor()) {
492 loc
= record
->getLocation();
493 return ImplicitDestructor
;
496 if (CXXDestructorDecl
* dtor
= record
->getDestructor()) {
497 if (dtor
->getAccess() == AS_public
) {
498 loc
= dtor
->getInnerLocStart();
499 return PublicDestructor
;
506 // Adds either a warning or error, based on the current handling of
508 DiagnosticsEngine::Level
getErrorLevel() {
509 return diagnostic().getWarningsAsErrors() ?
510 DiagnosticsEngine::Error
: DiagnosticsEngine::Warning
;
513 // Returns true if |base| specifies one of the Chromium reference counted
514 // classes (base::RefCounted / base::RefCountedThreadSafe).
515 static bool IsRefCountedCallback(const CXXBaseSpecifier
* base
,
518 FindBadConstructsConsumer
* self
=
519 static_cast<FindBadConstructsConsumer
*>(user_data
);
521 const TemplateSpecializationType
* base_type
=
522 dyn_cast
<TemplateSpecializationType
>(
523 UnwrapType(base
->getType().getTypePtr()));
525 // Base-most definition is not a template, so this cannot derive from
526 // base::RefCounted. However, it may still be possible to use with a
527 // scoped_refptr<> and support ref-counting, so this is not a perfect
528 // guarantee of safety.
532 TemplateName name
= base_type
->getTemplateName();
533 if (TemplateDecl
* decl
= name
.getAsTemplateDecl()) {
534 std::string base_name
= decl
->getNameAsString();
536 // Check for both base::RefCounted and base::RefCountedThreadSafe.
537 if (base_name
.compare(0, 10, "RefCounted") == 0 &&
538 self
->GetNamespace(decl
) == "base") {
546 // Returns true if |base| specifies a class that has a public destructor,
547 // either explicitly or implicitly.
548 static bool HasPublicDtorCallback(const CXXBaseSpecifier
* base
,
551 // Only examine paths that have public inheritance, as they are the
552 // only ones which will result in the destructor potentially being
553 // exposed. This check is largely redundant, as Chromium code should be
554 // exclusively using public inheritance.
555 if (path
.Access
!= AS_public
)
558 CXXRecordDecl
* record
= dyn_cast
<CXXRecordDecl
>(
559 base
->getType()->getAs
<RecordType
>()->getDecl());
560 SourceLocation unused
;
561 return None
!= CheckRecordForRefcountIssue(record
, unused
);
564 // Outputs a C++ inheritance chain as a diagnostic aid.
565 void PrintInheritanceChain(const CXXBasePath
& path
) {
566 for (CXXBasePath::const_iterator it
= path
.begin(); it
!= path
.end();
568 diagnostic().Report(it
->Base
->getLocStart(), diag_note_inheritance_
)
569 << it
->Class
<< it
->Base
->getType();
573 unsigned DiagnosticForIssue(RefcountIssue issue
) {
575 case ImplicitDestructor
:
576 return diag_no_explicit_dtor_
;
577 case PublicDestructor
:
578 return diag_public_dtor_
;
580 assert(false && "Do not call DiagnosticForIssue with issue None");
587 // Check |record| to determine if it has any problematic refcounting
588 // issues and, if so, print them as warnings/errors based on the current
589 // value of getErrorLevel().
591 // If |record| is a C++ class, and if it inherits from one of the Chromium
592 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
593 // ensure that there are no public destructors in the class hierarchy. This
594 // is to guard against accidentally stack-allocating a RefCounted class or
595 // sticking it in a non-ref-counted container (like scoped_ptr<>).
596 void CheckRefCountedDtors(SourceLocation record_location
,
597 CXXRecordDecl
* record
) {
598 // Skip anonymous structs.
599 if (record
->getIdentifier() == NULL
)
602 // Determine if the current type is even ref-counted.
603 CXXBasePaths refcounted_path
;
604 if (!record
->lookupInBases(
605 &FindBadConstructsConsumer::IsRefCountedCallback
, this,
607 return; // Class does not derive from a ref-counted base class.
610 // Easy check: Check to see if the current type is problematic.
612 RefcountIssue issue
= CheckRecordForRefcountIssue(record
, loc
);
614 diagnostic().Report(loc
, DiagnosticForIssue(issue
));
615 PrintInheritanceChain(refcounted_path
.front());
618 if (CXXDestructorDecl
* dtor
=
619 refcounted_path
.begin()->back().Class
->getDestructor()) {
620 if (dtor
->getAccess() == AS_protected
&&
621 !dtor
->isVirtual()) {
622 loc
= dtor
->getInnerLocStart();
623 diagnostic().Report(loc
, diag_protected_non_virtual_dtor_
);
628 // Long check: Check all possible base classes for problematic
629 // destructors. This checks for situations involving multiple
630 // inheritance, where the ref-counted class may be implementing an
631 // interface that has a public or implicit destructor.
633 // struct SomeInterface {
634 // virtual void DoFoo();
637 // struct RefCountedInterface
638 // : public base::RefCounted<RefCountedInterface>,
639 // public SomeInterface {
641 // friend class base::Refcounted<RefCountedInterface>;
642 // virtual ~RefCountedInterface() {}
645 // While RefCountedInterface is "safe", in that its destructor is
646 // private, it's possible to do the following "unsafe" code:
647 // scoped_refptr<RefCountedInterface> some_class(
648 // new RefCountedInterface);
649 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
650 // delete static_cast<SomeInterface*>(some_class.get());
651 if (!options_
.check_base_classes
)
654 // Find all public destructors. This will record the class hierarchy
655 // that leads to the public destructor in |dtor_paths|.
656 CXXBasePaths dtor_paths
;
657 if (!record
->lookupInBases(
658 &FindBadConstructsConsumer::HasPublicDtorCallback
, this,
663 for (CXXBasePaths::const_paths_iterator it
= dtor_paths
.begin();
664 it
!= dtor_paths
.end(); ++it
) {
665 // The record with the problem will always be the last record
666 // in the path, since it is the record that stopped the search.
667 const CXXRecordDecl
* problem_record
= dyn_cast
<CXXRecordDecl
>(
668 it
->back().Base
->getType()->getAs
<RecordType
>()->getDecl());
670 issue
= CheckRecordForRefcountIssue(problem_record
, loc
);
672 if (issue
== ImplicitDestructor
) {
673 diagnostic().Report(record_location
, diag_no_explicit_dtor_
);
674 PrintInheritanceChain(refcounted_path
.front());
675 diagnostic().Report(loc
, diag_note_implicit_dtor_
) << problem_record
;
676 PrintInheritanceChain(*it
);
677 } else if (issue
== PublicDestructor
) {
678 diagnostic().Report(record_location
, diag_public_dtor_
);
679 PrintInheritanceChain(refcounted_path
.front());
680 diagnostic().Report(loc
, diag_note_public_dtor_
);
681 PrintInheritanceChain(*it
);
686 // Check for any problems with WeakPtrFactory class members. This currently
687 // only checks that any WeakPtrFactory<T> member of T appears as the last
688 // data member in T. We could consider checking for bad uses of
689 // WeakPtrFactory to refer to other data members, but that would require
690 // looking at the initializer list in constructors to see what the factory
692 // Note, if we later add other unrelated checks of data members, we should
693 // consider collapsing them in to one loop to avoid iterating over the data
694 // members more than once.
695 void CheckWeakPtrFactoryMembers(SourceLocation record_location
,
696 CXXRecordDecl
* record
) {
697 // Skip anonymous structs.
698 if (record
->getIdentifier() == NULL
)
701 // Iterate through members of the class.
702 RecordDecl::field_iterator
iter(record
->field_begin()),
703 the_end(record
->field_end());
704 SourceLocation weak_ptr_factory_location
; // Invalid initially.
705 for (; iter
!= the_end
; ++iter
) {
706 // If we enter the loop but have already seen a matching WeakPtrFactory,
707 // it means there is at least one member after the factory.
708 if (weak_ptr_factory_location
.isValid()) {
709 diagnostic().Report(weak_ptr_factory_location
,
710 diag_weak_ptr_factory_order_
);
712 const TemplateSpecializationType
* template_spec_type
=
713 iter
->getType().getTypePtr()->getAs
<TemplateSpecializationType
>();
714 if (template_spec_type
) {
715 const TemplateDecl
* template_decl
=
716 template_spec_type
->getTemplateName().getAsTemplateDecl();
717 if (template_decl
&& template_spec_type
->getNumArgs() >= 1) {
718 if (template_decl
->getNameAsString().compare("WeakPtrFactory") == 0 &&
719 GetNamespace(template_decl
) == "base") {
720 const TemplateArgument
& arg
= template_spec_type
->getArg(0);
721 if (arg
.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
722 record
->getTypeForDecl()->getAsCXXRecordDecl()) {
723 weak_ptr_factory_location
= iter
->getLocation();
733 class FindBadConstructsAction
: public PluginASTAction
{
735 FindBadConstructsAction() {
739 // Overridden from PluginASTAction:
740 virtual ASTConsumer
* CreateASTConsumer(CompilerInstance
& instance
,
741 llvm::StringRef ref
) {
742 return new FindBadConstructsConsumer(instance
, options_
);
745 virtual bool ParseArgs(const CompilerInstance
& instance
,
746 const std::vector
<std::string
>& args
) {
749 for (size_t i
= 0; i
< args
.size() && parsed
; ++i
) {
750 if (args
[i
] == "skip-virtuals-in-implementations") {
751 // TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
752 options_
.check_virtuals_in_implementations
= false;
753 } else if (args
[i
] == "check-base-classes") {
754 // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
755 options_
.check_base_classes
= true;
756 } else if (args
[i
] == "check-weak-ptr-factory-order") {
757 // TODO(dmichael): Remove this once http://crbug.com/303818 is fixed.
758 options_
.check_weak_ptr_factory_order
= true;
759 } else if (args
[i
] == "check-enum-last-value") {
760 // TODO(tsepez): Enable this by default once http://crbug.com/356815
761 // and http://crbug.com/356816 are fixed.
762 options_
.check_enum_last_value
= true;
765 llvm::errs() << "Unknown clang plugin argument: " << args
[i
] << "\n";
773 FindBadConstructsOptions options_
;
778 static FrontendPluginRegistry::Add
<FindBadConstructsAction
>
779 X("find-bad-constructs", "Finds bad C++ constructs");