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

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

Issue 150943005: 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: 500s Created 6 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
« 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.
17 19
18 #include "clang/AST/ASTConsumer.h" 20 #include "clang/AST/ASTConsumer.h"
19 #include "clang/AST/AST.h" 21 #include "clang/AST/AST.h"
20 #include "clang/AST/Attr.h" 22 #include "clang/AST/Attr.h"
21 #include "clang/AST/CXXInheritance.h" 23 #include "clang/AST/CXXInheritance.h"
22 #include "clang/AST/TypeLoc.h" 24 #include "clang/AST/TypeLoc.h"
23 #include "clang/Basic/SourceManager.h" 25 #include "clang/Basic/SourceManager.h"
24 #include "clang/Frontend/CompilerInstance.h" 26 #include "clang/Frontend/CompilerInstance.h"
25 #include "clang/Frontend/FrontendPluginRegistry.h" 27 #include "clang/Frontend/FrontendPluginRegistry.h"
26 #include "clang/Lex/Lexer.h" 28 #include "clang/Lex/Lexer.h"
(...skipping 14 matching lines...) Expand all
41 "destructors that are declared protected or private."; 43 "destructors that are declared protected or private.";
42 const char kPublicDtor[] = 44 const char kPublicDtor[] =
43 "[chromium-style] Classes that are ref-counted should have " 45 "[chromium-style] Classes that are ref-counted should have "
44 "destructors that are declared protected or private."; 46 "destructors that are declared protected or private.";
45 const char kProtectedNonVirtualDtor[] = 47 const char kProtectedNonVirtualDtor[] =
46 "[chromium-style] Classes that are ref-counted and have non-private " 48 "[chromium-style] Classes that are ref-counted and have non-private "
47 "destructors should declare their destructor virtual."; 49 "destructors should declare their destructor virtual.";
48 const char kWeakPtrFactoryOrder[] = 50 const char kWeakPtrFactoryOrder[] =
49 "[chromium-style] WeakPtrFactory members which refer to their outer class " 51 "[chromium-style] WeakPtrFactory members which refer to their outer class "
50 "must be the last member in the outer class definition."; 52 "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.";
51 const char kNoteInheritance[] = 56 const char kNoteInheritance[] =
52 "[chromium-style] %0 inherits from %1 here"; 57 "[chromium-style] %0 inherits from %1 here";
53 const char kNoteImplicitDtor[] = 58 const char kNoteImplicitDtor[] =
54 "[chromium-style] No explicit destructor for %0 defined"; 59 "[chromium-style] No explicit destructor for %0 defined";
55 const char kNotePublicDtor[] = 60 const char kNotePublicDtor[] =
56 "[chromium-style] Public destructor declared here"; 61 "[chromium-style] Public destructor declared here";
57 const char kNoteProtectedNonVirtualDtor[] = 62 const char kNoteProtectedNonVirtualDtor[] =
58 "[chromium-style] Protected non-virtual destructor declared here"; 63 "[chromium-style] Protected non-virtual destructor declared here";
59 64
60 bool TypeHasNonTrivialDtor(const Type* type) { 65 bool TypeHasNonTrivialDtor(const Type* type) {
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
100 diag_method_requires_virtual_ = diagnostic().getCustomDiagID( 105 diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
101 getErrorLevel(), kMethodRequiresVirtual); 106 getErrorLevel(), kMethodRequiresVirtual);
102 diag_no_explicit_dtor_ = diagnostic().getCustomDiagID( 107 diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
103 getErrorLevel(), kNoExplicitDtor); 108 getErrorLevel(), kNoExplicitDtor);
104 diag_public_dtor_ = diagnostic().getCustomDiagID( 109 diag_public_dtor_ = diagnostic().getCustomDiagID(
105 getErrorLevel(), kPublicDtor); 110 getErrorLevel(), kPublicDtor);
106 diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID( 111 diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
107 getErrorLevel(), kProtectedNonVirtualDtor); 112 getErrorLevel(), kProtectedNonVirtualDtor);
108 diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID( 113 diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
109 getErrorLevel(), kWeakPtrFactoryOrder); 114 getErrorLevel(), kWeakPtrFactoryOrder);
115 diag_bad_enum_last_value_ = diagnostic().getCustomDiagID(
116 getErrorLevel(), kBadLastEnumValue);
110 117
111 // Registers notes to make it easier to interpret warnings. 118 // Registers notes to make it easier to interpret warnings.
112 diag_note_inheritance_ = diagnostic().getCustomDiagID( 119 diag_note_inheritance_ = diagnostic().getCustomDiagID(
113 DiagnosticsEngine::Note, kNoteInheritance); 120 DiagnosticsEngine::Note, kNoteInheritance);
114 diag_note_implicit_dtor_ = diagnostic().getCustomDiagID( 121 diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
115 DiagnosticsEngine::Note, kNoteImplicitDtor); 122 DiagnosticsEngine::Note, kNoteImplicitDtor);
116 diag_note_public_dtor_ = diagnostic().getCustomDiagID( 123 diag_note_public_dtor_ = diagnostic().getCustomDiagID(
117 DiagnosticsEngine::Note, kNotePublicDtor); 124 DiagnosticsEngine::Note, kNotePublicDtor);
118 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID( 125 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
119 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor); 126 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
(...skipping 16 matching lines...) Expand all
136 // virtual and OVERRIDE. 143 // virtual and OVERRIDE.
137 CheckVirtualMethods(record_location, record, warn_on_inline_bodies); 144 CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
138 } 145 }
139 146
140 CheckRefCountedDtors(record_location, record); 147 CheckRefCountedDtors(record_location, record);
141 148
142 if (options_.check_weak_ptr_factory_order) 149 if (options_.check_weak_ptr_factory_order)
143 CheckWeakPtrFactoryMembers(record_location, record); 150 CheckWeakPtrFactoryMembers(record_location, record);
144 } 151 }
145 152
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
146 private: 179 private:
147 // The type of problematic ref-counting pattern that was encountered. 180 // The type of problematic ref-counting pattern that was encountered.
148 enum RefcountIssue { 181 enum RefcountIssue {
149 None, 182 None,
150 ImplicitDestructor, 183 ImplicitDestructor,
151 PublicDestructor 184 PublicDestructor
152 }; 185 };
153 186
154 FindBadConstructsOptions options_; 187 FindBadConstructsOptions options_;
155 188
156 unsigned diag_method_requires_override_; 189 unsigned diag_method_requires_override_;
157 unsigned diag_method_requires_virtual_; 190 unsigned diag_method_requires_virtual_;
158 unsigned diag_no_explicit_dtor_; 191 unsigned diag_no_explicit_dtor_;
159 unsigned diag_public_dtor_; 192 unsigned diag_public_dtor_;
160 unsigned diag_protected_non_virtual_dtor_; 193 unsigned diag_protected_non_virtual_dtor_;
161 unsigned diag_weak_ptr_factory_order_; 194 unsigned diag_weak_ptr_factory_order_;
195 unsigned diag_bad_enum_last_value_;
162 unsigned diag_note_inheritance_; 196 unsigned diag_note_inheritance_;
163 unsigned diag_note_implicit_dtor_; 197 unsigned diag_note_implicit_dtor_;
164 unsigned diag_note_public_dtor_; 198 unsigned diag_note_public_dtor_;
165 unsigned diag_note_protected_non_virtual_dtor_; 199 unsigned diag_note_protected_non_virtual_dtor_;
166 200
167 // Prints errors if the constructor/destructor weight is too heavy. 201 // Prints errors if the constructor/destructor weight is too heavy.
168 void CheckCtorDtorWeight(SourceLocation record_location, 202 void CheckCtorDtorWeight(SourceLocation record_location,
169 CXXRecordDecl* record) { 203 CXXRecordDecl* record) {
170 // We don't handle anonymous structs. If this record doesn't have a 204 // We don't handle anonymous structs. If this record doesn't have a
171 // name, it's of the form: 205 // name, it's of the form:
(...skipping 550 matching lines...) Expand 10 before | Expand all | Expand 10 after
722 } 756 }
723 757
724 private: 758 private:
725 FindBadConstructsOptions options_; 759 FindBadConstructsOptions options_;
726 }; 760 };
727 761
728 } // namespace 762 } // namespace
729 763
730 static FrontendPluginRegistry::Add<FindBadConstructsAction> 764 static FrontendPluginRegistry::Add<FindBadConstructsAction>
731 X("find-bad-constructs", "Finds bad C++ constructs"); 765 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