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

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

Issue 862133002: Update from https://crrev.com/312398 (Closed) Base URL: git@github.com:domokit/mojo.git@master
Patch Set: Created 5 years, 11 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
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 #include "FindBadConstructsConsumer.h" 5 #include "FindBadConstructsConsumer.h"
6 6
7 #include "clang/Frontend/CompilerInstance.h" 7 #include "clang/Frontend/CompilerInstance.h"
8 #include "clang/AST/Attr.h" 8 #include "clang/AST/Attr.h"
9 #include "clang/Lex/Lexer.h" 9 #include "clang/Lex/Lexer.h"
10 #include "llvm/Support/raw_ostream.h" 10 #include "llvm/Support/raw_ostream.h"
(...skipping 71 matching lines...) Expand 10 before | Expand all | Expand 10 after
82 // Get the spelling loc just in case it was expanded from a macro. 82 // Get the spelling loc just in case it was expanded from a macro.
83 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin())); 83 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
84 // Sanity check that the text looks like virtual. 84 // Sanity check that the text looks like virtual.
85 StringRef text = clang::Lexer::getSourceText( 85 StringRef text = clang::Lexer::getSourceText(
86 CharSourceRange::getTokenRange(spelling_range), manager, LangOptions()); 86 CharSourceRange::getTokenRange(spelling_range), manager, LangOptions());
87 if (text.trim() != "virtual") 87 if (text.trim() != "virtual")
88 return FixItHint(); 88 return FixItHint();
89 return FixItHint::CreateRemoval(range); 89 return FixItHint::CreateRemoval(range);
90 } 90 }
91 91
92 bool IsPodOrTemplateType(const CXXRecordDecl& record) {
93 return record.isPOD() ||
94 record.getDescribedClassTemplate() ||
95 record.getTemplateSpecializationKind() ||
96 record.isDependentType();
97 }
98
92 } // namespace 99 } // namespace
93 100
94 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance, 101 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
95 const Options& options) 102 const Options& options)
96 : ChromeClassTester(instance), options_(options) { 103 : ChromeClassTester(instance), options_(options) {
97 // Messages for virtual method specifiers. 104 // Messages for virtual method specifiers.
98 diag_method_requires_override_ = 105 diag_method_requires_override_ =
99 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride); 106 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride);
100 diag_redundant_virtual_specifier_ = 107 diag_redundant_virtual_specifier_ =
101 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier); 108 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier);
(...skipping 27 matching lines...) Expand all
129 136
130 bool FindBadConstructsConsumer::VisitDecl(clang::Decl* decl) { 137 bool FindBadConstructsConsumer::VisitDecl(clang::Decl* decl) {
131 clang::TagDecl* tag_decl = dyn_cast<clang::TagDecl>(decl); 138 clang::TagDecl* tag_decl = dyn_cast<clang::TagDecl>(decl);
132 if (tag_decl && tag_decl->isCompleteDefinition()) 139 if (tag_decl && tag_decl->isCompleteDefinition())
133 CheckTag(tag_decl); 140 CheckTag(tag_decl);
134 return true; 141 return true;
135 } 142 }
136 143
137 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location, 144 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
138 CXXRecordDecl* record) { 145 CXXRecordDecl* record) {
146 // By default, the clang checker doesn't check some types (templates, etc).
147 // That was only a mistake; once Chromium code passes these checks, we should
148 // remove the "check-templates" option and remove this code.
149 // See crbug.com/441916
150 if (!options_.check_templates && IsPodOrTemplateType(*record))
151 return;
152
139 bool implementation_file = InImplementationFile(record_location); 153 bool implementation_file = InImplementationFile(record_location);
140 154
141 if (!implementation_file) { 155 if (!implementation_file) {
142 // Only check for "heavy" constructors/destructors in header files; 156 // Only check for "heavy" constructors/destructors in header files;
143 // within implementation files, there is no performance cost. 157 // within implementation files, there is no performance cost.
144 CheckCtorDtorWeight(record_location, record); 158
159 // If this is a POD or a class template or a type dependent on a
160 // templated class, assume there's no ctor/dtor/virtual method
161 // optimization that we should do.
162 if (!IsPodOrTemplateType(*record))
163 CheckCtorDtorWeight(record_location, record);
145 } 164 }
146 165
147 bool warn_on_inline_bodies = !implementation_file; 166 bool warn_on_inline_bodies = !implementation_file;
148
149 // Check that all virtual methods are annotated with override or final. 167 // Check that all virtual methods are annotated with override or final.
150 CheckVirtualMethods(record_location, record, warn_on_inline_bodies); 168 // Note this could also apply to templates, but for some reason Clang
169 // does not always see the "override", so we get false positives.
170 // See http://llvm.org/bugs/show_bug.cgi?id=18440 and
171 // http://llvm.org/bugs/show_bug.cgi?id=21942
172 if (!IsPodOrTemplateType(*record))
173 CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
151 174
152 CheckRefCountedDtors(record_location, record); 175 CheckRefCountedDtors(record_location, record);
153 176
154 if (options_.check_weak_ptr_factory_order) 177 CheckWeakPtrFactoryMembers(record_location, record);
155 CheckWeakPtrFactoryMembers(record_location, record);
156 } 178 }
157 179
158 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location, 180 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location,
159 EnumDecl* enum_decl) { 181 EnumDecl* enum_decl) {
160 if (!options_.check_enum_last_value) 182 if (!options_.check_enum_last_value)
161 return; 183 return;
162 184
163 bool got_one = false; 185 bool got_one = false;
164 bool is_signed = false; 186 bool is_signed = false;
165 llvm::APSInt max_so_far; 187 llvm::APSInt max_so_far;
(...skipping 593 matching lines...) Expand 10 before | Expand all | Expand 10 after
759 // one of those, it means there is at least one member after a factory. 781 // one of those, it means there is at least one member after a factory.
760 if (weak_ptr_factory_location.isValid() && 782 if (weak_ptr_factory_location.isValid() &&
761 !param_is_weak_ptr_factory_to_self) { 783 !param_is_weak_ptr_factory_to_self) {
762 diagnostic().Report(weak_ptr_factory_location, 784 diagnostic().Report(weak_ptr_factory_location,
763 diag_weak_ptr_factory_order_); 785 diag_weak_ptr_factory_order_);
764 } 786 }
765 } 787 }
766 } 788 }
767 789
768 } // namespace chrome_checker 790 } // namespace chrome_checker
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698