refactor RecomputeOutputDirty to return true/false for dirty

Rather than taking whether the edge is dirty as an input, instead
only consider the arguments and return true/false, allowing the caller
to decide what to do with that information.  (In the restat case we
were faking out the environment to get particular behavior for this.)

Should have no side effects, but this is part of fixing issue 148.

3 files changed, 38 insertions, 39 deletions

diff --git a/src/build.cc b/src/build.cc
index ae6ebbb..25d3ba9 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -337,14 +337,9 @@ void Plan::CleanNode(BuildLog* build_log, Node* node) {
         if (!(*ni)->dirty_)
           continue;
 
-        // RecomputeOutputDirty will not modify dirty_ if the output is clean.
-        (*ni)->dirty_ = false;
-
-        // Since we know that all non-order-only inputs are clean, we can pass
-        // "false" as the "dirty" argument here.
-        (*ei)->RecomputeOutputDirty(build_log, most_recent_input, false,
-                                    command, *ni);
-        if ((*ni)->dirty_) {
+        if ((*ei)->RecomputeOutputDirty(build_log, most_recent_input, command,
+                                        *ni)) {
+          (*ni)->dirty_ = true;
           all_outputs_clean = false;
         } else {
           CleanNode(build_log, *ni);
diff --git a/src/graph.cc b/src/graph.cc
index d13b10c..341cfd5 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -77,35 +77,31 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface,
     // yet (or never will).  Stat them if we haven't already to mark that we've
     // visited their dependents.
     (*i)->file_->StatIfNecessary(disk_interface);
-
-    RecomputeOutputDirty(build_log, most_recent_input, dirty, command, *i);
-    if ((*i)->dirty_)
+    if (dirty || RecomputeOutputDirty(build_log, most_recent_input, command, *i)) {
+      (*i)->dirty_ = true;
       outputs_ready_ = false;
+    }
   }
 
   return true;
 }
 
-void Edge::RecomputeOutputDirty(BuildLog* build_log, time_t most_recent_input,
-                                bool dirty, const string& command,
-                                Node* output) {
+bool Edge::RecomputeOutputDirty(BuildLog* build_log, time_t most_recent_input,
+                                const string& command, Node* output) {
   if (is_phony()) {
     // Phony edges don't write any output.
-    // They're only dirty if an input is dirty, or if there are no inputs
-    // and we're missing the output.
-    if (dirty)
-      output->dirty_ = true;
-    else if (inputs_.empty() && !output->file_->exists())
-      output->dirty_ = true;
-    return;
+    // Outputs are only dirty if there are no inputs and we're missing the output.
+    return inputs_.empty() && !output->file_->exists();
   }
 
   BuildLog::LogEntry* entry = 0;
-  // Output is dirty if we're dirty, we're missing the output,
-  // or if it's older than the most recent input mtime.
-  if (dirty || !output->file_->exists()) {
-    output->dirty_ = true;
-  } else if (output->file_->mtime_ < most_recent_input) {
+
+  // Dirty if we're missing the output.
+  if (!output->file_->exists())
+    return true;
+
+  // Dirty if the output is older than the input.
+  if (output->file_->mtime_ < most_recent_input) {
     // If this is a restat rule, we may have cleaned the output with a restat
     // rule in a previous run and stored the most recent input mtime in the
     // build log.  Use that mtime instead, so that the file will only be
@@ -113,22 +109,22 @@ void Edge::RecomputeOutputDirty(BuildLog* build_log, time_t most_recent_input,
     if (rule_->restat_ && build_log &&
         (entry = build_log->LookupByOutput(output->file_->path_))) {
       if (entry->restat_mtime < most_recent_input)
-        output->dirty_ = true;
+        return true;
     } else {
-      output->dirty_ = true;
+      return true;
     }
   }
 
-  if (!output->dirty_) {
-    // May also be dirty due to the command changing since the last build.
-    // But if this is a generator rule, the command changing does not make us
-    // dirty.
-    if (!rule_->generator_ && build_log &&
-        (entry || (entry = build_log->LookupByOutput(output->file_->path_)))) {
-      if (command != entry->command)
-        output->dirty_ = true;
-    }
+  // May also be dirty due to the command changing since the last build.
+  // But if this is a generator rule, the command changing does not make us
+  // dirty.
+  if (!rule_->generator_ && build_log &&
+      (entry || (entry = build_log->LookupByOutput(output->file_->path_)))) {
+    if (command != entry->command)
+      return true;
   }
+
+  return false;
 }
 
 /// An Env for an Edge, providing $in and $out.
diff --git a/src/graph.h b/src/graph.h
index 64496d5..9080dcc 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -80,9 +80,17 @@ struct Edge {
   Edge() : rule_(NULL), env_(NULL), outputs_ready_(false), implicit_deps_(0),
            order_only_deps_(0) {}
 
+  /// Examine inputs, outputs, and command lines to judge whether this edge needs
+  /// to be re-run, and update outputs_ready_ and each outputs' dirty_ state
+  /// accordingly.
+  /// Returns false on failure.
   bool RecomputeDirty(State* state, DiskInterface* disk_interface, string* err);
-  void RecomputeOutputDirty(BuildLog* build_log, time_t most_recent_input,
-                            bool dirty, const string& command, Node* output);
+
+  /// Recompute whether a given single output should be marked dirty.
+  /// Returns true if so.
+  bool RecomputeOutputDirty(BuildLog* build_log, time_t most_recent_input,
+                            const string& command, Node* output);
+
   string EvaluateCommand();  // XXX move to env, take env ptr
   string GetDescription();
   bool LoadDepFile(State* state, DiskInterface* disk_interface, string* err);