1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
|
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file defines a bunch of recurring problems in the Chromium C++ code.
//
// Checks that are implemented:
// - Constructors/Destructors should not be inlined if they are of a complex
// class type.
// - Missing "virtual" keywords on methods that should be virtual.
// - Non-annotated overriding virtual methods.
// - Virtual methods with nonempty implementations in their headers.
// - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
// should have protected or private destructors.
// - WeakPtrFactory members that refer to their outer class should be the last
// member.
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/AST.h"
#include "clang/AST/Attr.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendPluginRegistry.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/raw_ostream.h"
#include "ChromeClassTester.h"
using namespace clang;
namespace {
const char kMethodRequiresOverride[] =
"[chromium-style] Overriding method must be marked with OVERRIDE.";
const char kMethodRequiresVirtual[] =
"[chromium-style] Overriding method must have \"virtual\" keyword.";
const char kNoExplicitDtor[] =
"[chromium-style] Classes that are ref-counted should have explicit "
"destructors that are declared protected or private.";
const char kPublicDtor[] =
"[chromium-style] Classes that are ref-counted should have "
"destructors that are declared protected or private.";
const char kProtectedNonVirtualDtor[] =
"[chromium-style] Classes that are ref-counted and have non-private "
"destructors should declare their destructor virtual.";
const char kWeakPtrFactoryOrder[] =
"[chromium-style] WeakPtrFactory members which refer to their outer class "
"must be the last member in the outer class definition.";
const char kNoteInheritance[] =
"[chromium-style] %0 inherits from %1 here";
const char kNoteImplicitDtor[] =
"[chromium-style] No explicit destructor for %0 defined";
const char kNotePublicDtor[] =
"[chromium-style] Public destructor declared here";
const char kNoteProtectedNonVirtualDtor[] =
"[chromium-style] Protected non-virtual destructor declared here";
bool TypeHasNonTrivialDtor(const Type* type) {
if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
return cxx_r->hasTrivialDestructor();
return false;
}
// Returns the underlying Type for |type| by expanding typedefs and removing
// any namespace qualifiers. This is similar to desugaring, except that for
// ElaboratedTypes, desugar will unwrap too much.
const Type* UnwrapType(const Type* type) {
if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
return UnwrapType(elaborated->getNamedType().getTypePtr());
if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
return UnwrapType(typedefed->desugar().getTypePtr());
return type;
}
struct FindBadConstructsOptions {
FindBadConstructsOptions() : check_base_classes(false),
check_virtuals_in_implementations(true),
check_url_directory(false),
check_weak_ptr_factory_order(false) {
}
bool check_base_classes;
bool check_virtuals_in_implementations;
bool check_url_directory;
bool check_weak_ptr_factory_order;
};
// Searches for constructs that we know we don't want in the Chromium code base.
class FindBadConstructsConsumer : public ChromeClassTester {
public:
FindBadConstructsConsumer(CompilerInstance& instance,
const FindBadConstructsOptions& options)
: ChromeClassTester(instance, options.check_url_directory),
options_(options) {
// Register warning/error messages.
diag_method_requires_override_ = diagnostic().getCustomDiagID(
getErrorLevel(), kMethodRequiresOverride);
diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
getErrorLevel(), kMethodRequiresVirtual);
diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
getErrorLevel(), kNoExplicitDtor);
diag_public_dtor_ = diagnostic().getCustomDiagID(
getErrorLevel(), kPublicDtor);
diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
getErrorLevel(), kProtectedNonVirtualDtor);
diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
getErrorLevel(), kWeakPtrFactoryOrder);
// Registers notes to make it easier to interpret warnings.
diag_note_inheritance_ = diagnostic().getCustomDiagID(
DiagnosticsEngine::Note, kNoteInheritance);
diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
DiagnosticsEngine::Note, kNoteImplicitDtor);
diag_note_public_dtor_ = diagnostic().getCustomDiagID(
DiagnosticsEngine::Note, kNotePublicDtor);
diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
}
virtual void CheckChromeClass(SourceLocation record_location,
CXXRecordDecl* record) {
bool implementation_file = InImplementationFile(record_location);
if (!implementation_file) {
// Only check for "heavy" constructors/destructors in header files;
// within implementation files, there is no performance cost.
CheckCtorDtorWeight(record_location, record);
}
if (!implementation_file || options_.check_virtuals_in_implementations) {
bool warn_on_inline_bodies = !implementation_file;
// Check that all virtual methods are marked accordingly with both
// virtual and OVERRIDE.
CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
}
CheckRefCountedDtors(record_location, record);
if (options_.check_weak_ptr_factory_order)
CheckWeakPtrFactoryMembers(record_location, record);
}
private:
// The type of problematic ref-counting pattern that was encountered.
enum RefcountIssue {
None,
ImplicitDestructor,
PublicDestructor
};
FindBadConstructsOptions options_;
unsigned diag_method_requires_override_;
unsigned diag_method_requires_virtual_;
unsigned diag_no_explicit_dtor_;
unsigned diag_public_dtor_;
unsigned diag_protected_non_virtual_dtor_;
unsigned diag_weak_ptr_factory_order_;
unsigned diag_note_inheritance_;
unsigned diag_note_implicit_dtor_;
unsigned diag_note_public_dtor_;
unsigned diag_note_protected_non_virtual_dtor_;
// Prints errors if the constructor/destructor weight is too heavy.
void CheckCtorDtorWeight(SourceLocation record_location,
CXXRecordDecl* record) {
// We don't handle anonymous structs. If this record doesn't have a
// name, it's of the form:
//
// struct {
// ...
// } name_;
if (record->getIdentifier() == NULL)
return;
// Count the number of templated base classes as a feature of whether the
// destructor can be inlined.
int templated_base_classes = 0;
for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
it != record->bases_end(); ++it) {
if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
TypeLoc::TemplateSpecialization) {
++templated_base_classes;
}
}
// Count the number of trivial and non-trivial member variables.
int trivial_member = 0;
int non_trivial_member = 0;
int templated_non_trivial_member = 0;
for (RecordDecl::field_iterator it = record->field_begin();
it != record->field_end(); ++it) {
CountType(it->getType().getTypePtr(),
&trivial_member,
&non_trivial_member,
&templated_non_trivial_member);
}
// Check to see if we need to ban inlined/synthesized constructors. Note
// that the cutoffs here are kind of arbitrary. Scores over 10 break.
int dtor_score = 0;
// Deriving from a templated base class shouldn't be enough to trigger
// the ctor warning, but if you do *anything* else, it should.
//
// TODO(erg): This is motivated by templated base classes that don't have
// any data members. Somehow detect when templated base classes have data
// members and treat them differently.
dtor_score += templated_base_classes * 9;
// Instantiating a template is an insta-hit.
dtor_score += templated_non_trivial_member * 10;
// The fourth normal class member should trigger the warning.
dtor_score += non_trivial_member * 3;
int ctor_score = dtor_score;
// You should be able to have 9 ints before we warn you.
ctor_score += trivial_member;
if (ctor_score >= 10) {
if (!record->hasUserDeclaredConstructor()) {
emitWarning(record_location,
"Complex class/struct needs an explicit out-of-line "
"constructor.");
} else {
// Iterate across all the constructors in this file and yell if we
// find one that tries to be inline.
for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
it != record->ctor_end(); ++it) {
if (it->hasInlineBody()) {
if (it->isCopyConstructor() &&
!record->hasUserDeclaredCopyConstructor()) {
emitWarning(record_location,
"Complex class/struct needs an explicit out-of-line "
"copy constructor.");
} else {
emitWarning(it->getInnerLocStart(),
"Complex constructor has an inlined body.");
}
}
}
}
}
// The destructor side is equivalent except that we don't check for
// trivial members; 20 ints don't need a destructor.
if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
if (!record->hasUserDeclaredDestructor()) {
emitWarning(
record_location,
"Complex class/struct needs an explicit out-of-line "
"destructor.");
} else if (CXXDestructorDecl* dtor = record->getDestructor()) {
if (dtor->hasInlineBody()) {
emitWarning(dtor->getInnerLocStart(),
"Complex destructor has an inline body.");
}
}
}
}
void CheckVirtualMethod(const CXXMethodDecl* method,
bool warn_on_inline_bodies) {
if (!method->isVirtual())
return;
if (!method->isVirtualAsWritten()) {
SourceLocation loc = method->getTypeSpecStartLoc();
if (isa<CXXDestructorDecl>(method))
loc = method->getInnerLocStart();
SourceManager& manager = instance().getSourceManager();
FullSourceLoc full_loc(loc, manager);
SourceLocation spelling_loc = manager.getSpellingLoc(loc);
diagnostic().Report(full_loc, diag_method_requires_virtual_)
<< FixItHint::CreateInsertion(spelling_loc, "virtual ");
}
// Virtual methods should not have inline definitions beyond "{}". This
// only matters for header files.
if (warn_on_inline_bodies && method->hasBody() &&
method->hasInlineBody()) {
if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
if (cs->size()) {
emitWarning(
cs->getLBracLoc(),
"virtual methods with non-empty bodies shouldn't be "
"declared inline.");
}
}
}
}
bool InTestingNamespace(const Decl* record) {
return GetNamespace(record).find("testing") != std::string::npos;
}
bool IsMethodInBannedOrTestingNamespace(const CXXMethodDecl* method) {
if (InBannedNamespace(method))
return true;
for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
i != method->end_overridden_methods();
++i) {
const CXXMethodDecl* overridden = *i;
if (IsMethodInBannedOrTestingNamespace(overridden) ||
InTestingNamespace(overridden)) {
return true;
}
}
return false;
}
void CheckOverriddenMethod(const CXXMethodDecl* method) {
if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
return;
if (isa<CXXDestructorDecl>(method) || method->isPure())
return;
if (IsMethodInBannedOrTestingNamespace(method))
return;
SourceManager& manager = instance().getSourceManager();
SourceRange type_info_range =
method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
FullSourceLoc loc(type_info_range.getBegin(), manager);
// Build the FixIt insertion point after the end of the method definition,
// including any const-qualifiers and attributes, and before the opening
// of the l-curly-brace (if inline) or the semi-color (if a declaration).
SourceLocation spelling_end =
manager.getSpellingLoc(type_info_range.getEnd());
if (spelling_end.isValid()) {
SourceLocation token_end = Lexer::getLocForEndOfToken(
spelling_end, 0, manager, LangOptions());
diagnostic().Report(token_end, diag_method_requires_override_)
<< FixItHint::CreateInsertion(token_end, " OVERRIDE");
} else {
diagnostic().Report(loc, diag_method_requires_override_);
}
}
// Makes sure there is a "virtual" keyword on virtual methods.
//
// Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
// trick to get around that. If a class has member variables whose types are
// in the "testing" namespace (which is how gmock works behind the scenes),
// there's a really high chance we won't care about these errors
void CheckVirtualMethods(SourceLocation record_location,
CXXRecordDecl* record,
bool warn_on_inline_bodies) {
for (CXXRecordDecl::field_iterator it = record->field_begin();
it != record->field_end(); ++it) {
CXXRecordDecl* record_type =
it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
getAsCXXRecordDecl();
if (record_type) {
if (InTestingNamespace(record_type)) {
return;
}
}
}
for (CXXRecordDecl::method_iterator it = record->method_begin();
it != record->method_end(); ++it) {
if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
// Ignore constructors and assignment operators.
} else if (isa<CXXDestructorDecl>(*it) &&
!record->hasUserDeclaredDestructor()) {
// Ignore non-user-declared destructors.
} else {
CheckVirtualMethod(*it, warn_on_inline_bodies);
CheckOverriddenMethod(*it);
}
}
}
void CountType(const Type* type,
int* trivial_member,
int* non_trivial_member,
int* templated_non_trivial_member) {
switch (type->getTypeClass()) {
case Type::Record: {
// Simplifying; the whole class isn't trivial if the dtor is, but
// we use this as a signal about complexity.
if (TypeHasNonTrivialDtor(type))
(*trivial_member)++;
else
(*non_trivial_member)++;
break;
}
case Type::TemplateSpecialization: {
TemplateName name =
dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
bool whitelisted_template = false;
// HACK: I'm at a loss about how to get the syntax checker to get
// whether a template is exterened or not. For the first pass here,
// just do retarded string comparisons.
if (TemplateDecl* decl = name.getAsTemplateDecl()) {
std::string base_name = decl->getNameAsString();
if (base_name == "basic_string")
whitelisted_template = true;
}
if (whitelisted_template)
(*non_trivial_member)++;
else
(*templated_non_trivial_member)++;
break;
}
case Type::Elaborated: {
CountType(
dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
trivial_member, non_trivial_member, templated_non_trivial_member);
break;
}
case Type::Typedef: {
while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
type = TT->getDecl()->getUnderlyingType().getTypePtr();
}
CountType(type, trivial_member, non_trivial_member,
templated_non_trivial_member);
break;
}
default: {
// Stupid assumption: anything we see that isn't the above is one of
// the 20 integer types.
(*trivial_member)++;
break;
}
}
}
// Check |record| for issues that are problematic for ref-counted types.
// Note that |record| may not be a ref-counted type, but a base class for
// a type that is.
// If there are issues, update |loc| with the SourceLocation of the issue
// and returns appropriately, or returns None if there are no issues.
static RefcountIssue CheckRecordForRefcountIssue(
const CXXRecordDecl* record,
SourceLocation &loc) {
if (!record->hasUserDeclaredDestructor()) {
loc = record->getLocation();
return ImplicitDestructor;
}
if (CXXDestructorDecl* dtor = record->getDestructor()) {
if (dtor->getAccess() == AS_public) {
loc = dtor->getInnerLocStart();
return PublicDestructor;
}
}
return None;
}
// Adds either a warning or error, based on the current handling of
// -Werror.
DiagnosticsEngine::Level getErrorLevel() {
return diagnostic().getWarningsAsErrors() ?
DiagnosticsEngine::Error : DiagnosticsEngine::Warning;
}
// Returns true if |base| specifies one of the Chromium reference counted
// classes (base::RefCounted / base::RefCountedThreadSafe).
static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
CXXBasePath& path,
void* user_data) {
FindBadConstructsConsumer* self =
static_cast<FindBadConstructsConsumer*>(user_data);
const TemplateSpecializationType* base_type =
dyn_cast<TemplateSpecializationType>(
UnwrapType(base->getType().getTypePtr()));
if (!base_type) {
// Base-most definition is not a template, so this cannot derive from
// base::RefCounted. However, it may still be possible to use with a
// scoped_refptr<> and support ref-counting, so this is not a perfect
// guarantee of safety.
return false;
}
TemplateName name = base_type->getTemplateName();
if (TemplateDecl* decl = name.getAsTemplateDecl()) {
std::string base_name = decl->getNameAsString();
// Check for both base::RefCounted and base::RefCountedThreadSafe.
if (base_name.compare(0, 10, "RefCounted") == 0 &&
self->GetNamespace(decl) == "base") {
return true;
}
}
return false;
}
// Returns true if |base| specifies a class that has a public destructor,
// either explicitly or implicitly.
static bool HasPublicDtorCallback(const CXXBaseSpecifier* base,
CXXBasePath& path,
void* user_data) {
// Only examine paths that have public inheritance, as they are the
// only ones which will result in the destructor potentially being
// exposed. This check is largely redundant, as Chromium code should be
// exclusively using public inheritance.
if (path.Access != AS_public)
return false;
CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(
base->getType()->getAs<RecordType>()->getDecl());
SourceLocation unused;
return None != CheckRecordForRefcountIssue(record, unused);
}
// Outputs a C++ inheritance chain as a diagnostic aid.
void PrintInheritanceChain(const CXXBasePath& path) {
for (CXXBasePath::const_iterator it = path.begin(); it != path.end();
++it) {
diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
<< it->Class << it->Base->getType();
}
}
unsigned DiagnosticForIssue(RefcountIssue issue) {
switch (issue) {
case ImplicitDestructor:
return diag_no_explicit_dtor_;
case PublicDestructor:
return diag_public_dtor_;
case None:
assert(false && "Do not call DiagnosticForIssue with issue None");
return 0;
}
assert(false);
return 0;
}
// Check |record| to determine if it has any problematic refcounting
// issues and, if so, print them as warnings/errors based on the current
// value of getErrorLevel().
//
// If |record| is a C++ class, and if it inherits from one of the Chromium
// ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
// ensure that there are no public destructors in the class hierarchy. This
// is to guard against accidentally stack-allocating a RefCounted class or
// sticking it in a non-ref-counted container (like scoped_ptr<>).
void CheckRefCountedDtors(SourceLocation record_location,
CXXRecordDecl* record) {
// Skip anonymous structs.
if (record->getIdentifier() == NULL)
return;
// Determine if the current type is even ref-counted.
CXXBasePaths refcounted_path;
if (!record->lookupInBases(
&FindBadConstructsConsumer::IsRefCountedCallback, this,
refcounted_path)) {
return; // Class does not derive from a ref-counted base class.
}
// Easy check: Check to see if the current type is problematic.
SourceLocation loc;
RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
if (issue != None) {
diagnostic().Report(loc, DiagnosticForIssue(issue));
PrintInheritanceChain(refcounted_path.front());
return;
}
if (CXXDestructorDecl* dtor =
refcounted_path.begin()->back().Class->getDestructor()) {
if (dtor->getAccess() == AS_protected &&
!dtor->isVirtual()) {
loc = dtor->getInnerLocStart();
diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
return;
}
}
// Long check: Check all possible base classes for problematic
// destructors. This checks for situations involving multiple
// inheritance, where the ref-counted class may be implementing an
// interface that has a public or implicit destructor.
//
// struct SomeInterface {
// virtual void DoFoo();
// };
//
// struct RefCountedInterface
// : public base::RefCounted<RefCountedInterface>,
// public SomeInterface {
// private:
// friend class base::Refcounted<RefCountedInterface>;
// virtual ~RefCountedInterface() {}
// };
//
// While RefCountedInterface is "safe", in that its destructor is
// private, it's possible to do the following "unsafe" code:
// scoped_refptr<RefCountedInterface> some_class(
// new RefCountedInterface);
// // Calls SomeInterface::~SomeInterface(), which is unsafe.
// delete static_cast<SomeInterface*>(some_class.get());
if (!options_.check_base_classes)
return;
// Find all public destructors. This will record the class hierarchy
// that leads to the public destructor in |dtor_paths|.
CXXBasePaths dtor_paths;
if (!record->lookupInBases(
&FindBadConstructsConsumer::HasPublicDtorCallback, this,
dtor_paths)) {
return;
}
for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
it != dtor_paths.end(); ++it) {
// The record with the problem will always be the last record
// in the path, since it is the record that stopped the search.
const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
it->back().Base->getType()->getAs<RecordType>()->getDecl());
issue = CheckRecordForRefcountIssue(problem_record, loc);
if (issue == ImplicitDestructor) {
diagnostic().Report(record_location, diag_no_explicit_dtor_);
PrintInheritanceChain(refcounted_path.front());
diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
PrintInheritanceChain(*it);
} else if (issue == PublicDestructor) {
diagnostic().Report(record_location, diag_public_dtor_);
PrintInheritanceChain(refcounted_path.front());
diagnostic().Report(loc, diag_note_public_dtor_);
PrintInheritanceChain(*it);
}
}
}
// Check for any problems with WeakPtrFactory class members. This currently
// only checks that any WeakPtrFactory<T> member of T appears as the last
// data member in T. We could consider checking for bad uses of
// WeakPtrFactory to refer to other data members, but that would require
// looking at the initializer list in constructors to see what the factory
// points to.
// Note, if we later add other unrelated checks of data members, we should
// consider collapsing them in to one loop to avoid iterating over the data
// members more than once.
void CheckWeakPtrFactoryMembers(SourceLocation record_location,
CXXRecordDecl* record) {
// Skip anonymous structs.
if (record->getIdentifier() == NULL)
return;
// Iterate through members of the class.
RecordDecl::field_iterator iter(record->field_begin()),
the_end(record->field_end());
SourceLocation weak_ptr_factory_location; // Invalid initially.
for (; iter != the_end; ++iter) {
// If we enter the loop but have already seen a matching WeakPtrFactory,
// it means there is at least one member after the factory.
if (weak_ptr_factory_location.isValid()) {
diagnostic().Report(weak_ptr_factory_location,
diag_weak_ptr_factory_order_);
}
const TemplateSpecializationType* template_spec_type =
iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
if (template_spec_type) {
const TemplateDecl* template_decl =
template_spec_type->getTemplateName().getAsTemplateDecl();
if (template_decl && template_spec_type->getNumArgs() >= 1) {
if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
GetNamespace(template_decl) == "base") {
const TemplateArgument& arg = template_spec_type->getArg(0);
if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
record->getTypeForDecl()->getAsCXXRecordDecl()) {
weak_ptr_factory_location = iter->getLocation();
}
}
}
}
}
}
};
class FindBadConstructsAction : public PluginASTAction {
public:
FindBadConstructsAction() {
}
protected:
// Overridden from PluginASTAction:
virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
llvm::StringRef ref) {
return new FindBadConstructsConsumer(instance, options_);
}
virtual bool ParseArgs(const CompilerInstance& instance,
const std::vector<std::string>& args) {
bool parsed = true;
for (size_t i = 0; i < args.size() && parsed; ++i) {
if (args[i] == "skip-virtuals-in-implementations") {
// TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
options_.check_virtuals_in_implementations = false;
} else if (args[i] == "check-base-classes") {
// TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
options_.check_base_classes = true;
} else if (args[i] == "check-url-directory") {
// TODO(tfarina): Remove this once http://crbug.com/229660 is fixed.
options_.check_url_directory = true;
} else if (args[i] == "check-weak-ptr-factory-order") {
// TODO(dmichael): Remove this once http://crbug.com/303818 is fixed.
options_.check_weak_ptr_factory_order = true;
} else {
parsed = false;
llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
}
}
return parsed;
}
private:
FindBadConstructsOptions options_;
};
} // namespace
static FrontendPluginRegistry::Add<FindBadConstructsAction>
X("find-bad-constructs", "Finds bad C++ constructs");
|