GN: Throw an error for duplicate object files.

Detect duplicate object files in the same target and throw an error. This provides a stronger barrier and a more actionable message than the Ninja "multiple rules" error that will result.

Review URL: https://codereview.chromium.org/1210143003

Cr-Original-Commit-Position: refs/heads/master@{#336853}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 874d7ad541925ea79c94d9a8000407c7d5bb2c91

tools/gn/ninja_binary_target_writer.cc

diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index 7b81bfa..a0a4e9f 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -8,6 +8,7 @@
 #include <set>
 #include <sstream>
 
+#include "base/containers/hash_tables.h"
 #include "base/strings/string_util.h"
 #include "tools/gn/config_values_extractors.h"
 #include "tools/gn/deps_iterator.h"
@@ -15,6 +16,7 @@
 #include "tools/gn/escape.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/ninja_utils.h"
+#include "tools/gn/scheduler.h"
 #include "tools/gn/settings.h"
 #include "tools/gn/source_file_type.h"
 #include "tools/gn/string_utils.h"
@@ -294,6 +296,9 @@
   // Also link all pch object files.
   obj_files.insert(obj_files.end(), pch_obj_files.begin(), pch_obj_files.end());
 
+  if (!CheckForDuplicateObjectFiles(obj_files))
+    return;
+
   if (target_->output_type() == Target::SOURCE_SET) {
     WriteSourceSetStamp(obj_files);
 #ifndef NDEBUG
@@ -791,3 +796,30 @@
 
   return ret;
 }
+
+bool NinjaBinaryTargetWriter::CheckForDuplicateObjectFiles(
+    const std::vector<OutputFile>& files) const {
+  base::hash_set<std::string> set;
+  for (const auto& file : files) {
+    if (!set.insert(file.value()).second) {
+      Err err(
+          target_->defined_from(),
+          "Duplicate object file",
+          "The target " + target_->label().GetUserVisibleName(false) +
+          "\ngenerates two object files with the same name:\n  " +
+          file.value() + "\n"
+          "\n"
+          "It could be you accidentally have a file listed twice in the\n"
+          "sources. Or, depending on how your toolchain maps sources to\n"
+          "object files, two source files with the same name in different\n"
+          "directories could map to the same object file.\n"
+          "\n"
+          "In the latter case, either rename one of the files or move one of\n"
+          "the sources to a separate source_set to avoid them both being in\n"
+          "the same target.");
+      g_scheduler->FailWithError(err);
+      return false;
+    }
+  }
+  return true;
+}

tools/gn/ninja_binary_target_writer.h

diff --git a/tools/gn/ninja_binary_target_writer.h b/tools/gn/ninja_binary_target_writer.h
index fe7a132..5888ac5 100644
--- a/tools/gn/ninja_binary_target_writer.h
+++ b/tools/gn/ninja_binary_target_writer.h
@@ -116,6 +116,10 @@
   // tool type. The tool must support precompiled headers.
   OutputFile GetWindowsPCHFile(Toolchain::ToolType tool_type) const;
 
+  // Checks for duplicates in the given list of output files. If any duplicates
+  // are found, throws an error and return false.
+  bool CheckForDuplicateObjectFiles(const std::vector<OutputFile>& files) const;
+
   const Tool* tool_;
 
   // Cached version of the prefix used for rule types for this toolchain.

tools/gn/ninja_binary_target_writer_unittest.cc

diff --git a/tools/gn/ninja_binary_target_writer_unittest.cc b/tools/gn/ninja_binary_target_writer_unittest.cc
index 2ad816a..5438c4d 100644
--- a/tools/gn/ninja_binary_target_writer_unittest.cc
+++ b/tools/gn/ninja_binary_target_writer_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "tools/gn/ninja_binary_target_writer.h"
+#include "tools/gn/scheduler.h"
 #include "tools/gn/target.h"
 #include "tools/gn/test_with_scope.h"
 
@@ -459,3 +460,23 @@
     EXPECT_EQ(pch_win_expected, out.str());
   }
 }
+
+// Should throw an error with the scheduler if a duplicate object file exists.
+// This is dependent on the toolchain's object file mapping.
+TEST(NinjaBinaryTargetWriter, DupeObjFileError) {
+  Scheduler scheduler;
+
+  TestWithScope setup;
+  TestTarget target(setup, "//foo:bar", Target::EXECUTABLE);
+  target.sources().push_back(SourceFile("//a.cc"));
+  target.sources().push_back(SourceFile("//a.cc"));
+
+  EXPECT_FALSE(scheduler.is_failed());
+
+  std::ostringstream out;
+  NinjaBinaryTargetWriter writer(&target, out);
+  writer.Run();
+
+  // Should have issued an error.
+  EXPECT_TRUE(scheduler.is_failed());
+}