GN header checker enhancements.

tools/gn/header_checker.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/header_checker.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
@@ skipping 97 matching lines @@
           ret.append(" -->");
         else
           ret.append(" --[private]-->");
       }
       ret.append("\n");
     }
   }
   return ret;
 }
 
+// Returns true if the two targets have the same label not counting the
+// toolchain.
+bool TargetLabelsMatchExceptToolchain(const Target* a, const Target* b) {
+  return a->label().dir() == b->label().dir() &&

[scottmg, 2015/02/20 22:41:40]

maybe GetWithNoToolchain() and == would be clearer

[brettw, 2015/02/20 22:50:03]

Slightly but that's constructing extra strings for each invocation which I was
preferring to avoid. If this comes up more often I should make this a function
on the Label class, but this is the first and likely only case we'll need to do
this.

+         a->label().name() == b->label().name();
+}
+
 }  // namespace
 
 HeaderChecker::HeaderChecker(const BuildSettings* build_settings,
                              const std::vector<const Target*>& targets)
     : main_loop_(base::MessageLoop::current()),
       build_settings_(build_settings) {
   for (const auto& target : targets)
     AddTargetToFileMap(target, &file_map_);
 }
 
@@ skipping 20 matching lines @@
 
   scoped_refptr<base::SequencedWorkerPool> pool(
       new base::SequencedWorkerPool(16, "HeaderChecker"));
   for (const auto& file : files) {
     // Only check C-like source files (RC files also have includes).
     SourceFileType type = GetSourceFileType(file.first);
     if (type != SOURCE_CC && type != SOURCE_H && type != SOURCE_C &&
         type != SOURCE_M && type != SOURCE_MM && type != SOURCE_RC)
       continue;
 
-    // Do a first pass to find if this should be skipped. All targets including
-    // this source file must exclude it from checking, or any target
-    // must mark it as generated (for cases where one target generates a file,
-    // and another lists it as a source to compile it).
-    if (!force_check) {
-      bool check_includes = false;
-      bool is_generated = false;
-      for (const auto& vect_i : file.second) {
-        check_includes |= vect_i.target->check_includes();
-        is_generated |= vect_i.is_generated;
-      }
-      if (!check_includes || is_generated)
-        continue;
-    }
+    // If any target marks it as generated, don't check it.
+    bool is_generated = false;
+    for (const auto& vect_i : file.second)
+      is_generated |= vect_i.is_generated;
+    if (is_generated)
+      continue;
 
     for (const auto& vect_i : file.second) {
-      pool->PostWorkerTaskWithShutdownBehavior(
-          FROM_HERE,
-          base::Bind(&HeaderChecker::DoWork, this, vect_i.target, file.first),
-          base::SequencedWorkerPool::BLOCK_SHUTDOWN);
+      if (vect_i.target->check_includes()) {
+        pool->PostWorkerTaskWithShutdownBehavior(
+            FROM_HERE,
+            base::Bind(&HeaderChecker::DoWork, this, vect_i.target, file.first),
+            base::SequencedWorkerPool::BLOCK_SHUTDOWN);
+      }
     }
   }
 
   // After this call we're single-threaded again.
   pool->Shutdown();
 }
 
 void HeaderChecker::DoWork(const Target* target, const SourceFile& file) {
   Err err;
   if (!CheckFile(target, file, &err)) {
@@ skipping 141 matching lines @@
   Err last_error;
 
   bool found_dependency = false;
   for (size_t i = 0; i < targets.size(); i++) {
     // We always allow source files in a target to include headers also in that
     // target.
     const Target* to_target = targets[i].target;
     if (to_target == from_target)
       return true;
 
+    // Additionally, allow a target to include files from that same target
+    // in other toolchains. This is a bit of a hack to account for the fact that
+    // the include finder doesn't understand the preprocessor.
+    //
+    // If a source file conditionally depends on a platform-specific include in
+    // the same target, and there is a cross-compile such that GN sees
+    // definitions of the target both with and without that include, it would
+    // give an error that the target needs to depend on itself in the other
+    // toolchain (where the platform-specific header is defined as a source).
+    if (TargetLabelsMatchExceptToolchain(to_target, from_target))
+      return true;
+
     bool is_permitted_chain = false;
     if (IsDependencyOf(to_target, from_target, &chain, &is_permitted_chain)) {
       DCHECK(chain.size() >= 2);
       DCHECK(chain[0].target == to_target);
       DCHECK(chain[chain.size() - 1].target == from_target);
 
       found_dependency = true;
 
       if (targets[i].is_public && is_permitted_chain) {
         // This one is OK, we're done.
@@ skipping 22 matching lines @@
         to_target->allow_circular_includes_from().end()) {
       // Not a dependency, but this include is whitelisted from the destination.
       found_dependency = true;
       last_error = Err();
       break;
     }
   }
 
   if (!found_dependency) {
     DCHECK(!last_error.has_error());
-
-    std::string msg = "It is not in any dependency of " +
-        from_target->label().GetUserVisibleName(false);
-    msg += "\nThe include file is in the target(s):\n";
-    for (const auto& target : targets)
-      msg += "  " + target.target->label().GetUserVisibleName(false) + "\n";
-    if (targets.size() > 1)
-      msg += "at least one of ";
-    msg += "which should somehow be reachable from " +
-        from_target->label().GetUserVisibleName(false);
-
-    // Danger: must call CreatePersistentRange to put in Err.
-    *err = Err(CreatePersistentRange(source_file, range),
-               "Include not allowed.", msg);
+    *err = MakeUnreachableError(source_file, range, from_target, targets);
     return false;
   }
   if (last_error.has_error()) {
     // Found at least one dependency chain above, but it had an error.
     *err = last_error;
     return false;
   }
 
   // One thing we didn't check for is targets that expose their dependents
   // headers in their own public headers.
@@ skipping 96 matching lines @@
       first_time = false;
       for (const auto& dep : target->private_deps()) {
         if (breadcrumbs.insert(std::make_pair(dep.ptr, cur_link)).second)
           work_queue.push(ChainLink(dep.ptr, false));
       }
     }
   }
 
   return false;
 }
