OLD | NEW |
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 Loading... |
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 Loading... |
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 Loading... |
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 Loading... |
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"); |
OLD | NEW |