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

Side by Side Diff: tools/gn/header_checker.cc

Issue 610293003: Replace more for loops in GN (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: review Created 6 years, 2 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
« no previous file with comments | « tools/gn/gn_main.cc ('k') | tools/gn/input_conversion.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 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 "tools/gn/header_checker.h" 5 #include "tools/gn/header_checker.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/files/file_util.h" 10 #include "base/files/file_util.h"
(...skipping 103 matching lines...) Expand 10 before | Expand all | Expand 10 after
114 } 114 }
115 return ret; 115 return ret;
116 } 116 }
117 117
118 } // namespace 118 } // namespace
119 119
120 HeaderChecker::HeaderChecker(const BuildSettings* build_settings, 120 HeaderChecker::HeaderChecker(const BuildSettings* build_settings,
121 const std::vector<const Target*>& targets) 121 const std::vector<const Target*>& targets)
122 : main_loop_(base::MessageLoop::current()), 122 : main_loop_(base::MessageLoop::current()),
123 build_settings_(build_settings) { 123 build_settings_(build_settings) {
124 for (size_t i = 0; i < targets.size(); i++) 124 for (const auto& target : targets)
125 AddTargetToFileMap(targets[i], &file_map_); 125 AddTargetToFileMap(target, &file_map_);
126 } 126 }
127 127
128 HeaderChecker::~HeaderChecker() { 128 HeaderChecker::~HeaderChecker() {
129 } 129 }
130 130
131 bool HeaderChecker::Run(const std::vector<const Target*>& to_check, 131 bool HeaderChecker::Run(const std::vector<const Target*>& to_check,
132 bool force_check, 132 bool force_check,
133 std::vector<Err>* errors) { 133 std::vector<Err>* errors) {
134 if (to_check.empty()) { 134 if (to_check.empty()) {
135 // Check all files. 135 // Check all files.
136 RunCheckOverFiles(file_map_, force_check); 136 RunCheckOverFiles(file_map_, force_check);
137 } else { 137 } else {
138 // Run only over the files in the given targets. 138 // Run only over the files in the given targets.
139 FileMap files_to_check; 139 FileMap files_to_check;
140 for (size_t i = 0; i < to_check.size(); i++) 140 for (const auto& check : to_check)
141 AddTargetToFileMap(to_check[i], &files_to_check); 141 AddTargetToFileMap(check, &files_to_check);
142 RunCheckOverFiles(files_to_check, force_check); 142 RunCheckOverFiles(files_to_check, force_check);
143 } 143 }
144 144
145 if (errors_.empty()) 145 if (errors_.empty())
146 return true; 146 return true;
147 *errors = errors_; 147 *errors = errors_;
148 return false; 148 return false;
149 } 149 }
150 150
151 void HeaderChecker::RunCheckOverFiles(const FileMap& files, bool force_check) { 151 void HeaderChecker::RunCheckOverFiles(const FileMap& files, bool force_check) {
152 if (files.empty()) 152 if (files.empty())
153 return; 153 return;
154 154
155 scoped_refptr<base::SequencedWorkerPool> pool( 155 scoped_refptr<base::SequencedWorkerPool> pool(
156 new base::SequencedWorkerPool(16, "HeaderChecker")); 156 new base::SequencedWorkerPool(16, "HeaderChecker"));
157 for (FileMap::const_iterator file_i = files.begin(); 157 for (const auto& file : files) {
158 file_i != files.end(); ++file_i) {
159 const TargetVector& vect = file_i->second;
160
161 // Only check C-like source files (RC files also have includes). 158 // Only check C-like source files (RC files also have includes).
162 SourceFileType type = GetSourceFileType(file_i->first); 159 SourceFileType type = GetSourceFileType(file.first);
163 if (type != SOURCE_CC && type != SOURCE_H && type != SOURCE_C && 160 if (type != SOURCE_CC && type != SOURCE_H && type != SOURCE_C &&
164 type != SOURCE_M && type != SOURCE_MM && type != SOURCE_RC) 161 type != SOURCE_M && type != SOURCE_MM && type != SOURCE_RC)
165 continue; 162 continue;
166 163
167 // Do a first pass to find if this should be skipped. All targets including 164 // Do a first pass to find if this should be skipped. All targets including
168 // this source file must exclude it from checking, or any target 165 // this source file must exclude it from checking, or any target
169 // must mark it as generated (for cases where one target generates a file, 166 // must mark it as generated (for cases where one target generates a file,
170 // and another lists it as a source to compile it). 167 // and another lists it as a source to compile it).
171 if (!force_check) { 168 if (!force_check) {
172 bool check_includes = false; 169 bool check_includes = false;
173 bool is_generated = false; 170 bool is_generated = false;
174 for (size_t vect_i = 0; vect_i < vect.size(); ++vect_i) { 171 for (const auto& vect_i : file.second) {
175 check_includes |= vect[vect_i].target->check_includes(); 172 check_includes |= vect_i.target->check_includes();
176 is_generated |= vect[vect_i].is_generated; 173 is_generated |= vect_i.is_generated;
177 } 174 }
178 if (!check_includes || is_generated) 175 if (!check_includes || is_generated)
179 continue; 176 continue;
180 } 177 }
181 178
182 for (size_t vect_i = 0; vect_i < vect.size(); ++vect_i) { 179 for (const auto& vect_i : file.second) {
183 pool->PostWorkerTaskWithShutdownBehavior( 180 pool->PostWorkerTaskWithShutdownBehavior(
184 FROM_HERE, 181 FROM_HERE,
185 base::Bind(&HeaderChecker::DoWork, this, 182 base::Bind(&HeaderChecker::DoWork, this, vect_i.target, file.first),
186 vect[vect_i].target, file_i->first),
187 base::SequencedWorkerPool::BLOCK_SHUTDOWN); 183 base::SequencedWorkerPool::BLOCK_SHUTDOWN);
188 } 184 }
189 } 185 }
190 186
191 // After this call we're single-threaded again. 187 // After this call we're single-threaded again.
192 pool->Shutdown(); 188 pool->Shutdown();
193 } 189 }
194 190
195 void HeaderChecker::DoWork(const Target* target, const SourceFile& file) { 191 void HeaderChecker::DoWork(const Target* target, const SourceFile& file) {
196 Err err; 192 Err err;
197 if (!CheckFile(target, file, &err)) { 193 if (!CheckFile(target, file, &err)) {
198 base::AutoLock lock(lock_); 194 base::AutoLock lock(lock_);
199 errors_.push_back(err); 195 errors_.push_back(err);
200 } 196 }
201 } 197 }
202 198
203 // static 199 // static
204 void HeaderChecker::AddTargetToFileMap(const Target* target, FileMap* dest) { 200 void HeaderChecker::AddTargetToFileMap(const Target* target, FileMap* dest) {
205 // Files in the sources have this public bit by default. 201 // Files in the sources have this public bit by default.
206 bool default_public = target->all_headers_public(); 202 bool default_public = target->all_headers_public();
207 203
208 std::map<SourceFile, PublicGeneratedPair> files_to_public; 204 std::map<SourceFile, PublicGeneratedPair> files_to_public;
209 205
210 // First collect the normal files, they get the default visibility. Always 206 // First collect the normal files, they get the default visibility. Always
211 // trim the root gen dir if it exists. This will only exist on outputs of an 207 // trim the root gen dir if it exists. This will only exist on outputs of an
212 // action, but those are often then wired into the sources of a compiled 208 // action, but those are often then wired into the sources of a compiled
213 // target to actually compile generated code. If you depend on the compiled 209 // target to actually compile generated code. If you depend on the compiled
214 // target, it should be enough to be able to include the header. 210 // target, it should be enough to be able to include the header.
215 const Target::FileList& sources = target->sources(); 211 for (const auto& source : target->sources()) {
216 for (size_t i = 0; i < sources.size(); i++) { 212 SourceFile file = RemoveRootGenDirFromFile(target, source);
217 SourceFile file = RemoveRootGenDirFromFile(target, sources[i]);
218 files_to_public[file].is_public = default_public; 213 files_to_public[file].is_public = default_public;
219 } 214 }
220 215
221 // Add in the public files, forcing them to public. This may overwrite some 216 // Add in the public files, forcing them to public. This may overwrite some
222 // entries, and it may add new ones. 217 // entries, and it may add new ones.
223 const Target::FileList& public_list = target->public_headers(); 218 if (default_public) // List only used when default is not public.
224 if (default_public) 219 DCHECK(target->public_headers().empty());
225 DCHECK(public_list.empty()); // List only used when default is not public. 220 for (const auto& source : target->public_headers()) {
226 for (size_t i = 0; i < public_list.size(); i++) { 221 SourceFile file = RemoveRootGenDirFromFile(target, source);
227 SourceFile file = RemoveRootGenDirFromFile(target, public_list[i]);
228 files_to_public[file].is_public = true; 222 files_to_public[file].is_public = true;
229 } 223 }
230 224
231 // Add in outputs from actions. These are treated as public (since if other 225 // Add in outputs from actions. These are treated as public (since if other
232 // targets can't use them, then there wouldn't be any point in outputting). 226 // targets can't use them, then there wouldn't be any point in outputting).
233 std::vector<SourceFile> outputs; 227 std::vector<SourceFile> outputs;
234 target->action_values().GetOutputsAsSourceFiles(target, &outputs); 228 target->action_values().GetOutputsAsSourceFiles(target, &outputs);
235 for (size_t i = 0; i < outputs.size(); i++) { 229 for (const auto& output : outputs) {
236 // For generated files in the "gen" directory, add the filename to the 230 // For generated files in the "gen" directory, add the filename to the
237 // map assuming "gen" is the source root. This means that when files include 231 // map assuming "gen" is the source root. This means that when files include
238 // the generated header relative to there (the recommended practice), we'll 232 // the generated header relative to there (the recommended practice), we'll
239 // find the file. 233 // find the file.
240 SourceFile output_file = RemoveRootGenDirFromFile(target, outputs[i]); 234 SourceFile output_file = RemoveRootGenDirFromFile(target, output);
241 PublicGeneratedPair* pair = &files_to_public[output_file]; 235 PublicGeneratedPair* pair = &files_to_public[output_file];
242 pair->is_public = true; 236 pair->is_public = true;
243 pair->is_generated = true; 237 pair->is_generated = true;
244 } 238 }
245 239
246 // Add the merged list to the master list of all files. 240 // Add the merged list to the master list of all files.
247 for (std::map<SourceFile, PublicGeneratedPair>::const_iterator i = 241 for (const auto& cur : files_to_public) {
248 files_to_public.begin(); 242 (*dest)[cur.first].push_back(TargetInfo(
249 i != files_to_public.end(); ++i) { 243 target, cur.second.is_public, cur.second.is_generated));
250 (*dest)[i->first].push_back(TargetInfo(
251 target, i->second.is_public, i->second.is_generated));
252 } 244 }
253 } 245 }
254 246
255 bool HeaderChecker::IsFileInOuputDir(const SourceFile& file) const { 247 bool HeaderChecker::IsFileInOuputDir(const SourceFile& file) const {
256 const std::string& build_dir = build_settings_->build_dir().value(); 248 const std::string& build_dir = build_settings_->build_dir().value();
257 return file.value().compare(0, build_dir.size(), build_dir) == 0; 249 return file.value().compare(0, build_dir.size(), build_dir) == 0;
258 } 250 }
259 251
260 // This current assumes all include paths are relative to the source root 252 // This current assumes all include paths are relative to the source root
261 // which is generally the case for Chromium. 253 // which is generally the case for Chromium.
(...skipping 127 matching lines...) Expand 10 before | Expand all | Expand 10 after
389 break; 381 break;
390 } 382 }
391 } 383 }
392 384
393 if (!found_dependency) { 385 if (!found_dependency) {
394 DCHECK(!last_error.has_error()); 386 DCHECK(!last_error.has_error());
395 387
396 std::string msg = "It is not in any dependency of " + 388 std::string msg = "It is not in any dependency of " +
397 from_target->label().GetUserVisibleName(false); 389 from_target->label().GetUserVisibleName(false);
398 msg += "\nThe include file is in the target(s):\n"; 390 msg += "\nThe include file is in the target(s):\n";
399 for (size_t i = 0; i < targets.size(); i++) 391 for (const auto& target : targets)
400 msg += " " + targets[i].target->label().GetUserVisibleName(false) + "\n"; 392 msg += " " + target.target->label().GetUserVisibleName(false) + "\n";
401 if (targets.size() > 1) 393 if (targets.size() > 1)
402 msg += "at least one of "; 394 msg += "at least one of ";
403 msg += "which should somehow be reachable from " + 395 msg += "which should somehow be reachable from " +
404 from_target->label().GetUserVisibleName(false); 396 from_target->label().GetUserVisibleName(false);
405 397
406 // Danger: must call CreatePersistentRange to put in Err. 398 // Danger: must call CreatePersistentRange to put in Err.
407 *err = Err(CreatePersistentRange(source_file, range), 399 *err = Err(CreatePersistentRange(source_file, range),
408 "Include not allowed.", msg); 400 "Include not allowed.", msg);
409 return false; 401 return false;
410 } 402 }
(...skipping 84 matching lines...) Expand 10 before | Expand all | Expand 10 after
495 while (target != search_from) { 487 while (target != search_from) {
496 chain->push_back(cur_link); 488 chain->push_back(cur_link);
497 cur_link = breadcrumbs[target]; 489 cur_link = breadcrumbs[target];
498 target = cur_link.target; 490 target = cur_link.target;
499 } 491 }
500 chain->push_back(ChainLink(search_from, true)); 492 chain->push_back(ChainLink(search_from, true));
501 return true; 493 return true;
502 } 494 }
503 495
504 // Always consider public dependencies as possibilities. 496 // Always consider public dependencies as possibilities.
505 const LabelTargetVector& public_deps = target->public_deps(); 497 for (const auto& dep : target->public_deps()) {
506 for (size_t i = 0; i < public_deps.size(); i++) { 498 if (breadcrumbs.insert(std::make_pair(dep.ptr, cur_link)).second)
507 if (breadcrumbs.insert( 499 work_queue.push(ChainLink(dep.ptr, true));
508 std::make_pair(public_deps[i].ptr, cur_link)).second)
509 work_queue.push(ChainLink(public_deps[i].ptr, true));
510 } 500 }
511 501
512 if (first_time || !require_permitted) { 502 if (first_time || !require_permitted) {
513 // Consider all dependencies since all target paths are allowed, so add 503 // Consider all dependencies since all target paths are allowed, so add
514 // in private ones. Also do this the first time through the loop, since 504 // in private ones. Also do this the first time through the loop, since
515 // a target can include headers from its direct deps regardless of 505 // a target can include headers from its direct deps regardless of
516 // public/private-ness. 506 // public/private-ness.
517 first_time = false; 507 first_time = false;
518 const LabelTargetVector& private_deps = target->private_deps(); 508 for (const auto& dep : target->private_deps()) {
519 for (size_t i = 0; i < private_deps.size(); i++) { 509 if (breadcrumbs.insert(std::make_pair(dep.ptr, cur_link)).second)
520 if (breadcrumbs.insert( 510 work_queue.push(ChainLink(dep.ptr, false));
521 std::make_pair(private_deps[i].ptr, cur_link)).second)
522 work_queue.push(ChainLink(private_deps[i].ptr, false));
523 } 511 }
524 } 512 }
525 } 513 }
526 514
527 return false; 515 return false;
528 } 516 }
OLDNEW
« no previous file with comments | « tools/gn/gn_main.cc ('k') | tools/gn/input_conversion.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698