+
+Err HeaderChecker::MakeUnreachableError(
+    const InputFile& source_file,
+    const LocationRange& range,
+    const Target* from_target,
+    const TargetVector& targets) {
+  // Normally the toolchains will all match, but when cross-compiling, we can
+  // get targets with more than one toolchain in the list of possibilities.
+  std::vector<const Target*> targets_with_matching_toolchains;
+  std::vector<const Target*> targets_with_other_toolchains;
+  for (const TargetInfo& candidate : targets) {
+    if (candidate.target->toolchain() == from_target->toolchain())
+      targets_with_matching_toolchains.push_back(candidate.target);
+    else
+      targets_with_other_toolchains.push_back(candidate.target);
+  }
+
+  // It's common when cross-compiling to have a target with the same file in
+  // more than one toolchain. We could output all of them, but this is
+  // generally confusing to people (most end-users won't understand toolchains
+  // well).
+  //
+  // So delete any candidates in other toolchains that also appear in the same
+  // toolchain as the from_target.
+  for (int other_index = 0;
+       other_index < static_cast<int>(targets_with_other_toolchains.size());
+       other_index++) {
+    for (const Target* cur_matching : targets_with_matching_toolchains) {
+      if (TargetLabelsMatchExceptToolchain(
+              cur_matching, targets_with_other_toolchains[other_index])) {
+        // Found a duplicate, erase it.
+        targets_with_other_toolchains.erase(
+            targets_with_other_toolchains.begin() + other_index);
+        other_index--;
+        break;
+      }
+    }
+  }
+
+  // Only display toolchains on labels if they don't all match.
+  bool include_toolchain = !targets_with_other_toolchains.empty();
+
+  std::string msg = "It is not in any dependency of\n  " +
+      from_target->label().GetUserVisibleName(include_toolchain);
+  msg += "\nThe include file is in the target(s):\n";
+  for (const auto& target : targets_with_matching_toolchains)
+    msg += "  " + target->label().GetUserVisibleName(include_toolchain) + "\n";
+  for (const auto& target : targets_with_other_toolchains)
+    msg += "  " + target->label().GetUserVisibleName(include_toolchain) + "\n";
+  if (targets_with_other_toolchains.size() +
+      targets_with_matching_toolchains.size() > 1)
+    msg += "at least one of ";
+  msg += "which should somehow be reachable.";
+
+  // Danger: must call CreatePersistentRange to put in Err.
+  return Err(CreatePersistentRange(source_file, range),
+             "Include not allowed.", msg);
+}
+