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