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

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

Issue 212673008: Revert of Add a check to the FindBadConstructs.cpp clang plugin for bad enum last values. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 8 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
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.cpp ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 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 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 // This file defines a bunch of recurring problems in the Chromium C++ code. 5 // This file defines a bunch of recurring problems in the Chromium C++ code.
6 // 6 //
7 // Checks that are implemented: 7 // Checks that are implemented:
8 // - Constructors/Destructors should not be inlined if they are of a complex 8 // - Constructors/Destructors should not be inlined if they are of a complex
9 // class type. 9 // class type.
10 // - Missing "virtual" keywords on methods that should be virtual. 10 // - Missing "virtual" keywords on methods that should be virtual.
11 // - Non-annotated overriding virtual methods. 11 // - Non-annotated overriding virtual methods.
12 // - Virtual methods with nonempty implementations in their headers. 12 // - Virtual methods with nonempty implementations in their headers.
13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe 13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
14 // should have protected or private destructors. 14 // should have protected or private destructors.
15 // - WeakPtrFactory members that refer to their outer class should be the last 15 // - WeakPtrFactory members that refer to their outer class should be the last
16 // member. 16 // member.
17 // - Enum types with a xxxx_LAST or xxxxLast const actually have that constant
18 // have the maximal value for that type.
19 17
20 #include "clang/AST/ASTConsumer.h" 18 #include "clang/AST/ASTConsumer.h"
21 #include "clang/AST/AST.h" 19 #include "clang/AST/AST.h"
22 #include "clang/AST/Attr.h" 20 #include "clang/AST/Attr.h"
23 #include "clang/AST/CXXInheritance.h" 21 #include "clang/AST/CXXInheritance.h"
24 #include "clang/AST/TypeLoc.h" 22 #include "clang/AST/TypeLoc.h"
25 #include "clang/Basic/SourceManager.h" 23 #include "clang/Basic/SourceManager.h"
26 #include "clang/Frontend/CompilerInstance.h" 24 #include "clang/Frontend/CompilerInstance.h"
27 #include "clang/Frontend/FrontendPluginRegistry.h" 25 #include "clang/Frontend/FrontendPluginRegistry.h"
28 #include "clang/Lex/Lexer.h" 26 #include "clang/Lex/Lexer.h"
(...skipping 14 matching lines...) Expand all
43 "destructors that are declared protected or private."; 41 "destructors that are declared protected or private.";
44 const char kPublicDtor[] = 42 const char kPublicDtor[] =
45 "[chromium-style] Classes that are ref-counted should have " 43 "[chromium-style] Classes that are ref-counted should have "
46 "destructors that are declared protected or private."; 44 "destructors that are declared protected or private.";
47 const char kProtectedNonVirtualDtor[] = 45 const char kProtectedNonVirtualDtor[] =
48 "[chromium-style] Classes that are ref-counted and have non-private " 46 "[chromium-style] Classes that are ref-counted and have non-private "
49 "destructors should declare their destructor virtual."; 47 "destructors should declare their destructor virtual.";
50 const char kWeakPtrFactoryOrder[] = 48 const char kWeakPtrFactoryOrder[] =
51 "[chromium-style] WeakPtrFactory members which refer to their outer class " 49 "[chromium-style] WeakPtrFactory members which refer to their outer class "
52 "must be the last member in the outer class definition."; 50 "must be the last member in the outer class definition.";
53 const char kBadLastEnumValue[] =
54 "[chromium-style] _LAST/Last constants of enum types must have the maximal "
55 "value for any constant of that type.";
56 const char kNoteInheritance[] = 51 const char kNoteInheritance[] =
57 "[chromium-style] %0 inherits from %1 here"; 52 "[chromium-style] %0 inherits from %1 here";
58 const char kNoteImplicitDtor[] = 53 const char kNoteImplicitDtor[] =
59 "[chromium-style] No explicit destructor for %0 defined"; 54 "[chromium-style] No explicit destructor for %0 defined";
60 const char kNotePublicDtor[] = 55 const char kNotePublicDtor[] =
61 "[chromium-style] Public destructor declared here"; 56 "[chromium-style] Public destructor declared here";
62 const char kNoteProtectedNonVirtualDtor[] = 57 const char kNoteProtectedNonVirtualDtor[] =
63 "[chromium-style] Protected non-virtual destructor declared here"; 58 "[chromium-style] Protected non-virtual destructor declared here";
64 59
65 bool TypeHasNonTrivialDtor(const Type* type) { 60 bool TypeHasNonTrivialDtor(const Type* type) {
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
105 diag_method_requires_virtual_ = diagnostic().getCustomDiagID( 100 diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
106 getErrorLevel(), kMethodRequiresVirtual); 101 getErrorLevel(), kMethodRequiresVirtual);
107 diag_no_explicit_dtor_ = diagnostic().getCustomDiagID( 102 diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
108 getErrorLevel(), kNoExplicitDtor); 103 getErrorLevel(), kNoExplicitDtor);
109 diag_public_dtor_ = diagnostic().getCustomDiagID( 104 diag_public_dtor_ = diagnostic().getCustomDiagID(
110 getErrorLevel(), kPublicDtor); 105 getErrorLevel(), kPublicDtor);
111 diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID( 106 diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
112 getErrorLevel(), kProtectedNonVirtualDtor); 107 getErrorLevel(), kProtectedNonVirtualDtor);
113 diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID( 108 diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
114 getErrorLevel(), kWeakPtrFactoryOrder); 109 getErrorLevel(), kWeakPtrFactoryOrder);
115 diag_bad_enum_last_value_ = diagnostic().getCustomDiagID(
116 getErrorLevel(), kBadLastEnumValue);
117 110
118 // Registers notes to make it easier to interpret warnings. 111 // Registers notes to make it easier to interpret warnings.
119 diag_note_inheritance_ = diagnostic().getCustomDiagID( 112 diag_note_inheritance_ = diagnostic().getCustomDiagID(
120 DiagnosticsEngine::Note, kNoteInheritance); 113 DiagnosticsEngine::Note, kNoteInheritance);
121 diag_note_implicit_dtor_ = diagnostic().getCustomDiagID( 114 diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
122 DiagnosticsEngine::Note, kNoteImplicitDtor); 115 DiagnosticsEngine::Note, kNoteImplicitDtor);
123 diag_note_public_dtor_ = diagnostic().getCustomDiagID( 116 diag_note_public_dtor_ = diagnostic().getCustomDiagID(
124 DiagnosticsEngine::Note, kNotePublicDtor); 117 DiagnosticsEngine::Note, kNotePublicDtor);
125 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID( 118 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
126 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor); 119 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
(...skipping 16 matching lines...) Expand all
143 // virtual and OVERRIDE. 136 // virtual and OVERRIDE.
144 CheckVirtualMethods(record_location, record, warn_on_inline_bodies); 137 CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
145 } 138 }
146 139
147 CheckRefCountedDtors(record_location, record); 140 CheckRefCountedDtors(record_location, record);
148 141
149 if (options_.check_weak_ptr_factory_order) 142 if (options_.check_weak_ptr_factory_order)
150 CheckWeakPtrFactoryMembers(record_location, record); 143 CheckWeakPtrFactoryMembers(record_location, record);
151 } 144 }
152 145
153 virtual void CheckChromeEnum(SourceLocation enum_location,
154 EnumDecl* enum_decl) {
155 bool got_one = false;
156 llvm::APSInt max_so_far;
157 EnumDecl::enumerator_iterator iter;
158 for (iter = enum_decl->enumerator_begin();
159 iter != enum_decl->enumerator_end(); ++iter) {
160 if (!got_one) {
161 max_so_far = iter->getInitVal();
162 got_one = true;
163 } else if (iter->getInitVal() > max_so_far)
164 max_so_far = iter->getInitVal();
165 }
166 for (iter = enum_decl->enumerator_begin();
167 iter != enum_decl->enumerator_end(); ++iter) {
168 std::string name = iter->getNameAsString();
169 if (((name.size() > 4 &&
170 name.compare(name.size() - 4, 4, "Last") == 0) ||
171 (name.size() > 5 &&
172 name.compare(name.size() - 5, 5, "_LAST") == 0)) &&
173 iter->getInitVal() < max_so_far) {
174 diagnostic().Report(iter->getLocation(), diag_bad_enum_last_value_);
175 }
176 }
177 }
178
179 private: 146 private:
180 // The type of problematic ref-counting pattern that was encountered. 147 // The type of problematic ref-counting pattern that was encountered.
181 enum RefcountIssue { 148 enum RefcountIssue {
182 None, 149 None,
183 ImplicitDestructor, 150 ImplicitDestructor,
184 PublicDestructor 151 PublicDestructor
185 }; 152 };
186 153
187 FindBadConstructsOptions options_; 154 FindBadConstructsOptions options_;
188 155
189 unsigned diag_method_requires_override_; 156 unsigned diag_method_requires_override_;
190 unsigned diag_method_requires_virtual_; 157 unsigned diag_method_requires_virtual_;
191 unsigned diag_no_explicit_dtor_; 158 unsigned diag_no_explicit_dtor_;
192 unsigned diag_public_dtor_; 159 unsigned diag_public_dtor_;
193 unsigned diag_protected_non_virtual_dtor_; 160 unsigned diag_protected_non_virtual_dtor_;
194 unsigned diag_weak_ptr_factory_order_; 161 unsigned diag_weak_ptr_factory_order_;
195 unsigned diag_bad_enum_last_value_;
196 unsigned diag_note_inheritance_; 162 unsigned diag_note_inheritance_;
197 unsigned diag_note_implicit_dtor_; 163 unsigned diag_note_implicit_dtor_;
198 unsigned diag_note_public_dtor_; 164 unsigned diag_note_public_dtor_;
199 unsigned diag_note_protected_non_virtual_dtor_; 165 unsigned diag_note_protected_non_virtual_dtor_;
200 166
201 // Prints errors if the constructor/destructor weight is too heavy. 167 // Prints errors if the constructor/destructor weight is too heavy.
202 void CheckCtorDtorWeight(SourceLocation record_location, 168 void CheckCtorDtorWeight(SourceLocation record_location,
203 CXXRecordDecl* record) { 169 CXXRecordDecl* record) {
204 // We don't handle anonymous structs. If this record doesn't have a 170 // We don't handle anonymous structs. If this record doesn't have a
205 // name, it's of the form: 171 // name, it's of the form:
(...skipping 550 matching lines...) Expand 10 before | Expand all | Expand 10 after
756 } 722 }
757 723
758 private: 724 private:
759 FindBadConstructsOptions options_; 725 FindBadConstructsOptions options_;
760 }; 726 };
761 727
762 } // namespace 728 } // namespace
763 729
764 static FrontendPluginRegistry::Add<FindBadConstructsAction> 730 static FrontendPluginRegistry::Add<FindBadConstructsAction>
765 X("find-bad-constructs", "Finds bad C++ constructs"); 731 X("find-bad-constructs", "Finds bad C++ constructs");
OLDNEW
« no previous file with comments | « tools/clang/plugins/ChromeClassTester.cpp ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698