From 87b57e88755f130159b759b8a9954c362bc463d4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 19 Apr 2016 15:57:49 -0400 Subject: Improve Plan::EdgeFinished signature Use an enumeration instead of a boolean to clarify the purpose of arguments at call sites. Suggested-by: Nico Weber --- src/build.cc | 12 ++++++------ src/build.h | 7 ++++++- src/build_test.cc | 52 ++++++++++++++++++++++++++-------------------------- 3 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/build.cc b/src/build.cc index 0971a87..3a17bdb 100644 --- a/src/build.cc +++ b/src/build.cc @@ -385,7 +385,7 @@ void Plan::ScheduleWork(Edge* edge) { } } -void Plan::EdgeFinished(Edge* edge, bool success) { +void Plan::EdgeFinished(Edge* edge, EdgeResult result) { map::iterator e = want_.find(edge); assert(e != want_.end()); bool directly_wanted = e->second; @@ -396,7 +396,7 @@ void Plan::EdgeFinished(Edge* edge, bool success) { edge->pool()->RetrieveReadyEdges(&ready_); // The rest of this function only applies to successful commands. - if (!success) + if (result != kEdgeSucceeded) return; if (directly_wanted) @@ -426,7 +426,7 @@ void Plan::NodeFinished(Node* node) { } else { // We do not need to build this edge, but we might need to build one of // its dependents. - EdgeFinished(*oe, true); + EdgeFinished(*oe, kEdgeSucceeded); } } } @@ -659,7 +659,7 @@ bool Builder::Build(string* err) { } if (edge->is_phony()) { - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); } else { ++pending_commands; } @@ -779,7 +779,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { // The rest of this function only applies to successful commands. if (!result->success()) { - plan_.EdgeFinished(edge, false); + plan_.EdgeFinished(edge, Plan::kEdgeFailed); return true; } @@ -830,7 +830,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { } } - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); // Delete any left over response file. string rspfile = edge->GetUnescapedRspfile(); diff --git a/src/build.h b/src/build.h index 3a7183f..51589ef 100644 --- a/src/build.h +++ b/src/build.h @@ -56,8 +56,13 @@ struct Plan { /// Dumps the current state of the plan. void Dump(); + enum EdgeResult { + kEdgeFailed, + kEdgeSucceeded + }; + /// Mark an edge as done building (whether it succeeded or failed). - void EdgeFinished(Edge* edge, bool success); + void EdgeFinished(Edge* edge, EdgeResult result); /// Clean the given node during the build. /// Return false on error. diff --git a/src/build_test.cc b/src/build_test.cc index 32f2690..55d093e 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -68,14 +68,14 @@ TEST_F(PlanTest, Basic) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); ASSERT_EQ("mid", edge->inputs_[0]->path()); ASSERT_EQ("out", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); edge = plan_.FindWork(); @@ -99,11 +99,11 @@ TEST_F(PlanTest, DoubleOutputDirect) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid1 mid2 - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -129,19 +129,19 @@ TEST_F(PlanTest, DoubleOutputIndirect) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a1 - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a2 - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat b1 b2 - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -167,19 +167,19 @@ TEST_F(PlanTest, DoubleDependent) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a1 a2 - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -257,7 +257,7 @@ void PlanTest::TestPoolWithDepthOne(const char* test_case) { // This will be false since poolcat is serialized ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -266,7 +266,7 @@ void PlanTest::TestPoolWithDepthOne(const char* test_case) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); edge = plan_.FindWork(); @@ -342,7 +342,7 @@ TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_EQ("outb3", edge->outputs_[0]->path()); // finish out1 - plan_.EdgeFinished(edges.front(), true); + plan_.EdgeFinished(edges.front(), Plan::kEdgeSucceeded); edges.pop_front(); // out3 should be available @@ -353,19 +353,19 @@ TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(out3, true); + plan_.EdgeFinished(out3, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.FindWork()); for (deque::iterator it = edges.begin(); it != edges.end(); ++it) { - plan_.EdgeFinished(*it, true); + plan_.EdgeFinished(*it, Plan::kEdgeSucceeded); } Edge* last = plan_.FindWork(); ASSERT_TRUE(last); ASSERT_EQ("allTheThings", last->outputs_[0]->path()); - plan_.EdgeFinished(last, true); + plan_.EdgeFinished(last, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); ASSERT_FALSE(plan_.FindWork()); @@ -407,7 +407,7 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { edge = initial_edges[1]; // Foo first ASSERT_EQ("foo.cpp", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -415,11 +415,11 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("foo.cpp", edge->inputs_[0]->path()); ASSERT_EQ("foo.cpp", edge->inputs_[1]->path()); ASSERT_EQ("foo.cpp.obj", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = initial_edges[0]; // Now for bar ASSERT_EQ("bar.cpp", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -427,7 +427,7 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("bar.cpp", edge->inputs_[0]->path()); ASSERT_EQ("bar.cpp", edge->inputs_[1]->path()); ASSERT_EQ("bar.cpp.obj", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -435,14 +435,14 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("foo.cpp.obj", edge->inputs_[0]->path()); ASSERT_EQ("bar.cpp.obj", edge->inputs_[1]->path()); ASSERT_EQ("libfoo.a", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); ASSERT_FALSE(plan_.FindWork()); ASSERT_EQ("libfoo.a", edge->inputs_[0]->path()); ASSERT_EQ("all", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge, true); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); @@ -475,7 +475,7 @@ TEST_F(PlanTest, PoolWithFailingEdge) { // This will be false since poolcat is serialized ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge, false); + plan_.EdgeFinished(edge, Plan::kEdgeFailed); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -484,7 +484,7 @@ TEST_F(PlanTest, PoolWithFailingEdge) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge, false); + plan_.EdgeFinished(edge, Plan::kEdgeFailed); ASSERT_TRUE(plan_.more_to_do()); // Jobs have failed edge = plan_.FindWork(); -- cgit v0.12