media/webrtc/trunk/tools/clang/plugins/FindBadConstructs.cpp

Fri, 16 Jan 2015 18:13:44 +0100

author
Michael Schloh von Bennewitz <michael@schloh.com>
date
Fri, 16 Jan 2015 18:13:44 +0100
branch
TOR_BUG_9701
changeset 14
925c144e1f1f
permissions
-rw-r--r--

Integrate suggestion from review to improve consistency with existing code.

michael@0 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
michael@0 2 // Use of this source code is governed by a BSD-style license that can be
michael@0 3 // found in the LICENSE file.
michael@0 4
michael@0 5 // This file defines a bunch of recurring problems in the Chromium C++ code.
michael@0 6 //
michael@0 7 // Checks that are implemented:
michael@0 8 // - Constructors/Destructors should not be inlined if they are of a complex
michael@0 9 // class type.
michael@0 10 // - Missing "virtual" keywords on methods that should be virtual.
michael@0 11 // - Non-annotated overriding virtual methods.
michael@0 12 // - Virtual methods with nonempty implementations in their headers.
michael@0 13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
michael@0 14 // should have protected or private destructors.
michael@0 15
michael@0 16 #include "clang/Frontend/FrontendPluginRegistry.h"
michael@0 17 #include "clang/AST/ASTConsumer.h"
michael@0 18 #include "clang/AST/AST.h"
michael@0 19 #include "clang/AST/CXXInheritance.h"
michael@0 20 #include "clang/AST/TypeLoc.h"
michael@0 21 #include "clang/Basic/SourceManager.h"
michael@0 22 #include "clang/Frontend/CompilerInstance.h"
michael@0 23 #include "llvm/Support/raw_ostream.h"
michael@0 24
michael@0 25 #include "ChromeClassTester.h"
michael@0 26
michael@0 27 using namespace clang;
michael@0 28
michael@0 29 namespace {
michael@0 30
michael@0 31 bool TypeHasNonTrivialDtor(const Type* type) {
michael@0 32 if (const CXXRecordDecl* cxx_r = type->getCXXRecordDeclForPointerType())
michael@0 33 return cxx_r->hasTrivialDestructor();
michael@0 34
michael@0 35 return false;
michael@0 36 }
michael@0 37
michael@0 38 // Returns the underlying Type for |type| by expanding typedefs and removing
michael@0 39 // any namespace qualifiers.
michael@0 40 const Type* UnwrapType(const Type* type) {
michael@0 41 if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
michael@0 42 return UnwrapType(elaborated->getNamedType().getTypePtr());
michael@0 43 if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
michael@0 44 return UnwrapType(typedefed->desugar().getTypePtr());
michael@0 45 return type;
michael@0 46 }
michael@0 47
michael@0 48 // Searches for constructs that we know we don't want in the Chromium code base.
michael@0 49 class FindBadConstructsConsumer : public ChromeClassTester {
michael@0 50 public:
michael@0 51 FindBadConstructsConsumer(CompilerInstance& instance,
michael@0 52 bool check_refcounted_dtors,
michael@0 53 bool check_virtuals_in_implementations)
michael@0 54 : ChromeClassTester(instance),
michael@0 55 check_refcounted_dtors_(check_refcounted_dtors),
michael@0 56 check_virtuals_in_implementations_(check_virtuals_in_implementations) {
michael@0 57 }
michael@0 58
michael@0 59 virtual void CheckChromeClass(SourceLocation record_location,
michael@0 60 CXXRecordDecl* record) {
michael@0 61 bool implementation_file = InImplementationFile(record_location);
michael@0 62
michael@0 63 if (!implementation_file) {
michael@0 64 // Only check for "heavy" constructors/destructors in header files;
michael@0 65 // within implementation files, there is no performance cost.
michael@0 66 CheckCtorDtorWeight(record_location, record);
michael@0 67 }
michael@0 68
michael@0 69 if (!implementation_file || check_virtuals_in_implementations_) {
michael@0 70 bool warn_on_inline_bodies = !implementation_file;
michael@0 71
michael@0 72 // Check that all virtual methods are marked accordingly with both
michael@0 73 // virtual and OVERRIDE.
michael@0 74 CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
michael@0 75 }
michael@0 76
michael@0 77 if (check_refcounted_dtors_)
michael@0 78 CheckRefCountedDtors(record_location, record);
michael@0 79 }
michael@0 80
michael@0 81 private:
michael@0 82 bool check_refcounted_dtors_;
michael@0 83 bool check_virtuals_in_implementations_;
michael@0 84
michael@0 85 // Returns true if |base| specifies one of the Chromium reference counted
michael@0 86 // classes (base::RefCounted / base::RefCountedThreadSafe). |user_data| is
michael@0 87 // ignored.
michael@0 88 static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
michael@0 89 CXXBasePath& path,
michael@0 90 void* user_data) {
michael@0 91 FindBadConstructsConsumer* self =
michael@0 92 static_cast<FindBadConstructsConsumer*>(user_data);
michael@0 93
michael@0 94 const TemplateSpecializationType* base_type =
michael@0 95 dyn_cast<TemplateSpecializationType>(
michael@0 96 UnwrapType(base->getType().getTypePtr()));
michael@0 97 if (!base_type) {
michael@0 98 // Base-most definition is not a template, so this cannot derive from
michael@0 99 // base::RefCounted. However, it may still be possible to use with a
michael@0 100 // scoped_refptr<> and support ref-counting, so this is not a perfect
michael@0 101 // guarantee of safety.
michael@0 102 return false;
michael@0 103 }
michael@0 104
michael@0 105 TemplateName name = base_type->getTemplateName();
michael@0 106 if (TemplateDecl* decl = name.getAsTemplateDecl()) {
michael@0 107 std::string base_name = decl->getNameAsString();
michael@0 108
michael@0 109 // Check for both base::RefCounted and base::RefCountedThreadSafe.
michael@0 110 if (base_name.compare(0, 10, "RefCounted") == 0 &&
michael@0 111 self->GetNamespace(decl) == "base") {
michael@0 112 return true;
michael@0 113 }
michael@0 114 }
michael@0 115 return false;
michael@0 116 }
michael@0 117
michael@0 118 // Prints errors if the destructor of a RefCounted class is public.
michael@0 119 void CheckRefCountedDtors(SourceLocation record_location,
michael@0 120 CXXRecordDecl* record) {
michael@0 121 // Skip anonymous structs.
michael@0 122 if (record->getIdentifier() == NULL)
michael@0 123 return;
michael@0 124
michael@0 125 CXXBasePaths paths;
michael@0 126 if (!record->lookupInBases(
michael@0 127 &FindBadConstructsConsumer::IsRefCountedCallback, this, paths)) {
michael@0 128 return; // Class does not derive from a ref-counted base class.
michael@0 129 }
michael@0 130
michael@0 131 if (!record->hasUserDeclaredDestructor()) {
michael@0 132 emitWarning(
michael@0 133 record_location,
michael@0 134 "Classes that are ref-counted should have explicit "
michael@0 135 "destructors that are protected or private.");
michael@0 136 } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
michael@0 137 if (dtor->getAccess() == AS_public) {
michael@0 138 emitWarning(
michael@0 139 dtor->getInnerLocStart(),
michael@0 140 "Classes that are ref-counted should not have "
michael@0 141 "public destructors.");
michael@0 142 }
michael@0 143 }
michael@0 144 }
michael@0 145
michael@0 146 // Prints errors if the constructor/destructor weight is too heavy.
michael@0 147 void CheckCtorDtorWeight(SourceLocation record_location,
michael@0 148 CXXRecordDecl* record) {
michael@0 149 // We don't handle anonymous structs. If this record doesn't have a
michael@0 150 // name, it's of the form:
michael@0 151 //
michael@0 152 // struct {
michael@0 153 // ...
michael@0 154 // } name_;
michael@0 155 if (record->getIdentifier() == NULL)
michael@0 156 return;
michael@0 157
michael@0 158 // Count the number of templated base classes as a feature of whether the
michael@0 159 // destructor can be inlined.
michael@0 160 int templated_base_classes = 0;
michael@0 161 for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
michael@0 162 it != record->bases_end(); ++it) {
michael@0 163 if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
michael@0 164 TypeLoc::TemplateSpecialization) {
michael@0 165 ++templated_base_classes;
michael@0 166 }
michael@0 167 }
michael@0 168
michael@0 169 // Count the number of trivial and non-trivial member variables.
michael@0 170 int trivial_member = 0;
michael@0 171 int non_trivial_member = 0;
michael@0 172 int templated_non_trivial_member = 0;
michael@0 173 for (RecordDecl::field_iterator it = record->field_begin();
michael@0 174 it != record->field_end(); ++it) {
michael@0 175 CountType(it->getType().getTypePtr(),
michael@0 176 &trivial_member,
michael@0 177 &non_trivial_member,
michael@0 178 &templated_non_trivial_member);
michael@0 179 }
michael@0 180
michael@0 181 // Check to see if we need to ban inlined/synthesized constructors. Note
michael@0 182 // that the cutoffs here are kind of arbitrary. Scores over 10 break.
michael@0 183 int dtor_score = 0;
michael@0 184 // Deriving from a templated base class shouldn't be enough to trigger
michael@0 185 // the ctor warning, but if you do *anything* else, it should.
michael@0 186 //
michael@0 187 // TODO(erg): This is motivated by templated base classes that don't have
michael@0 188 // any data members. Somehow detect when templated base classes have data
michael@0 189 // members and treat them differently.
michael@0 190 dtor_score += templated_base_classes * 9;
michael@0 191 // Instantiating a template is an insta-hit.
michael@0 192 dtor_score += templated_non_trivial_member * 10;
michael@0 193 // The fourth normal class member should trigger the warning.
michael@0 194 dtor_score += non_trivial_member * 3;
michael@0 195
michael@0 196 int ctor_score = dtor_score;
michael@0 197 // You should be able to have 9 ints before we warn you.
michael@0 198 ctor_score += trivial_member;
michael@0 199
michael@0 200 if (ctor_score >= 10) {
michael@0 201 if (!record->hasUserDeclaredConstructor()) {
michael@0 202 emitWarning(record_location,
michael@0 203 "Complex class/struct needs an explicit out-of-line "
michael@0 204 "constructor.");
michael@0 205 } else {
michael@0 206 // Iterate across all the constructors in this file and yell if we
michael@0 207 // find one that tries to be inline.
michael@0 208 for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
michael@0 209 it != record->ctor_end(); ++it) {
michael@0 210 if (it->hasInlineBody()) {
michael@0 211 if (it->isCopyConstructor() &&
michael@0 212 !record->hasUserDeclaredCopyConstructor()) {
michael@0 213 emitWarning(record_location,
michael@0 214 "Complex class/struct needs an explicit out-of-line "
michael@0 215 "copy constructor.");
michael@0 216 } else {
michael@0 217 emitWarning(it->getInnerLocStart(),
michael@0 218 "Complex constructor has an inlined body.");
michael@0 219 }
michael@0 220 }
michael@0 221 }
michael@0 222 }
michael@0 223 }
michael@0 224
michael@0 225 // The destructor side is equivalent except that we don't check for
michael@0 226 // trivial members; 20 ints don't need a destructor.
michael@0 227 if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
michael@0 228 if (!record->hasUserDeclaredDestructor()) {
michael@0 229 emitWarning(
michael@0 230 record_location,
michael@0 231 "Complex class/struct needs an explicit out-of-line "
michael@0 232 "destructor.");
michael@0 233 } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
michael@0 234 if (dtor->hasInlineBody()) {
michael@0 235 emitWarning(dtor->getInnerLocStart(),
michael@0 236 "Complex destructor has an inline body.");
michael@0 237 }
michael@0 238 }
michael@0 239 }
michael@0 240 }
michael@0 241
michael@0 242 void CheckVirtualMethod(const CXXMethodDecl* method,
michael@0 243 bool warn_on_inline_bodies) {
michael@0 244 if (!method->isVirtual())
michael@0 245 return;
michael@0 246
michael@0 247 if (!method->isVirtualAsWritten()) {
michael@0 248 SourceLocation loc = method->getTypeSpecStartLoc();
michael@0 249 if (isa<CXXDestructorDecl>(method))
michael@0 250 loc = method->getInnerLocStart();
michael@0 251 emitWarning(loc, "Overriding method must have \"virtual\" keyword.");
michael@0 252 }
michael@0 253
michael@0 254 // Virtual methods should not have inline definitions beyond "{}". This
michael@0 255 // only matters for header files.
michael@0 256 if (warn_on_inline_bodies && method->hasBody() &&
michael@0 257 method->hasInlineBody()) {
michael@0 258 if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
michael@0 259 if (cs->size()) {
michael@0 260 emitWarning(
michael@0 261 cs->getLBracLoc(),
michael@0 262 "virtual methods with non-empty bodies shouldn't be "
michael@0 263 "declared inline.");
michael@0 264 }
michael@0 265 }
michael@0 266 }
michael@0 267 }
michael@0 268
michael@0 269 bool InTestingNamespace(const Decl* record) {
michael@0 270 return GetNamespace(record).find("testing") != std::string::npos;
michael@0 271 }
michael@0 272
michael@0 273 bool IsMethodInBannedNamespace(const CXXMethodDecl* method) {
michael@0 274 if (InBannedNamespace(method))
michael@0 275 return true;
michael@0 276 for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
michael@0 277 i != method->end_overridden_methods();
michael@0 278 ++i) {
michael@0 279 const CXXMethodDecl* overridden = *i;
michael@0 280 if (IsMethodInBannedNamespace(overridden))
michael@0 281 return true;
michael@0 282 }
michael@0 283
michael@0 284 return false;
michael@0 285 }
michael@0 286
michael@0 287 void CheckOverriddenMethod(const CXXMethodDecl* method) {
michael@0 288 if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
michael@0 289 return;
michael@0 290
michael@0 291 if (isa<CXXDestructorDecl>(method) || method->isPure())
michael@0 292 return;
michael@0 293
michael@0 294 if (IsMethodInBannedNamespace(method))
michael@0 295 return;
michael@0 296
michael@0 297 SourceLocation loc = method->getTypeSpecStartLoc();
michael@0 298 emitWarning(loc, "Overriding method must be marked with OVERRIDE.");
michael@0 299 }
michael@0 300
michael@0 301 // Makes sure there is a "virtual" keyword on virtual methods.
michael@0 302 //
michael@0 303 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
michael@0 304 // trick to get around that. If a class has member variables whose types are
michael@0 305 // in the "testing" namespace (which is how gmock works behind the scenes),
michael@0 306 // there's a really high chance we won't care about these errors
michael@0 307 void CheckVirtualMethods(SourceLocation record_location,
michael@0 308 CXXRecordDecl* record,
michael@0 309 bool warn_on_inline_bodies) {
michael@0 310 for (CXXRecordDecl::field_iterator it = record->field_begin();
michael@0 311 it != record->field_end(); ++it) {
michael@0 312 CXXRecordDecl* record_type =
michael@0 313 it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
michael@0 314 getAsCXXRecordDecl();
michael@0 315 if (record_type) {
michael@0 316 if (InTestingNamespace(record_type)) {
michael@0 317 return;
michael@0 318 }
michael@0 319 }
michael@0 320 }
michael@0 321
michael@0 322 for (CXXRecordDecl::method_iterator it = record->method_begin();
michael@0 323 it != record->method_end(); ++it) {
michael@0 324 if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
michael@0 325 // Ignore constructors and assignment operators.
michael@0 326 } else if (isa<CXXDestructorDecl>(*it) &&
michael@0 327 !record->hasUserDeclaredDestructor()) {
michael@0 328 // Ignore non-user-declared destructors.
michael@0 329 } else {
michael@0 330 CheckVirtualMethod(*it, warn_on_inline_bodies);
michael@0 331 CheckOverriddenMethod(*it);
michael@0 332 }
michael@0 333 }
michael@0 334 }
michael@0 335
michael@0 336 void CountType(const Type* type,
michael@0 337 int* trivial_member,
michael@0 338 int* non_trivial_member,
michael@0 339 int* templated_non_trivial_member) {
michael@0 340 switch (type->getTypeClass()) {
michael@0 341 case Type::Record: {
michael@0 342 // Simplifying; the whole class isn't trivial if the dtor is, but
michael@0 343 // we use this as a signal about complexity.
michael@0 344 if (TypeHasNonTrivialDtor(type))
michael@0 345 (*trivial_member)++;
michael@0 346 else
michael@0 347 (*non_trivial_member)++;
michael@0 348 break;
michael@0 349 }
michael@0 350 case Type::TemplateSpecialization: {
michael@0 351 TemplateName name =
michael@0 352 dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
michael@0 353 bool whitelisted_template = false;
michael@0 354
michael@0 355 // HACK: I'm at a loss about how to get the syntax checker to get
michael@0 356 // whether a template is exterened or not. For the first pass here,
michael@0 357 // just do retarded string comparisons.
michael@0 358 if (TemplateDecl* decl = name.getAsTemplateDecl()) {
michael@0 359 std::string base_name = decl->getNameAsString();
michael@0 360 if (base_name == "basic_string")
michael@0 361 whitelisted_template = true;
michael@0 362 }
michael@0 363
michael@0 364 if (whitelisted_template)
michael@0 365 (*non_trivial_member)++;
michael@0 366 else
michael@0 367 (*templated_non_trivial_member)++;
michael@0 368 break;
michael@0 369 }
michael@0 370 case Type::Elaborated: {
michael@0 371 CountType(
michael@0 372 dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
michael@0 373 trivial_member, non_trivial_member, templated_non_trivial_member);
michael@0 374 break;
michael@0 375 }
michael@0 376 case Type::Typedef: {
michael@0 377 while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
michael@0 378 type = TT->getDecl()->getUnderlyingType().getTypePtr();
michael@0 379 }
michael@0 380 CountType(type, trivial_member, non_trivial_member,
michael@0 381 templated_non_trivial_member);
michael@0 382 break;
michael@0 383 }
michael@0 384 default: {
michael@0 385 // Stupid assumption: anything we see that isn't the above is one of
michael@0 386 // the 20 integer types.
michael@0 387 (*trivial_member)++;
michael@0 388 break;
michael@0 389 }
michael@0 390 }
michael@0 391 }
michael@0 392 };
michael@0 393
michael@0 394 class FindBadConstructsAction : public PluginASTAction {
michael@0 395 public:
michael@0 396 FindBadConstructsAction()
michael@0 397 : check_refcounted_dtors_(true),
michael@0 398 check_virtuals_in_implementations_(true) {
michael@0 399 }
michael@0 400
michael@0 401 protected:
michael@0 402 // Overridden from PluginASTAction:
michael@0 403 virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
michael@0 404 llvm::StringRef ref) {
michael@0 405 return new FindBadConstructsConsumer(
michael@0 406 instance, check_refcounted_dtors_, check_virtuals_in_implementations_);
michael@0 407 }
michael@0 408
michael@0 409 virtual bool ParseArgs(const CompilerInstance& instance,
michael@0 410 const std::vector<std::string>& args) {
michael@0 411 bool parsed = true;
michael@0 412
michael@0 413 for (size_t i = 0; i < args.size() && parsed; ++i) {
michael@0 414 if (args[i] == "skip-refcounted-dtors") {
michael@0 415 check_refcounted_dtors_ = false;
michael@0 416 } else if (args[i] == "skip-virtuals-in-implementations") {
michael@0 417 check_virtuals_in_implementations_ = false;
michael@0 418 } else {
michael@0 419 parsed = false;
michael@0 420 llvm::errs() << "Unknown argument: " << args[i] << "\n";
michael@0 421 }
michael@0 422 }
michael@0 423
michael@0 424 return parsed;
michael@0 425 }
michael@0 426
michael@0 427 private:
michael@0 428 bool check_refcounted_dtors_;
michael@0 429 bool check_virtuals_in_implementations_;
michael@0 430 };
michael@0 431
michael@0 432 } // namespace
michael@0 433
michael@0 434 static FrontendPluginRegistry::Add<FindBadConstructsAction>
michael@0 435 X("find-bad-constructs", "Finds bad C++ constructs");

mercurial