Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(131)

Side by Side Diff: tools/clang/plugins/FindBadConstructs.cpp

Issue 6368055: Commit my clang plugin and fix up documentation. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add makefile for thakis Created 9 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
(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");
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698