From afe3beb980a4780caecc12d3fc919feb3f9cce42 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 13 Nov 2015 13:05:51 -0500 Subject: Refactor RecomputeDirty to take a node instead of an edge All call sites have a node on which they call `in_edge()` to call RecomputeDirty. Simplify call sites by taking the node directly and calling `in_edge()` internally. --- src/build.cc | 5 +++-- src/disk_interface_test.cc | 8 ++++---- src/graph.cc | 22 +++++++++++---------- src/graph.h | 3 ++- src/graph_test.cc | 48 +++++++++++++++++----------------------------- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/build.cc b/src/build.cc index 8f4169b..c2a615a 100644 --- a/src/build.cc +++ b/src/build.cc @@ -640,9 +640,10 @@ Node* Builder::AddTarget(const string& name, string* err) { } bool Builder::AddTarget(Node* node, string* err) { + if (!scan_.RecomputeDirty(node, err)) + return false; + if (Edge* in_edge = node->in_edge()) { - if (!scan_.RecomputeDirty(in_edge, err)) - return false; if (in_edge->outputs_ready()) return true; // Nothing to do. } diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 7187bdf..d7fb8f8 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -245,7 +245,7 @@ TEST_F(StatTest, Simple) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(2u, stats_.size()); ASSERT_EQ("out", stats_[0]); ASSERT_EQ("in", stats_[1]); @@ -261,7 +261,7 @@ TEST_F(StatTest, TwoStep) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(3u, stats_.size()); ASSERT_EQ("out", stats_[0]); ASSERT_TRUE(GetNode("out")->dirty()); @@ -281,7 +281,7 @@ TEST_F(StatTest, Tree) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(1u + 6u, stats_.size()); ASSERT_EQ("mid1", stats_[1]); ASSERT_TRUE(GetNode("mid1")->dirty()); @@ -302,7 +302,7 @@ TEST_F(StatTest, Middle) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_FALSE(GetNode("in")->dirty()); ASSERT_TRUE(GetNode("mid")->dirty()); ASSERT_TRUE(GetNode("out")->dirty()); diff --git a/src/graph.cc b/src/graph.cc index c90aaad..c34fab8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -31,7 +31,16 @@ bool Node::Stat(DiskInterface* disk_interface, string* err) { return (mtime_ = disk_interface->Stat(path_, err)) != -1; } -bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { +bool DependencyScan::RecomputeDirty(Node* node, string* err) { + Edge* edge = node->in_edge(); + if (!edge) { + // This node has no in-edge; it is dirty if it is missing. + if (!node->exists()) + EXPLAIN("%s has no in-edge and is missing", node->path().c_str()); + node->set_dirty(!node->exists()); + return true; + } + bool dirty = false; edge->outputs_ready_ = true; edge->deps_missing_ = false; @@ -66,15 +75,8 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { if (!(*i)->status_known()) { if (!(*i)->StatIfNecessary(disk_interface_, err)) return false; - if (Edge* in_edge = (*i)->in_edge()) { - if (!RecomputeDirty(in_edge, err)) - return false; - } else { - // This input has no in-edge; it is dirty if it is missing. - if (!(*i)->exists()) - EXPLAIN("%s has no in-edge and is missing", (*i)->path().c_str()); - (*i)->set_dirty(!(*i)->exists()); - } + if (!RecomputeDirty(*i, err)) + return false; } // If an input is not ready, neither are our outputs. diff --git a/src/graph.h b/src/graph.h index ec24293..9e82ca2 100644 --- a/src/graph.h +++ b/src/graph.h @@ -246,11 +246,12 @@ struct DependencyScan { disk_interface_(disk_interface), dep_loader_(state, deps_log, disk_interface) {} + /// Update the |dirty_| state of the given node by inspecting its input edge. /// Examine inputs, outputs, and command lines to judge whether an edge /// needs to be re-run, and update outputs_ready_ and each outputs' |dirty_| /// state accordingly. /// Returns false on failure. - bool RecomputeDirty(Edge* edge, string* err); + bool RecomputeDirty(Node* node, string* err); /// Recompute whether any output of the edge is dirty, if so sets |*dirty|. /// Returns false on failure. diff --git a/src/graph_test.cc b/src/graph_test.cc index be08b19..87f3430 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -30,9 +30,8 @@ TEST_F(GraphTest, MissingImplicit) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); // A missing implicit dep *should* make the output dirty. @@ -49,9 +48,8 @@ TEST_F(GraphTest, ModifiedImplicit) { fs_.Tick(); fs_.Create("implicit", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); // A modified implicit dep should make the output dirty. @@ -70,9 +68,8 @@ TEST_F(GraphTest, FunkyMakefilePath) { fs_.Tick(); fs_.Create("implicit.h", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); // implicit.h has changed, though our depfile refers to it with a @@ -94,9 +91,8 @@ TEST_F(GraphTest, ExplicitImplicit) { fs_.Tick(); fs_.Create("data", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); // We have both an implicit and an explicit dep on implicit.h. @@ -123,9 +119,8 @@ TEST_F(GraphTest, ImplicitOutputMissing) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out")->dirty()); @@ -140,9 +135,8 @@ TEST_F(GraphTest, ImplicitOutputOutOfDate) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out")->dirty()); @@ -165,9 +159,8 @@ TEST_F(GraphTest, ImplicitOutputOnlyMissing) { "build | out.imp: cat in\n")); fs_.Create("in", ""); - Edge* edge = GetNode("out.imp")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.imp"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.imp")->dirty()); @@ -180,9 +173,8 @@ TEST_F(GraphTest, ImplicitOutputOnlyOutOfDate) { fs_.Tick(); fs_.Create("in", ""); - Edge* edge = GetNode("out.imp")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.imp"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.imp")->dirty()); @@ -198,9 +190,8 @@ TEST_F(GraphTest, PathWithCurrentDirectory) { fs_.Create("out.o.d", "out.o: foo.cc\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); @@ -247,9 +238,8 @@ TEST_F(GraphTest, DepfileWithCanonicalizablePath) { fs_.Create("out.o.d", "out.o: bar/../foo.cc\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); @@ -268,15 +258,14 @@ TEST_F(GraphTest, DepfileRemoved) { fs_.Create("out.o.d", "out.o: foo.h\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); state_.Reset(); fs_.RemoveFile("out.o.d"); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.o")->dirty()); } @@ -323,8 +312,7 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) { "build n2: phony n1\n" ); string err; - Edge* edge = GetNode("n2")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("n2"), &err)); ASSERT_EQ("", err); Plan plan_; @@ -347,13 +335,13 @@ TEST_F(GraphTest, CycleWithLengthZeroFromDepfile) { fs_.Create("dep.d", "a: b\n"); string err; - Edge* edge = GetNode("a")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (it has outputs a and b, // but the depfile also adds b as an input), the deps should have been loaded // only once: + Edge* edge = GetNode("a")->in_edge(); EXPECT_EQ(1, edge->inputs_.size()); EXPECT_EQ("b", edge->inputs_[0]->path()); } @@ -372,13 +360,13 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfile) { fs_.Create("dep.d", "a: c\n"); string err; - Edge* edge = GetNode("a")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, // but c's in_edge has b as input but the depfile also adds |edge| as // output)), the deps should have been loaded only once: + Edge* edge = GetNode("a")->in_edge(); EXPECT_EQ(1, edge->inputs_.size()); EXPECT_EQ("c", edge->inputs_[0]->path()); } @@ -399,7 +387,7 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfileOneHopAway) { fs_.Create("dep.d", "a: c\n"); string err; - EXPECT_TRUE(scan_.RecomputeDirty(GetNode("d")->in_edge(), &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("d"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, -- cgit v0.12