Ninja: clarify how DepNodes are passed and owned

Refactor the CommandEvaluator and Executor to prefer const& DepNode over
DepNode* as arguments. That makes ownership mutability more obvious.

Signed-off-by: Matthias Maennich <maennich@google.com>

4 files changed, 43 insertions, 44 deletions

diff --git a/src/command.cc b/src/command.cc
index 9d594f8..9178e9e 100644
--- a/src/command.cc
+++ b/src/command.cc
@@ -185,12 +185,12 @@ CommandEvaluator::CommandEvaluator(Evaluator* ev) : ev_(ev) {
   INSERT_AUTO_VAR(AutoNotImplementedVar, "|");
 }
 
-std::vector<Command> CommandEvaluator::Eval(DepNode* n) {
+std::vector<Command> CommandEvaluator::Eval(const DepNode& n) {
   std::vector<Command> result;
-  ev_->set_loc(n->loc);
-  ev_->set_current_scope(n->rule_vars);
-  current_dep_node_ = n;
-  for (Value* v : n->cmds) {
+  ev_->set_loc(n.loc);
+  ev_->set_current_scope(n.rule_vars);
+  current_dep_node_ = &n;
+  for (Value* v : n.cmds) {
     ev_->set_loc(v->Location());
     const string&& cmds_buf = v->Eval(ev_);
     StringPiece cmds = cmds_buf;
@@ -212,7 +212,7 @@ std::vector<Command> CommandEvaluator::Eval(DepNode* n) {
       ParseCommandPrefixes(&cmd, &echo, &ignore_error);
 
       if (!cmd.empty()) {
-        Command& command = result.emplace_back(n->output);
+        Command& command = result.emplace_back(n.output);
         command.cmd = cmd.as_string();
         command.echo = echo;
         command.ignore_error = ignore_error;
@@ -226,7 +226,7 @@ std::vector<Command> CommandEvaluator::Eval(DepNode* n) {
   if (!ev_->delayed_output_commands().empty()) {
     std::vector<Command> output_commands;
     for (const string& cmd : ev_->delayed_output_commands()) {
-      Command& c = output_commands.emplace_back(n->output);
+      Command& c = output_commands.emplace_back(n.output);
       c.cmd = cmd;
       c.echo = false;
       c.ignore_error = false;
diff --git a/src/command.h b/src/command.h
index e55ca93..017f6bb 100644
--- a/src/command.h
+++ b/src/command.h
@@ -35,13 +35,13 @@ struct Command {
 class CommandEvaluator {
  public:
   explicit CommandEvaluator(Evaluator* ev);
-  std::vector<Command> Eval(DepNode* n);
+  std::vector<Command> Eval(const DepNode& n);
   const DepNode* current_dep_node() const { return current_dep_node_; }
   Evaluator* evaluator() const { return ev_; }
 
  private:
   Evaluator* ev_;
-  DepNode* current_dep_node_;
+  const DepNode* current_dep_node_;
 };
 
 #endif  // COMMAND_H_
diff --git a/src/exec.cc b/src/exec.cc
index 37ffc12..40f37dc 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -49,52 +49,51 @@ class Executor {
     shellflag_ = ev->GetShellFlag();
   }
 
-  double ExecNode(DepNode* n, DepNode* needed_by) {
-    auto found = done_.find(n->output);
+  double ExecNode(const DepNode& n, const char* needed_by) {
+    auto found = done_.find(n.output);
     if (found != done_.end()) {
       if (found->second == kProcessing) {
         WARN("Circular %s <- %s dependency dropped.",
-             needed_by ? needed_by->output.c_str() : "(null)",
-             n->output.c_str());
+             needed_by ? needed_by : "(null)", n.output.c_str());
       }
       return found->second;
     }
     ScopedFrame frame(
-        ce_.evaluator()->Enter(FrameType::EXEC, n->output.c_str(), n->loc));
+        ce_.evaluator()->Enter(FrameType::EXEC, n.output.c_str(), n.loc));
 
-    done_[n->output] = kProcessing;
-    double output_ts = GetTimestamp(n->output.c_str());
+    done_[n.output] = kProcessing;
+    double output_ts = GetTimestamp(n.output.c_str());
 
-    LOG("ExecNode: %s for %s", n->output.c_str(),
-        needed_by ? needed_by->output.c_str() : "(null)");
+    LOG("ExecNode: %s for %s", n.output.c_str(),
+        needed_by ? needed_by : "(null)");
 
-    if (!n->has_rule && output_ts == kNotExist && !n->is_phony) {
+    if (!n.has_rule && output_ts == kNotExist && !n.is_phony) {
       if (needed_by) {
         ERROR("*** No rule to make target '%s', needed by '%s'.",
-              n->output.c_str(), needed_by->output.c_str());
+              n.output.c_str(), needed_by);
       } else {
-        ERROR("*** No rule to make target '%s'.", n->output.c_str());
+        ERROR("*** No rule to make target '%s'.", n.output.c_str());
       }
     }
 
     double latest = kProcessing;
-    for (auto const& d : n->order_onlys) {
+    for (auto const& d : n.order_onlys) {
       if (Exists(d.second->output.str())) {
         continue;
       }
-      double ts = ExecNode(d.second, n);
+      double ts = ExecNode(*d.second, n.output.c_str());
       if (latest < ts)
         latest = ts;
     }
 
-    for (auto const& d : n->deps) {
-      double ts = ExecNode(d.second, n);
+    for (auto const& d : n.deps) {
+      double ts = ExecNode(*d.second, n.output.c_str());
       if (latest < ts)
         latest = ts;
     }
 
-    if (output_ts >= latest && !n->is_phony) {
-      done_[n->output] = output_ts;
+    if (output_ts >= latest && !n.is_phony) {
+      done_[n.output] = output_ts;
       return output_ts;
     }
 
@@ -123,7 +122,7 @@ class Executor {
       }
     }
 
-    done_[n->output] = output_ts;
+    done_[n.output] = output_ts;
     return output_ts;
   }
 
@@ -142,7 +141,7 @@ class Executor {
 void Exec(const vector<NamedDepNode>& roots, Evaluator* ev) {
   Executor executor(ev);
   for (auto const& root : roots) {
-    executor.ExecNode(root.second, NULL);
+    executor.ExecNode(*root.second, nullptr);
   }
   if (executor.Count() == 0) {
     for (auto const& root : roots) {
diff --git a/src/ninja.cc b/src/ninja.cc
index 2cdfb49..5e55fd9 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -221,41 +221,41 @@ class NinjaGenerator {
   void PopulateNinjaNodes(const vector<NamedDepNode>& nodes) {
     ScopedTimeReporter tr("ninja gen (eval)");
     for (auto const& node : nodes) {
-      PopulateNinjaNode(node.second);
+      PopulateNinjaNode(*node.second);
     }
   }
 
-  void PopulateNinjaNode(DepNode* node) {
-    if (done_.exists(node->output)) {
+  void PopulateNinjaNode(const DepNode& node) {
+    if (done_.exists(node.output)) {
       return;
     }
-    done_.insert(node->output);
-    ScopedFrame frame(ce_.evaluator()->Enter(FrameType::NINJA,
-                                             node->output.str(), node->loc));
+    done_.insert(node.output);
+    ScopedFrame frame(
+        ce_.evaluator()->Enter(FrameType::NINJA, node.output.str(), node.loc));
 
     // A hack to exclude out phony target in Android. If this exists,
     // "ninja -t clean" tries to remove this directory and fails.
-    if (g_flags.detect_android_echo && node->output.str() == "out")
+    if (g_flags.detect_android_echo && node.output.str() == "out")
       return;
 
     // This node is a leaf node
-    if (!node->has_rule && !node->is_phony) {
+    if (!node.has_rule && !node.is_phony) {
       return;
     }
 
     NinjaNode& nn = nodes_.emplace_back();
-    nn.node = node;
+    nn.node = &node;
     nn.commands = ce_.Eval(node);
     nn.rule_id = nn.commands.empty() ? -1 : rule_id_++;
 
-    for (auto const& d : node->deps) {
-      PopulateNinjaNode(d.second);
+    for (auto const& d : node.deps) {
+      PopulateNinjaNode(*d.second);
     }
-    for (auto const& d : node->order_onlys) {
-      PopulateNinjaNode(d.second);
+    for (auto const& d : node.order_onlys) {
+      PopulateNinjaNode(*d.second);
     }
-    for (auto const& d : node->validations) {
-      PopulateNinjaNode(d.second);
+    for (auto const& d : node.validations) {
+      PopulateNinjaNode(*d.second);
     }
   }