Fix crash with duplicate implicit outputs.
authorNico Weber <nicolasweber@gmx.de>
Wed, 27 Apr 2016 01:55:20 +0000 (21:55 -0400)
committerNico Weber <nicolasweber@gmx.de>
Wed, 27 Apr 2016 13:36:42 +0000 (09:36 -0400)
Sadly, duplicate outputs aren't an error by default in Ninja (see also
a new edge has no effect.  Remember to decrement the "number of
implicit outputs" counter for the new edge when this happens.

Fixes #1136.

src/manifest_parser.cc
src/manifest_parser_test.cc

index be267a3..a4f489e 100644 (file)
@@ -339,8 +339,8 @@ bool ManifestParser::ParseEdge(string* err) {
   }
 
   edge->outputs_.reserve(outs.size());
-  for (vector<EvalString>::iterator i = outs.begin(); i != outs.end(); ++i) {
-    string path = i->Evaluate(env);
+  for (size_t i = 0, e = outs.size(); i != e; ++i) {
+    string path = outs[i].Evaluate(env);
     string path_err;
     unsigned int slash_bits;
     if (!CanonicalizePath(&path, &slash_bits, &path_err))
@@ -350,11 +350,15 @@ bool ManifestParser::ParseEdge(string* err) {
         lexer_.Error("multiple rules generate " + path + " [-w dupbuild=err]",
                      err);
         return false;
-      } else if (!quiet_) {
-        Warning("multiple rules generate %s. "
-                "builds involving this target will not be correct; "
-                "continuing anyway [-w dupbuild=warn]",
-                path.c_str());
+      } else {
+        if (!quiet_) {
+          Warning("multiple rules generate %s. "
+                  "builds involving this target will not be correct; "
+                  "continuing anyway [-w dupbuild=warn]",
+                  path.c_str());
+        }
+        if (e - i <= static_cast<size_t>(implicit_outs))
+          --implicit_outs;
       }
     }
   }
index ba83a67..1312d26 100644 (file)
@@ -949,6 +949,30 @@ TEST_F(ParserTest, ImplicitOutputEmpty) {
   EXPECT_FALSE(edge->is_implicit_out(0));
 }
 
+TEST_F(ParserTest, ImplicitOutputDupe) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(
+"rule cat\n"
+"  command = cat $in > $out\n"
+"build foo baz | foo baq foo: cat bar\n"));
+
+  Edge* edge = state.LookupNode("foo")->in_edge();
+  ASSERT_EQ(edge->outputs_.size(), 3);
+  EXPECT_FALSE(edge->is_implicit_out(0));
+  EXPECT_FALSE(edge->is_implicit_out(1));
+  EXPECT_TRUE(edge->is_implicit_out(2));
+}
+
+TEST_F(ParserTest, ImplicitOutputDupes) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(
+"rule cat\n"
+"  command = cat $in > $out\n"
+"build foo foo foo | foo foo foo foo: cat bar\n"));
+
+  Edge* edge = state.LookupNode("foo")->in_edge();
+  ASSERT_EQ(edge->outputs_.size(), 1);
+  EXPECT_FALSE(edge->is_implicit_out(0));
+}
+
 TEST_F(ParserTest, NoExplicitOutput) {
   ManifestParser parser(&state, NULL, kDupeEdgeActionWarn);
   string err;