From d27d21a8c38debfd69c1deaa302cdb500511e0af Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Wed, 7 Dec 2011 09:27:01 -0800 Subject: [PATCH] make Node::dirty_ private --- src/build.cc | 8 ++++---- src/build_test.cc | 40 ++++++++++++++++++++-------------------- src/disk_interface_test.cc | 12 ++++++------ src/graph.cc | 6 +++--- src/graph.h | 16 +++++++++++----- src/graph_test.cc | 10 +++++----- 6 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/build.cc b/src/build.cc index 6da821f..e109535 100644 --- a/src/build.cc +++ b/src/build.cc @@ -192,7 +192,7 @@ bool Plan::AddTarget(Node* node, string* err) { bool Plan::AddSubTarget(Node* node, vector* stack, string* err) { Edge* edge = node->in_edge_; if (!edge) { // Leaf node. - if (node->dirty_) { + if (node->dirty()) { string referenced; if (!stack->empty()) referenced = ", needed by '" + stack->back()->path() + "',"; @@ -307,7 +307,7 @@ void Plan::NodeFinished(Node* node) { } void Plan::CleanNode(BuildLog* build_log, Node* node) { - node->dirty_ = false; + node->set_dirty(false); for (vector::iterator ei = node->out_edges_.begin(); ei != node->out_edges_.end(); ++ei) { @@ -332,12 +332,12 @@ void Plan::CleanNode(BuildLog* build_log, Node* node) { bool all_outputs_clean = true; for (vector::iterator ni = (*ei)->outputs_.begin(); ni != (*ei)->outputs_.end(); ++ni) { - if (!(*ni)->dirty_) + if (!(*ni)->dirty()) continue; if ((*ei)->RecomputeOutputDirty(build_log, most_recent_input, command, *ni)) { - (*ni)->dirty_ = true; + (*ni)->MarkDirty(); all_outputs_clean = false; } else { CleanNode(build_log, *ni); diff --git a/src/build_test.cc b/src/build_test.cc index eb9ead0..5a95701 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -29,8 +29,8 @@ TEST_F(PlanTest, Basic) { AssertParse(&state_, "build out: cat mid\n" "build mid: cat in\n"); - GetNode("mid")->dirty_ = true; - GetNode("out")->dirty_ = true; + GetNode("mid")->MarkDirty(); + GetNode("out")->MarkDirty(); string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -62,9 +62,9 @@ TEST_F(PlanTest, DoubleOutputDirect) { AssertParse(&state_, "build out: cat mid1 mid2\n" "build mid1 mid2: cat in\n"); - GetNode("mid1")->dirty_ = true; - GetNode("mid2")->dirty_ = true; - GetNode("out")->dirty_ = true; + GetNode("mid1")->MarkDirty(); + GetNode("mid2")->MarkDirty(); + GetNode("out")->MarkDirty(); string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); @@ -91,11 +91,11 @@ TEST_F(PlanTest, DoubleOutputIndirect) { "build b1: cat a1\n" "build b2: cat a2\n" "build a1 a2: cat in\n"); - GetNode("a1")->dirty_ = true; - GetNode("a2")->dirty_ = true; - GetNode("b1")->dirty_ = true; - GetNode("b2")->dirty_ = true; - GetNode("out")->dirty_ = true; + GetNode("a1")->MarkDirty(); + GetNode("a2")->MarkDirty(); + GetNode("b1")->MarkDirty(); + GetNode("b2")->MarkDirty(); + GetNode("out")->MarkDirty(); string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -129,10 +129,10 @@ TEST_F(PlanTest, DoubleDependent) { "build a1: cat mid\n" "build a2: cat mid\n" "build mid: cat in\n"); - GetNode("mid")->dirty_ = true; - GetNode("a1")->dirty_ = true; - GetNode("a2")->dirty_ = true; - GetNode("out")->dirty_ = true; + GetNode("mid")->MarkDirty(); + GetNode("a1")->MarkDirty(); + GetNode("a2")->MarkDirty(); + GetNode("out")->MarkDirty(); string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); @@ -166,10 +166,10 @@ TEST_F(PlanTest, DependencyCycle) { "build mid: cat in\n" "build in: cat pre\n" "build pre: cat out\n"); - GetNode("out")->dirty_ = true; - GetNode("mid")->dirty_ = true; - GetNode("in")->dirty_ = true; - GetNode("pre")->dirty_ = true; + GetNode("out")->MarkDirty(); + GetNode("mid")->MarkDirty(); + GetNode("in")->MarkDirty(); + GetNode("pre")->MarkDirty(); string err; EXPECT_FALSE(plan_.AddTarget(GetNode("out"), &err)); @@ -217,7 +217,7 @@ struct BuildTest : public StateTestWithBuiltinRules, void BuildTest::Dirty(const string& path) { Node* node = GetNode(path); - node->dirty_ = true; + node->MarkDirty(); // If it's an input file, mark that we've already stat()ed it and // it's missing. @@ -461,7 +461,7 @@ TEST_F(BuildTest, DepFileOK) { Edge* edge = state_.edges_.back(); fs_.Create("foo.c", now_, ""); - GetNode("bar.h")->dirty_ = true; // Mark bar.h as missing. + GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. fs_.Create("foo.o.d", now_, "foo.o: blah.h bar.h\n"); EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); ASSERT_EQ("", err); diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index a7f94bb..a39c06a 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -214,9 +214,9 @@ TEST_F(StatTest, TwoStep) { edge->RecomputeDirty(NULL, this, NULL); ASSERT_EQ(3u, stats_.size()); ASSERT_EQ("out", stats_[0]); - ASSERT_TRUE(GetNode("out")->dirty_); + ASSERT_TRUE(GetNode("out")->dirty()); ASSERT_EQ("mid", stats_[1]); - ASSERT_TRUE(GetNode("mid")->dirty_); + ASSERT_TRUE(GetNode("mid")->dirty()); ASSERT_EQ("in", stats_[2]); } @@ -233,7 +233,7 @@ TEST_F(StatTest, Tree) { edge->RecomputeDirty(NULL, this, NULL); ASSERT_EQ(1u + 6u, stats_.size()); ASSERT_EQ("mid1", stats_[1]); - ASSERT_TRUE(GetNode("mid1")->dirty_); + ASSERT_TRUE(GetNode("mid1")->dirty()); ASSERT_EQ("in11", stats_[2]); } @@ -251,9 +251,9 @@ TEST_F(StatTest, Middle) { ASSERT_EQ(1u, stats_.size()); Edge* edge = out->in_edge_; edge->RecomputeDirty(NULL, this, NULL); - ASSERT_FALSE(GetNode("in")->dirty_); - ASSERT_TRUE(GetNode("mid")->dirty_); - ASSERT_TRUE(GetNode("out")->dirty_); + ASSERT_FALSE(GetNode("in")->dirty()); + ASSERT_TRUE(GetNode("mid")->dirty()); + ASSERT_TRUE(GetNode("out")->dirty()); } } // namespace diff --git a/src/graph.cc b/src/graph.cc index 0aa155b..4de7226 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -47,7 +47,7 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface, return false; } else { // This input has no in-edge; it is dirty if it is missing. - (*i)->dirty_ = !(*i)->exists(); + (*i)->set_dirty(!(*i)->exists()); } } @@ -60,7 +60,7 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface, if (!is_order_only(i - inputs_.begin())) { // If a regular input is dirty (or missing), we're dirty. // Otherwise consider mtime. - if ((*i)->dirty_) { + if ((*i)->dirty()) { dirty = true; } else { if ((*i)->mtime() > most_recent_input) @@ -90,7 +90,7 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface, for (vector::iterator i = outputs_.begin(); i != outputs_.end(); ++i) { (*i)->StatIfNecessary(disk_interface); if (dirty) - (*i)->dirty_ = true; + (*i)->MarkDirty(); } // If we're dirty, our outputs are not ready. (It's possible to be diff --git a/src/graph.h b/src/graph.h index a52a3d7..694cf35 100644 --- a/src/graph.h +++ b/src/graph.h @@ -65,19 +65,25 @@ struct Node { time_t mtime() const { return mtime_; } bool dirty() const { return dirty_; } + void set_dirty(bool dirty) { dirty_ = dirty; } + void MarkDirty() { dirty_ = true; } private: string path_; - // Possible values of mtime_: - // -1: file hasn't been examined - // 0: we looked, and file doesn't exist - // >0: actual file's mtime + /// Possible values of mtime_: + /// -1: file hasn't been examined + /// 0: we looked, and file doesn't exist + /// >0: actual file's mtime time_t mtime_; + /// Dirty is true when the underlying file is out-of-date. + /// But note that Edge::outputs_ready_ is also used in judging which + /// edges to build. + bool dirty_; + // TODO: make these private as well. But don't just blindly add // setters/getters, instead pay attention to the proper API. public: - bool dirty_; Edge* in_edge_; vector out_edges_; }; diff --git a/src/graph_test.cc b/src/graph_test.cc index 8ab921e..ab2213a 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -34,7 +34,7 @@ TEST_F(GraphTest, MissingImplicit) { // A missing implicit dep *should* make the output dirty. // (In fact, a build will fail.) // This is a change from prior semantics of ninja. - EXPECT_TRUE(GetNode("out")->dirty_); + EXPECT_TRUE(GetNode("out")->dirty()); } TEST_F(GraphTest, ModifiedImplicit) { @@ -50,7 +50,7 @@ TEST_F(GraphTest, ModifiedImplicit) { ASSERT_EQ("", err); // A modified implicit dep should make the output dirty. - EXPECT_TRUE(GetNode("out")->dirty_); + EXPECT_TRUE(GetNode("out")->dirty()); } TEST_F(GraphTest, FunkyMakefilePath) { @@ -71,7 +71,7 @@ TEST_F(GraphTest, FunkyMakefilePath) { // implicit.h has changed, though our depfile refers to it with a // non-canonical path; we should still find it. - EXPECT_TRUE(GetNode("out.o")->dirty_); + EXPECT_TRUE(GetNode("out.o")->dirty()); } TEST_F(GraphTest, ExplicitImplicit) { @@ -95,7 +95,7 @@ TEST_F(GraphTest, ExplicitImplicit) { // We have both an implicit and an explicit dep on implicit.h. // The implicit dep should "win" (in the sense that it should cause // the output to be dirty). - EXPECT_TRUE(GetNode("out.o")->dirty_); + EXPECT_TRUE(GetNode("out.o")->dirty()); } TEST_F(GraphTest, PathWithCurrentDirectory) { @@ -113,7 +113,7 @@ TEST_F(GraphTest, PathWithCurrentDirectory) { EXPECT_TRUE(edge->RecomputeDirty(&state_, &fs_, &err)); ASSERT_EQ("", err); - EXPECT_FALSE(GetNode("out.o")->dirty_); + EXPECT_FALSE(GetNode("out.o")->dirty()); } TEST_F(GraphTest, RootNodes) { -- 2.7.4