Allow scoping rules through subninja

Ninja didn't support scoping rules through subninja and assumed
a unique rule name in the whole namespace. With this change, this
behavior is changed to allow scoping rules. Two rules can have the
same name if they belong to two different scopes. However, two
rules can NOT have the same name in the same scope.

diff --git a/src/clean.cc b/src/clean.cc
index 98c638c..7b044c5 100644 (file)
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -220,7 +220,7 @@ int Cleaner::CleanRule(const char* rule) {
   assert(rule);
 
   Reset();
-  const Rule* r = state_->LookupRule(rule);
+  const Rule* r = state_->bindings_.LookupRule(rule);
   if (r) {
     CleanRule(r);
   } else {
@@ -237,7 +237,7 @@ int Cleaner::CleanRules(int rule_count, char* rules[]) {
   PrintHeader();
   for (int i = 0; i < rule_count; ++i) {
     const char* rule_name = rules[i];
-    const Rule* rule = state_->LookupRule(rule_name);
+    const Rule* rule = state_->bindings_.LookupRule(rule_name);
     if (rule) {
       if (IsVerbose())
         printf("Rule %s\n", rule_name);

diff --git a/src/eval_env.cc b/src/eval_env.cc
index 834b7e1..8f5c8ee 100644 (file)
--- a/src/eval_env.cc
+++ b/src/eval_env.cc
@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <assert.h>
+
 #include "eval_env.h"
 
 string BindingEnv::LookupVariable(const string& var) {
@@ -27,6 +29,55 @@ void BindingEnv::AddBinding(const string& key, const string& val) {
   bindings_[key] = val;
 }
 
+void BindingEnv::AddRule(const Rule* rule) {
+  assert(LookupRule(rule->name()) == NULL);
+  rules_[rule->name()] = rule;
+}
+
+const Rule* BindingEnv::LookupRuleCurrentScope(const string& rule_name) {
+  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+  if (i == rules_.end())
+    return NULL;
+  return i->second;
+}
+
+const Rule* BindingEnv::LookupRule(const string& rule_name) {
+  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+  if (i != rules_.end())
+    return i->second;
+  if (parent_)
+    return parent_->LookupRule(rule_name);
+  return NULL;
+}
+
+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 == "deps" ||
+      var == "generator" ||
+      var == "pool" ||
+      var == "restat" ||
+      var == "rspfile" ||
+      var == "rspfile_content";
+}
+
+const map<string, const Rule*> BindingEnv::GetRules() const {
+  return rules_;
+}
+
 string BindingEnv::LookupWithFallback(const string& var,
                                       const EvalString* eval,
                                       Env* env) {

diff --git a/src/eval_env.h b/src/eval_env.h
index f3c959a..46ea131 100644 (file)
--- a/src/eval_env.h
+++ b/src/eval_env.h
@@ -24,10 +24,32 @@ using namespace std;
 
 struct EvalString;
 
+/// An invokable build command and associated metadata (description, etc.).
+struct Rule {
+  explicit Rule(const string& name) : name_(name) {}
+
+  const string& name() const { return name_; }
+
+  typedef map<string, EvalString> Bindings;
+  void AddBinding(const string& key, const EvalString& val);
+
+  static bool IsReservedBinding(const string& var);
+
+  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_;
+  map<string, EvalString> bindings_;
+};
+
 /// An interface for a scope for variable (e.g. "$foo") lookups.
 struct Env {
   virtual ~Env() {}
   virtual string LookupVariable(const string& var) = 0;
+  virtual const Rule* LookupRule(const string& rule_name) = 0;
 };
 
 /// An Env which contains a mapping of variables to values
@@ -39,6 +61,11 @@ struct BindingEnv : public Env {
   virtual ~BindingEnv() {}
   virtual string LookupVariable(const string& var);
 
+  void AddRule(const Rule* rule);
+  const Rule* LookupRule(const string& rule_name);
+  const Rule* LookupRuleCurrentScope(const string& rule_name);
+  const map<string, const Rule*> GetRules() const;
+
   void AddBinding(const string& key, const string& val);
 
   /// This is tricky.  Edges want lookup scope to go in this order:
@@ -51,6 +78,7 @@ struct BindingEnv : public Env {
 
 private:
   map<string, string> bindings_;
+  map<string, const Rule*> rules_;
   Env* parent_;
 };
 

diff --git a/src/graph.cc b/src/graph.cc
index 6b977eb..cbf7921 100644 (file)
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -33,30 +33,6 @@ 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 == "deps" ||
-      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;
@@ -231,6 +207,7 @@ struct EdgeEnv : public Env {
   EdgeEnv(Edge* edge, EscapeKind escape)
       : edge_(edge), escape_in_out_(escape) {}
   virtual string LookupVariable(const string& var);
+  virtual const Rule* LookupRule(const string& rule_name);
 
   /// Given a span of Nodes, construct a list of paths suitable for a command
   /// line.
@@ -242,6 +219,10 @@ struct EdgeEnv : public Env {
   EscapeKind escape_in_out_;
 };
 
+const Rule* EdgeEnv::LookupRule(const string& rule_name) {
+  return NULL;
+}
+
 string EdgeEnv::LookupVariable(const string& var) {
   if (var == "in" || var == "in_newline") {
     int explicit_deps_count = edge_->inputs_.size() - edge_->implicit_deps_ -

diff --git a/src/graph.h b/src/graph.h
index 9aada38..9eafc05 100644 (file)
--- a/src/graph.h
+++ b/src/graph.h
@@ -121,27 +121,6 @@ private:
   int id_;
 };
 
-/// An invokable build command and associated metadata (description, etc.).
-struct Rule {
-  explicit Rule(const string& name) : name_(name) {}
-
-  const string& name() const { return name_; }
-
-  typedef map<string, EvalString> Bindings;
-  void AddBinding(const string& key, const EvalString& val);
-
-  static bool IsReservedBinding(const string& var);
-
-  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_;
-  map<string, EvalString> bindings_;
-};
-
 /// An edge in the dependency graph; links between Nodes using Rules.
 struct Edge {
   Edge() : rule_(NULL), env_(NULL), outputs_ready_(false), deps_missing_(false),

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 7ee990b..044b259 100644 (file)
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -156,7 +156,7 @@ bool ManifestParser::ParseRule(string* err) {
   if (!ExpectToken(Lexer::NEWLINE, err))
     return false;
 
-  if (state_->LookupRule(name) != NULL)
+  if (env_->LookupRuleCurrentScope(name) != NULL)
     return lexer_.Error("duplicate rule '" + name + "'", err);
 
   Rule* rule = new Rule(name);  // XXX scoped_ptr
@@ -185,7 +185,7 @@ bool ManifestParser::ParseRule(string* err) {
   if (rule->bindings_["command"].empty())
     return lexer_.Error("expected 'command =' line", err);
 
-  state_->AddRule(rule);
+  env_->AddRule(rule);
   return true;
 }
 
@@ -252,7 +252,7 @@ bool ManifestParser::ParseEdge(string* err) {
   if (!lexer_.ReadIdent(&rule_name))
     return lexer_.Error("expected build command name", err);
 
-  const Rule* rule = state_->LookupRule(rule_name);
+  const Rule* rule = env_->LookupRule(rule_name);
   if (!rule)
     return lexer_.Error("unknown build rule '" + rule_name + "'", err);
 

diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc
index a8f2e53..301a35c 100644 (file)
--- a/src/manifest_parser_test.cc
+++ b/src/manifest_parser_test.cc
@@ -60,8 +60,8 @@ TEST_F(ParserTest, Rules) {
 "\n"
 "build result: cat in_1.cc in-2.O\n"));
 
-  ASSERT_EQ(3u, state.rules_.size());
-  const Rule* rule = state.rules_.begin()->second;
+  ASSERT_EQ(3u, state.bindings_.GetRules().size());
+  const Rule* rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat", rule->name());
   EXPECT_EQ("[cat ][$in][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -93,8 +93,8 @@ TEST_F(ParserTest, IgnoreIndentedComments) {
 "build result: cat in_1.cc in-2.O\n"
 "  #comment\n"));
 
-  ASSERT_EQ(2u, state.rules_.size());
-  const Rule* rule = state.rules_.begin()->second;
+  ASSERT_EQ(2u, state.bindings_.GetRules().size());
+  const Rule* rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat", rule->name());
   Edge* edge = state.GetNode("result", 0)->in_edge();
   EXPECT_TRUE(edge->GetBindingBool("restat"));
@@ -126,8 +126,8 @@ TEST_F(ParserTest, ResponseFiles) {
 "build out: cat_rsp in\n"
 "  rspfile=out.rsp\n"));
 
-  ASSERT_EQ(2u, state.rules_.size());
-  const Rule* rule = state.rules_.begin()->second;
+  ASSERT_EQ(2u, state.bindings_.GetRules().size());
+  const Rule* rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
   EXPECT_EQ("[cat ][$rspfile][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -143,8 +143,8 @@ TEST_F(ParserTest, InNewline) {
 "build out: cat_rsp in in2\n"
 "  rspfile=out.rsp\n"));
 
-  ASSERT_EQ(2u, state.rules_.size());
-  const Rule* rule = state.rules_.begin()->second;
+  ASSERT_EQ(2u, state.bindings_.GetRules().size());
+  const Rule* rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
   EXPECT_EQ("[cat ][$in_newline][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -204,8 +204,8 @@ TEST_F(ParserTest, Continuation) {
 "build a: link c $\n"
 " d e f\n"));
 
-  ASSERT_EQ(2u, state.rules_.size());
-  const Rule* rule = state.rules_.begin()->second;
+  ASSERT_EQ(2u, state.bindings_.GetRules().size());
+  const Rule* rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("link", rule->name());
   EXPECT_EQ("[foo bar baz]", rule->GetBinding("command")->Serialize());
 }
@@ -823,18 +823,14 @@ TEST_F(ParserTest, MissingSubNinja) {
 }
 
 TEST_F(ParserTest, DuplicateRuleInDifferentSubninjas) {
-  // Test that rules live in a global namespace and aren't scoped to subninjas.
+  // Test that rules are scoped to subninjas.
   files_["test.ninja"] = "rule cat\n"
                          "  command = cat\n";
   ManifestParser parser(&state, this);
   string err;
-  EXPECT_FALSE(parser.ParseTest("rule cat\n"
+  EXPECT_TRUE(parser.ParseTest("rule cat\n"
                                 "  command = cat\n"
                                 "subninja test.ninja\n", &err));
-  EXPECT_EQ("test.ninja:1: duplicate rule 'cat'\n"
-            "rule cat\n"
-            "        ^ near here"
-            , err);
 }
 
 TEST_F(ParserTest, Include) {

diff --git a/src/state.cc b/src/state.cc
index 1ceda45..89f90c0 100644 (file)
--- a/src/state.cc
+++ b/src/state.cc
@@ -73,23 +73,11 @@ Pool State::kConsolePool("console", 1);
 const Rule State::kPhonyRule("phony");
 
 State::State() {
-  AddRule(&kPhonyRule);
+  bindings_.AddRule(&kPhonyRule);
   AddPool(&kDefaultPool);
   AddPool(&kConsolePool);
 }
 
-void State::AddRule(const Rule* rule) {
-  assert(LookupRule(rule->name()) == NULL);
-  rules_[rule->name()] = rule;
-}
-
-const Rule* State::LookupRule(const string& rule_name) {
-  map<string, const Rule*>::iterator i = rules_.find(rule_name);
-  if (i == rules_.end())
-    return NULL;
-  return i->second;
-}
-
 void State::AddPool(Pool* pool) {
   assert(LookupPool(pool->name()) == NULL);
   pools_[pool->name()] = pool;

diff --git a/src/state.h b/src/state.h
index 311d740..db0e3aa 100644 (file)
--- a/src/state.h
+++ b/src/state.h
@@ -79,7 +79,7 @@ struct Pool {
   DelayedEdges delayed_;
 };
 
-/// Global state (file status, loaded rules) for a single run.
+/// Global state (file status) for a single run.
 struct State {
   static Pool kDefaultPool;
   static Pool kConsolePool;
@@ -87,9 +87,6 @@ struct State {
 
   State();
 
-  void AddRule(const Rule* rule);
-  const Rule* LookupRule(const string& rule_name);
-
   void AddPool(Pool* pool);
   Pool* LookupPool(const string& pool_name);
 
@@ -119,9 +116,6 @@ struct State {
   typedef ExternalStringHashMap<Node*>::Type Paths;
   Paths paths_;
 
-  /// All the rules used in the graph.
-  map<string, const Rule*> rules_;
-
   /// All the pools used in the graph.
   map<string, Pool*> pools_;
 

diff --git a/src/state_test.cc b/src/state_test.cc
index bd133b6..458b519 100644 (file)
--- a/src/state_test.cc
+++ b/src/state_test.cc
@@ -29,7 +29,7 @@ TEST(State, Basic) {
 
   Rule* rule = new Rule("cat");
   rule->AddBinding("command", command);
-  state.AddRule(rule);
+  state.bindings_.AddRule(rule);
 
   Edge* edge = state.AddEdge(rule);
   state.AddIn(edge, "in1", 0);