From 13dd08c1a03e5a8f4299816fbd3af1b6cb6d9642 Mon Sep 17 00:00:00 2001
From: Evan Martin <martine@danga.com>
Date: Sat, 27 Oct 2012 13:09:40 -0700
Subject: rearrange handling of builtin bindings to make rules simpler

Now, a 'build' block can override any special binding like 'command'
or 'description' if it needs to.
---
 src/build.cc                | 38 +++++++++++----------
 src/clean.cc                |  6 ++--
 src/eval_env.cc             | 16 +++++++++
 src/eval_env.h              | 12 +++++++
 src/graph.cc                | 81 +++++++++++++++++++++++++--------------------
 src/graph.h                 | 43 ++++++------------------
 src/graph_test.cc           | 35 ++++++++++++++++++++
 src/manifest_parser.cc      | 76 ++++++++++++++----------------------------
 src/manifest_parser_test.cc | 20 ++++++-----
 src/state.cc                |  4 +--
 src/state.h                 |  2 +-
 src/state_test.cc           |  4 +--
 12 files changed, 183 insertions(+), 154 deletions(-)

diff --git a/src/build.cc b/src/build.cc
index b4229c4..701fa92 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -256,9 +256,9 @@ void BuildStatus::PrintStatus(Edge* edge) {
 
   bool force_full_command = config_.verbosity == BuildConfig::VERBOSE;
 
-  string to_print = edge->GetDescription();
+  string to_print = edge->GetBinding("description");
   if (to_print.empty() || force_full_command)
-    to_print = edge->EvaluateCommand();
+    to_print = edge->GetBinding("command");
 
 #ifdef _WIN32
   CONSOLE_SCREEN_BUFFER_INFO csbi;
@@ -612,7 +612,7 @@ void Builder::Cleanup() {
 
     for (vector<Edge*>::iterator i = active_edges.begin();
          i != active_edges.end(); ++i) {
-      bool has_depfile = !(*i)->rule_->depfile().empty();
+      string depfile = (*i)->GetBinding("depfile");
       for (vector<Node*>::iterator ni = (*i)->outputs_.begin();
            ni != (*i)->outputs_.end(); ++ni) {
         // Only delete this output if it was actually modified.  This is
@@ -622,12 +622,13 @@ void Builder::Cleanup() {
         // need to rebuild an output because of a modified header file
         // mentioned in a depfile, and the command touches its depfile
         // but is interrupted before it touches its output file.)
-        if (has_depfile ||
-            (*ni)->mtime() != disk_interface_->Stat((*ni)->path()))
+        if (!depfile.empty() ||
+            (*ni)->mtime() != disk_interface_->Stat((*ni)->path())) {
           disk_interface_->RemoveFile((*ni)->path());
+        }
       }
-      if (has_depfile)
-        disk_interface_->RemoveFile((*i)->EvaluateDepFile());
+      if (!depfile.empty())
+        disk_interface_->RemoveFile(depfile);
     }
   }
 }
@@ -771,11 +772,11 @@ bool Builder::StartEdge(Edge* edge, string* err) {
 
   // Create response file, if needed
   // XXX: this may also block; do we care?
-  if (edge->HasRspFile()) {
-    if (!disk_interface_->WriteFile(edge->GetRspFile(),
-                                    edge->GetRspFileContent())) {
+  string rspfile = edge->GetBinding("rspfile");
+  if (!rspfile.empty()) {
+    string content = edge->GetBinding("rspfile_content");
+    if (!disk_interface_->WriteFile(rspfile, content))
       return false;
-    }
   }
 
   // start command computing and run it
@@ -792,7 +793,7 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
   TimeStamp restat_mtime = 0;
 
   if (success) {
-    if (edge->rule().restat() && !config_.dry_run) {
+    if (edge->GetBindingBool("restat") && !config_.dry_run) {
       bool node_cleaned = false;
 
       for (vector<Node*>::iterator i = edge->outputs_.begin();
@@ -817,9 +818,9 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
             restat_mtime = input_mtime;
         }
 
-        if (restat_mtime != 0 && !edge->rule().depfile().empty()) {
-          TimeStamp depfile_mtime =
-              disk_interface_->Stat(edge->EvaluateDepFile());
+        string depfile = edge->GetBinding("depfile");
+        if (restat_mtime != 0 && !depfile.empty()) {
+          TimeStamp depfile_mtime = disk_interface_->Stat(depfile);
           if (depfile_mtime > restat_mtime)
             restat_mtime = depfile_mtime;
         }
@@ -830,9 +831,10 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
       }
     }
 
-    // delete the response file on success (if exists)
-    if (edge->HasRspFile())
-      disk_interface_->RemoveFile(edge->GetRspFile());
+    // Delete the response file on success (if exists)
+    string rspfile = edge->GetBinding("rspfile");
+    if (!rspfile.empty())
+      disk_interface_->RemoveFile(rspfile);
 
     plan_.EdgeFinished(edge);
   }
diff --git a/src/clean.cc b/src/clean.cc
index 0b8476b..12afb98 100644
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -83,11 +83,11 @@ bool Cleaner::IsAlreadyRemoved(const string& path) {
 }
 
 void Cleaner::RemoveEdgeFiles(Edge* edge) {
-  string depfile = edge->EvaluateDepFile();
+  string depfile = edge->GetBinding("depfile");
   if (!depfile.empty())
     Remove(depfile);
 
-  string rspfile = edge->GetRspFile();
+  string rspfile = edge->GetBinding("rspfile");
   if (!rspfile.empty())
     Remove(rspfile);
 }
@@ -117,7 +117,7 @@ int Cleaner::CleanAll(bool generator) {
     if ((*e)->is_phony())
       continue;
     // Do not remove generator's files unless generator specified.
-    if (!generator && (*e)->rule().generator())
+    if (!generator && (*e)->GetBindingBool("generator"))
       continue;
     for (vector<Node*>::iterator out_node = (*e)->outputs_.begin();
          out_node != (*e)->outputs_.end(); ++out_node) {
diff --git a/src/eval_env.cc b/src/eval_env.cc
index 81a8765..834b7e1 100644
--- a/src/eval_env.cc
+++ b/src/eval_env.cc
@@ -27,6 +27,22 @@ void BindingEnv::AddBinding(const string& key, const string& val) {
   bindings_[key] = val;
 }
 
+string BindingEnv::LookupWithFallback(const string& var,
+                                      const EvalString* eval,
+                                      Env* env) {
+  map<string, string>::iterator i = bindings_.find(var);
+  if (i != bindings_.end())
+    return i->second;
+
+  if (eval)
+    return eval->Evaluate(env);
+
+  if (parent_)
+    return parent_->LookupVariable(var);
+
+  return "";
+}
+
 string EvalString::Evaluate(Env* env) const {
   string result;
   for (TokenList::const_iterator i = parsed_.begin(); i != parsed_.end(); ++i) {
diff --git a/src/eval_env.h b/src/eval_env.h
index 6e0a0c0..f3c959a 100644
--- a/src/eval_env.h
+++ b/src/eval_env.h
@@ -22,6 +22,8 @@ using namespace std;
 
 #include "string_piece.h"
 
+struct EvalString;
+
 /// An interface for a scope for variable (e.g. "$foo") lookups.
 struct Env {
   virtual ~Env() {}
@@ -33,10 +35,20 @@ struct Env {
 struct BindingEnv : public Env {
   BindingEnv() : parent_(NULL) {}
   explicit BindingEnv(Env* parent) : parent_(parent) {}
+
   virtual ~BindingEnv() {}
   virtual string LookupVariable(const string& var);
+
   void AddBinding(const string& key, const string& val);
 
+  /// This is tricky.  Edges want lookup scope to go in this order:
+  /// 1) value set on edge itself (edge_->env_)
+  /// 2) value set on rule, with expansion in the edge's scope
+  /// 3) value set on enclosing scope of edge (edge_->env_->parent_)
+  /// This function takes as parameters the necessary info to do (2).
+  string LookupWithFallback(const string& var, const EvalString* eval,
+                            Env* env);
+
 private:
   map<string, string> bindings_;
   Env* parent_;
diff --git a/src/graph.cc b/src/graph.cc
index f9b9c6f..380ca7c 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -32,16 +32,40 @@ bool Node::Stat(DiskInterface* disk_interface) {
   return mtime_ > 0;
 }
 
+void Rule::AddBinding(const string& key, const EvalString& val) {
+  bindings_[key] = val;
+}
+
+const EvalString* Rule::GetBinding(const string& key) const {
+  map<string, EvalString>::const_iterator i = bindings_.find(key);
+  if (i == bindings_.end())
+    return NULL;
+  return &i->second;
+}
+
+// static
+bool Rule::IsReservedBinding(const string& var) {
+  return var == "command" ||
+      var == "depfile" ||
+      var == "description" ||
+      var == "generator" ||
+      var == "pool" ||
+      var == "restat" ||
+      var == "rspfile" ||
+      var == "rspfile_content";
+}
+
 bool DependencyScan::RecomputeDirty(Edge* edge, string* err) {
   bool dirty = false;
   edge->outputs_ready_ = true;
 
-  if (!edge->rule_->depfile().empty()) {
-    if (!LoadDepFile(edge, err)) {
+  string depfile = edge->GetBinding("depfile");
+  if (!depfile.empty()) {
+    if (!LoadDepFile(edge, depfile, err)) {
       if (!err->empty())
         return false;
       EXPLAIN("Edge targets are dirty because depfile '%s' is missing",
-              edge->EvaluateDepFile().c_str());
+              depfile.c_str());
       dirty = true;
     }
   }
@@ -142,7 +166,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge,
     // build log.  Use that mtime instead, so that the file will only be
     // considered dirty if an input was modified since the previous run.
     TimeStamp most_recent_stamp = most_recent_input->mtime();
-    if (edge->rule_->restat() && build_log() &&
+    if (edge->GetBindingBool("restat") && build_log() &&
         (entry = build_log()->LookupByOutput(output->path()))) {
       if (entry->restat_mtime < most_recent_stamp) {
         EXPLAIN("restat of output %s older than most recent input %s "
@@ -162,7 +186,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge,
   // 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 (!edge->rule_->generator() && build_log()) {
+  if (!edge->GetBindingBool("generator") && build_log()) {
     if (entry || (entry = build_log()->LookupByOutput(output->path()))) {
       if (BuildLog::LogEntry::HashCommand(command) != entry->command_hash) {
         EXPLAIN("command line changed for %s", output->path().c_str());
@@ -212,11 +236,11 @@ string EdgeEnv::LookupVariable(const string& var) {
     return MakePathList(edge_->outputs_.begin(),
                         edge_->outputs_.end(),
                         ' ');
-  } else if (edge_->env_) {
-    return edge_->env_->LookupVariable(var);
-  } else {
-    return string();
   }
+
+  // See notes on BindingEnv::LookupWithFallback.
+  const EvalString* eval = edge_->rule_->GetBinding(var);
+  return edge_->env_->LookupWithFallback(var, eval, this);
 }
 
 string EdgeEnv::MakePathList(vector<Node*>::iterator begin,
@@ -239,40 +263,26 @@ string EdgeEnv::MakePathList(vector<Node*>::iterator begin,
 }
 
 string Edge::EvaluateCommand(bool incl_rsp_file) {
-  EdgeEnv env(this);
-  string command = rule_->command().Evaluate(&env);
-  if (incl_rsp_file && HasRspFile())
-    command += ";rspfile=" + GetRspFileContent();
+  string command = GetBinding("command");
+  if (incl_rsp_file) {
+    string rspfile_content = GetBinding("rspfile_content");
+    if (!rspfile_content.empty())
+      command += ";rspfile=" + rspfile_content;
+  }
   return command;
 }
 
-string Edge::EvaluateDepFile() {
+string Edge::GetBinding(const string& key) {
   EdgeEnv env(this);
-  return rule_->depfile().Evaluate(&env);
+  return env.LookupVariable(key);
 }
 
-string Edge::GetDescription() {
-  EdgeEnv env(this);
-  return rule_->description().Evaluate(&env);
-}
-
-bool Edge::HasRspFile() {
-  return !rule_->rspfile().empty();
-}
-
-string Edge::GetRspFile() {
-  EdgeEnv env(this);
-  return rule_->rspfile().Evaluate(&env);
-}
-
-string Edge::GetRspFileContent() {
-  EdgeEnv env(this);
-  return rule_->rspfile_content().Evaluate(&env);
+bool Edge::GetBindingBool(const string& key) {
+  return !GetBinding(key).empty();
 }
 
-bool DependencyScan::LoadDepFile(Edge* edge, string* err) {
+bool DependencyScan::LoadDepFile(Edge* edge, const string& path, string* err) {
   METRIC_RECORD("depfile load");
-  string path = edge->EvaluateDepFile();
   string content = disk_interface_->ReadFile(path, err);
   if (!err->empty())
     return false;
@@ -317,8 +327,7 @@ bool DependencyScan::LoadDepFile(Edge* edge, string* err) {
     // create one; this makes us not abort if the input is missing,
     // but instead will rebuild in that circumstance.
     if (!node->in_edge()) {
-      Edge* phony_edge = state_->AddEdge(&State::kPhonyRule,
-                                         &State::kDefaultPool);
+      Edge* phony_edge = state_->AddEdge(&State::kPhonyRule);
       node->set_in_edge(phony_edge);
       phony_edge->outputs_.push_back(node);
 
diff --git a/src/graph.h b/src/graph.h
index 3c31e19..8b93e29 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -102,38 +102,23 @@ private:
 
 /// An invokable build command and associated metadata (description, etc.).
 struct Rule {
-  explicit Rule(const string& name)
-      : name_(name), generator_(false), restat_(false) {}
+  explicit Rule(const string& name) : name_(name) {}
 
   const string& name() const { return name_; }
 
-  bool generator() const { return generator_; }
-  bool restat() const { return restat_; }
+  typedef map<string, EvalString> Bindings;
+  void AddBinding(const string& key, const EvalString& val);
 
-  const EvalString& command() const { return command_; }
-  const EvalString& description() const { return description_; }
-  const EvalString& depfile() const { return depfile_; }
-  const EvalString& rspfile() const { return rspfile_; }
-  const EvalString& rspfile_content() const { return rspfile_content_; }
+  static bool IsReservedBinding(const string& var);
 
-  /// Used by a test.
-  void set_command(const EvalString& command) { command_ = command; }
+  const EvalString* GetBinding(const string& key) const;
 
  private:
   // Allow the parsers to reach into this object and fill out its fields.
   friend struct ManifestParser;
 
   string name_;
-
-  bool generator_;
-  bool restat_;
-
-  EvalString command_;
-  EvalString description_;
-  EvalString depfile_;
-  EvalString pool_;
-  EvalString rspfile_;
-  EvalString rspfile_content_;
+  map<string, EvalString> bindings_;
 };
 
 struct BuildLog;
@@ -153,17 +138,9 @@ struct Edge {
   /// If incl_rsp_file is enabled, the string will also contain the
   /// full contents of a response file (if applicable)
   string EvaluateCommand(bool incl_rsp_file = false);
-  string EvaluateDepFile();
-  string GetDescription();
-
-  /// Does the edge use a response file?
-  bool HasRspFile();
-
-  /// Get the path to the response file
-  string GetRspFile();
 
-  /// Get the contents of the response file
-  string GetRspFileContent();
+  string GetBinding(const string& key);
+  bool GetBindingBool(const string& key);
 
   void Dump(const char* prefix="") const;
 
@@ -171,7 +148,7 @@ struct Edge {
   Pool* pool_;
   vector<Node*> inputs_;
   vector<Node*> outputs_;
-  Env* env_;
+  BindingEnv* env_;
   bool outputs_ready_;
 
   const Rule& rule() const { return *rule_; }
@@ -220,7 +197,7 @@ struct DependencyScan {
   bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input,
                             const string& command, Node* output);
 
-  bool LoadDepFile(Edge* edge, string* err);
+  bool LoadDepFile(Edge* edge, const string& path, string* err);
 
   BuildLog* build_log() const {
     return build_log_;
diff --git a/src/graph_test.cc b/src/graph_test.cc
index 5b25c2f..396def4 100644
--- a/src/graph_test.cc
+++ b/src/graph_test.cc
@@ -187,3 +187,38 @@ TEST_F(GraphTest, DepfileRemoved) {
   ASSERT_EQ("", err);
   EXPECT_TRUE(GetNode("out.o")->dirty());
 }
+
+// Check that rule-level variables are in scope for eval.
+TEST_F(GraphTest, RuleVariablesInScope) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+"  depfile = x\n"
+"  command = depfile is $depfile\n"
+"build out: r in\n"));
+  Edge* edge = GetNode("out")->in_edge();
+  EXPECT_EQ("depfile is x", edge->EvaluateCommand());
+}
+
+// Check that build statements can override rule builtins like depfile.
+TEST_F(GraphTest, DepfileOverride) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+"  depfile = x\n"
+"  command = unused\n"
+"build out: r in\n"
+"  depfile = y\n"));
+  Edge* edge = GetNode("out")->in_edge();
+  EXPECT_EQ("y", edge->GetBinding("depfile"));
+}
+
+// Check that overridden values show up in expansion of rule-level bindings.
+TEST_F(GraphTest, DepfileOverrideParent) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+"  depfile = x\n"
+"  command = depfile is $depfile\n"
+"build out: r in\n"
+"  depfile = y\n"));
+  Edge* edge = GetNode("out")->in_edge();
+  EXPECT_EQ("depfile is y", edge->GetBinding("command"));
+}
diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 2d052b5..9d17932 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -154,22 +154,8 @@ bool ManifestParser::ParseRule(string* err) {
     if (!ParseLet(&key, &value, err))
       return false;
 
-    if (key == "command") {
-      rule->command_ = value;
-    } else if (key == "depfile") {
-      rule->depfile_ = value;
-    } else if (key == "description") {
-      rule->description_ = value;
-    } else if (key == "generator") {
-      rule->generator_ = true;
-    } else if (key == "restat") {
-      rule->restat_ = true;
-    } else if (key == "rspfile") {
-      rule->rspfile_ = value;
-    } else if (key == "rspfile_content") {
-      rule->rspfile_content_ = value;
-    } else if (key == "pool") {
-      rule->pool_ = value;
+    if (Rule::IsReservedBinding(key)) {
+      rule->AddBinding(key, value);
     } else {
       // Die on other keyvals for now; revisit if we want to add a
       // scope here.
@@ -177,12 +163,13 @@ bool ManifestParser::ParseRule(string* err) {
     }
   }
 
-  if (rule->rspfile_.empty() != rule->rspfile_content_.empty()) {
-    return lexer_.Error("rspfile and rspfile_content need to be both specified",
-                        err);
+  if (rule->bindings_["rspfile"].empty() !=
+      rule->bindings_["rspfile_content"].empty()) {
+    return lexer_.Error("rspfile and rspfile_content need to be "
+                        "both specified", err);
   }
 
-  if (rule->command_.empty())
+  if (rule->bindings_["command"].empty())
     return lexer_.Error("expected 'command =' line", err);
 
   state_->AddRule(rule);
@@ -296,42 +283,29 @@ bool ManifestParser::ParseEdge(string* err) {
   if (!ExpectToken(Lexer::NEWLINE, err))
     return false;
 
-  // Default to using outer env.
-  BindingEnv* env = env_;
-  Pool* pool = NULL;
+  // XXX scoped_ptr to handle error case.
+  BindingEnv* env = new BindingEnv(env_);
 
-  // But create and fill a nested env if there are variables in scope.
-  if (lexer_.PeekToken(Lexer::INDENT)) {
-    // XXX scoped_ptr to handle error case.
-    env = new BindingEnv(env_);
-    do {
-      string key;
-      EvalString val;
-      if (!ParseLet(&key, &val, err))
-        return false;
-      if (key == "pool") {
-        string pool_name = val.Evaluate(env_);
-        pool = state_->LookupPool(pool_name);
-        if (pool == NULL)
-          return lexer_.Error("undefined pool '" + pool_name + "'", err);
-      } else {
-        env->AddBinding(key, val.Evaluate(env_));
-      }
-    } while (lexer_.PeekToken(Lexer::INDENT));
-  }
+  while (lexer_.PeekToken(Lexer::INDENT)) {
+    string key;
+    EvalString val;
+    if (!ParseLet(&key, &val, err))
+      return false;
 
-  if (pool == NULL) {
-    if (!rule->pool_.empty()) {
-      pool = state_->LookupPool(rule->pool_.Evaluate(env_));
-      if (pool == NULL)
-        return lexer_.Error("cannot resolve pool for this edge.", err);
-    } else {
-      pool = &State::kDefaultPool;
-    }
+    env->AddBinding(key, val.Evaluate(env_));
   }
 
-  Edge* edge = state_->AddEdge(rule, pool);
+  Edge* edge = state_->AddEdge(rule);
   edge->env_ = env;
+
+  string pool_name = edge->GetBinding("pool");
+  if (!pool_name.empty()) {
+    Pool* pool = state_->LookupPool(pool_name);
+    if (pool == NULL)
+      return lexer_.Error("unknown pool name", err);
+    edge->pool_ = pool;
+  }
+
   for (vector<EvalString>::iterator i = ins.begin(); i != ins.end(); ++i) {
     string path = i->Evaluate(env);
     string path_err;
diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc
index 8b00efb..92f52d2 100644
--- a/src/manifest_parser_test.cc
+++ b/src/manifest_parser_test.cc
@@ -61,7 +61,8 @@ TEST_F(ParserTest, Rules) {
   ASSERT_EQ(3u, state.rules_.size());
   const Rule* rule = state.rules_.begin()->second;
   EXPECT_EQ("cat", rule->name());
-  EXPECT_EQ("[cat ][$in][ > ][$out]", rule->command().Serialize());
+  EXPECT_EQ("[cat ][$in][ > ][$out]",
+            rule->GetBinding("command")->Serialize());
 }
 
 TEST_F(ParserTest, RuleAttributes) {
@@ -92,8 +93,9 @@ TEST_F(ParserTest, IgnoreIndentedComments) {
   ASSERT_EQ(2u, state.rules_.size());
   const Rule* rule = state.rules_.begin()->second;
   EXPECT_EQ("cat", rule->name());
-  EXPECT_TRUE(rule->restat());
-  EXPECT_FALSE(rule->generator());
+  Edge* edge = state.GetNode("result")->in_edge();
+  EXPECT_TRUE(edge->GetBindingBool("restat"));
+  EXPECT_FALSE(edge->GetBindingBool("generator"));
 }
 
 TEST_F(ParserTest, IgnoreIndentedBlankLines) {
@@ -124,9 +126,10 @@ TEST_F(ParserTest, ResponseFiles) {
   ASSERT_EQ(2u, state.rules_.size());
   const Rule* rule = state.rules_.begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
-  EXPECT_EQ("[cat ][$rspfile][ > ][$out]", rule->command().Serialize());
-  EXPECT_EQ("[$rspfile]", rule->rspfile().Serialize());
-  EXPECT_EQ("[$in]", rule->rspfile_content().Serialize());
+  EXPECT_EQ("[cat ][$rspfile][ > ][$out]",
+            rule->GetBinding("command")->Serialize());
+  EXPECT_EQ("[$rspfile]", rule->GetBinding("rspfile")->Serialize());
+  EXPECT_EQ("[$in]", rule->GetBinding("rspfile_content")->Serialize());
 }
 
 TEST_F(ParserTest, InNewline) {
@@ -140,7 +143,8 @@ TEST_F(ParserTest, InNewline) {
   ASSERT_EQ(2u, state.rules_.size());
   const Rule* rule = state.rules_.begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
-  EXPECT_EQ("[cat ][$in_newline][ > ][$out]", rule->command().Serialize());
+  EXPECT_EQ("[cat ][$in_newline][ > ][$out]",
+            rule->GetBinding("command")->Serialize());
 
   Edge* edge = state.edges_[0];
   EXPECT_EQ("cat in\nin2 > out", edge->EvaluateCommand());
@@ -200,7 +204,7 @@ TEST_F(ParserTest, Continuation) {
   ASSERT_EQ(2u, state.rules_.size());
   const Rule* rule = state.rules_.begin()->second;
   EXPECT_EQ("link", rule->name());
-  EXPECT_EQ("[foo bar baz]", rule->command().Serialize());
+  EXPECT_EQ("[foo bar baz]", rule->GetBinding("command")->Serialize());
 }
 
 TEST_F(ParserTest, Backslash) {
diff --git a/src/state.cc b/src/state.cc
index bb0cc15..b5d2622 100644
--- a/src/state.cc
+++ b/src/state.cc
@@ -91,10 +91,10 @@ Pool* State::LookupPool(const string& pool_name) {
   return i->second;
 }
 
-Edge* State::AddEdge(const Rule* rule, Pool* pool) {
+Edge* State::AddEdge(const Rule* rule) {
   Edge* edge = new Edge();
   edge->rule_ = rule;
-  edge->pool_ = pool;
+  edge->pool_ = &State::kDefaultPool;
   edge->env_ = &bindings_;
   edges_.push_back(edge);
   return edge;
diff --git a/src/state.h b/src/state.h
index 918fe09..326fbac 100644
--- a/src/state.h
+++ b/src/state.h
@@ -92,7 +92,7 @@ struct State {
   void AddPool(Pool* pool);
   Pool* LookupPool(const string& pool_name);
 
-  Edge* AddEdge(const Rule* rule, Pool* pool);
+  Edge* AddEdge(const Rule* rule);
 
   Node* GetNode(StringPiece path);
   Node* LookupNode(StringPiece path);
diff --git a/src/state_test.cc b/src/state_test.cc
index 26177ff..af2bff1 100644
--- a/src/state_test.cc
+++ b/src/state_test.cc
@@ -29,10 +29,10 @@ TEST(State, Basic) {
   command.AddSpecial("out");
 
   Rule* rule = new Rule("cat");
-  rule->set_command(command);
+  rule->AddBinding("command", command);
   state.AddRule(rule);
 
-  Edge* edge = state.AddEdge(rule, &State::kDefaultPool);
+  Edge* edge = state.AddEdge(rule);
   state.AddIn(edge, "in1");
   state.AddIn(edge, "in2");
   state.AddOut(edge, "out");
-- 
cgit v0.12

