OLD | NEW |
(Empty) | |
| 1 // Copyright (c) 2011 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 // - Virtual methods with nonempty implementations in their headers. |
| 12 // |
| 13 // Things that are still TODO: |
| 14 // - Deriving from base::RefCounted and friends should mandate non-public |
| 15 // destructors. |
| 16 |
| 17 #include "clang/Frontend/FrontendPluginRegistry.h" |
| 18 #include "clang/AST/ASTConsumer.h" |
| 19 #include "clang/AST/AST.h" |
| 20 #include "clang/AST/TypeLoc.h" |
| 21 #include "clang/Basic/SourceManager.h" |
| 22 #include "clang/Frontend/CompilerInstance.h" |
| 23 #include "llvm/Support/raw_ostream.h" |
| 24 |
| 25 #include "ChromeClassTester.h" |
| 26 |
| 27 using namespace clang; |
| 28 |
| 29 namespace { |
| 30 |
| 31 bool TypeHasNonTrivialDtor(const Type* type) { |
| 32 if (const CXXRecordDecl* cxx_r = type->getCXXRecordDeclForPointerType()) |
| 33 return cxx_r->hasTrivialDestructor(); |
| 34 |
| 35 return false; |
| 36 } |
| 37 |
| 38 // Searches for constructs that we know we don't want in the Chromium code base. |
| 39 class FindBadConstructsConsumer : public ChromeClassTester { |
| 40 public: |
| 41 FindBadConstructsConsumer(CompilerInstance& instance) |
| 42 : ChromeClassTester(instance) {} |
| 43 |
| 44 virtual void CheckChromeClass(const SourceLocation& record_location, |
| 45 CXXRecordDecl* record) { |
| 46 CheckCtorDtorWeight(record_location, record); |
| 47 CheckVirtualMethods(record_location, record); |
| 48 } |
| 49 |
| 50 // Prints errors if the constructor/destructor weight it too heavy. |
| 51 void CheckCtorDtorWeight(const SourceLocation& record_location, |
| 52 CXXRecordDecl* record) { |
| 53 // We don't handle anonymous structs. If this record doesn't have a |
| 54 // name, it's of the form: |
| 55 // |
| 56 // struct { |
| 57 // ... |
| 58 // } name_; |
| 59 if (record->getIdentifier() == NULL) |
| 60 return; |
| 61 |
| 62 // Count the number of templated base classes as a feature of whether the |
| 63 // destructor can be inlined. |
| 64 int templated_base_classes = 0; |
| 65 for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin(); |
| 66 it != record->bases_end(); ++it) { |
| 67 if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() == |
| 68 TypeLoc::TemplateSpecialization) { |
| 69 ++templated_base_classes; |
| 70 } |
| 71 } |
| 72 |
| 73 // Count the number of trivial and non-trivial member variables. |
| 74 int trivial_member = 0; |
| 75 int non_trivial_member = 0; |
| 76 int templated_non_trivial_member = 0; |
| 77 for (RecordDecl::field_iterator it = record->field_begin(); |
| 78 it != record->field_end(); ++it) { |
| 79 CountType(it->getType().getTypePtr(), |
| 80 &trivial_member, |
| 81 &non_trivial_member, |
| 82 &templated_non_trivial_member); |
| 83 } |
| 84 |
| 85 // Check to see if we need to ban inlined/synthesized constructors. Note |
| 86 // that the cutoffs here are kind of arbitrary. Scores over 10 break. |
| 87 int dtor_score = 0; |
| 88 // Deriving from a templated base class shouldn't be enough to trigger |
| 89 // the ctor warning, but if you do *anything* else, it should. |
| 90 // |
| 91 // TODO(erg): This is motivated by templated base classes that don't have |
| 92 // any data members. Somehow detect when templated base classes have data |
| 93 // members and treat them differently. |
| 94 dtor_score += templated_base_classes * 9; |
| 95 // Instantiating a template is an insta-hit. |
| 96 dtor_score += templated_non_trivial_member * 10; |
| 97 // The fourth normal class member should trigger the warning. |
| 98 dtor_score += non_trivial_member * 3; |
| 99 |
| 100 int ctor_score = dtor_score; |
| 101 // You should be able to have 9 ints before we warn you. |
| 102 ctor_score += trivial_member; |
| 103 |
| 104 if (ctor_score >= 10) { |
| 105 if (!record->hasUserDeclaredConstructor()) { |
| 106 emitWarning(record_location, |
| 107 "Complex class/struct needs a declared constructor."); |
| 108 } else { |
| 109 // Iterate across all the constructors in this file and yell if we |
| 110 // find one that tries to be inline. |
| 111 for (CXXRecordDecl::ctor_iterator it = record->ctor_begin(); |
| 112 it != record->ctor_end(); ++it) { |
| 113 if (it->hasInlineBody()) { |
| 114 emitWarning(it->getInnerLocStart(), |
| 115 "Complex constructor has an inlined body."); |
| 116 } |
| 117 } |
| 118 } |
| 119 } |
| 120 |
| 121 // The destructor side is equivalent except that we don't check for |
| 122 // trivial members; 20 ints don't need a destructor. |
| 123 if (dtor_score >= 10 && !record->hasTrivialDestructor()) { |
| 124 if (!record->hasUserDeclaredDestructor()) { |
| 125 emitWarning(record_location, |
| 126 "Complex class/struct needs a declared destructor."); |
| 127 } else if (CXXDestructorDecl* dtor = record->getDestructor()) { |
| 128 if (dtor->hasInlineBody()) { |
| 129 emitWarning(dtor->getInnerLocStart(), |
| 130 "Complex destructor has an inline body."); |
| 131 } |
| 132 } |
| 133 } |
| 134 } |
| 135 |
| 136 // Makes sure there is a "virtual" keyword on virtual methods and that there |
| 137 // are no inline function bodies on them (but "{}" is allowed). |
| 138 void CheckVirtualMethods(const SourceLocation& record_location, |
| 139 CXXRecordDecl* record) { |
| 140 for (CXXRecordDecl::method_iterator it = record->method_begin(); |
| 141 it != record->method_end(); ++it) { |
| 142 if (it->isCopyAssignmentOperator() || |
| 143 dyn_cast<CXXConstructorDecl>(*it)) { |
| 144 // Ignore constructors and assignment operators. |
| 145 } else if (dyn_cast<CXXDestructorDecl>(*it)) { |
| 146 // TODO: I'd love to handle this case, but |
| 147 // CXXDestructorDecl::isImplicitlyDefined() asserts if I call it |
| 148 // here, and against my better judgment, I don't want to *always* |
| 149 // disallow implicit destructors. |
| 150 } else { |
| 151 if (it->isVirtual() && !it->isVirtualAsWritten()) { |
| 152 emitWarning(it->getTypeSpecStartLoc(), |
| 153 "Overridden method must have \"virtual\" keyword."); |
| 154 } |
| 155 |
| 156 // Virtual methods should not have inline definitions beyond "{}". |
| 157 if (it->isVirtual() && it->hasBody() && it->hasInlineBody()) { |
| 158 if (CompoundStmt* cs = dyn_cast<CompoundStmt>(it->getBody())) { |
| 159 if (cs->size()) { |
| 160 emitWarning( |
| 161 cs->getLBracLoc(), |
| 162 "virtual methods with non-empty bodies shouldn't be " |
| 163 "declared inline."); |
| 164 } |
| 165 } |
| 166 } |
| 167 } |
| 168 } |
| 169 } |
| 170 |
| 171 void CountType(const Type* type, |
| 172 int* trivial_member, |
| 173 int* non_trivial_member, |
| 174 int* templated_non_trivial_member) { |
| 175 switch (type->getTypeClass()) { |
| 176 case Type::Record: { |
| 177 // Simplifying; the whole class isn't trivial if the dtor is, but |
| 178 // we use this as a signal about complexity. |
| 179 if (TypeHasNonTrivialDtor(type)) |
| 180 (*trivial_member)++; |
| 181 else |
| 182 (*non_trivial_member)++; |
| 183 break; |
| 184 } |
| 185 case Type::TemplateSpecialization: { |
| 186 TemplateName name = |
| 187 dyn_cast<TemplateSpecializationType>(type)->getTemplateName(); |
| 188 bool whitelisted_template = false; |
| 189 |
| 190 // HACK: I'm at a loss about how to get the syntax checker to get |
| 191 // whether a template is exterened or not. For the first pass here, |
| 192 // just do retarded string comparisons. |
| 193 if (TemplateDecl* decl = name.getAsTemplateDecl()) { |
| 194 std::string base_name = decl->getNameAsString(); |
| 195 if (base_name == "basic_string") |
| 196 whitelisted_template = true; |
| 197 } |
| 198 |
| 199 if (whitelisted_template) |
| 200 (*non_trivial_member)++; |
| 201 else |
| 202 (*templated_non_trivial_member)++; |
| 203 break; |
| 204 } |
| 205 case Type::Elaborated: { |
| 206 CountType( |
| 207 dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(), |
| 208 trivial_member, non_trivial_member, templated_non_trivial_member); |
| 209 break; |
| 210 } |
| 211 case Type::Typedef: { |
| 212 while (const TypedefType *TT = dyn_cast<TypedefType>(type)) { |
| 213 type = TT->getDecl()->getUnderlyingType().getTypePtr(); |
| 214 } |
| 215 CountType(type, trivial_member, non_trivial_member, |
| 216 templated_non_trivial_member); |
| 217 break; |
| 218 } |
| 219 default: { |
| 220 // Stupid assumption: anything we see that isn't the above is one of |
| 221 // the 20 integer types. |
| 222 (*trivial_member)++; |
| 223 break; |
| 224 } |
| 225 } |
| 226 } |
| 227 }; |
| 228 |
| 229 class FindBadConstructsAction : public PluginASTAction { |
| 230 protected: |
| 231 ASTConsumer* CreateASTConsumer(CompilerInstance &CI, llvm::StringRef ref) { |
| 232 return new FindBadConstructsConsumer(CI); |
| 233 } |
| 234 |
| 235 bool ParseArgs(const CompilerInstance &CI, |
| 236 const std::vector<std::string>& args) { |
| 237 // We don't take any additional arguments here. |
| 238 return true; |
| 239 } |
| 240 }; |
| 241 |
| 242 } // namespace |
| 243 |
| 244 static FrontendPluginRegistry::Add<FindBadConstructsAction> |
| 245 X("find-bad-constructs", "Finds bad C++ constructs"); |
OLD | NEW |