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