From f8f293730de2e12f6575c5d890a16504340f75fe Mon Sep 17 00:00:00 2001 From: Mohamed Bamakhrama Date: Mon, 2 Mar 2015 00:57:33 +0100 Subject: [PATCH] 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. --- src/clean.cc | 4 ++-- src/eval_env.cc | 51 +++++++++++++++++++++++++++++++++++++++++++++ src/eval_env.h | 28 +++++++++++++++++++++++++ src/graph.cc | 29 +++++--------------------- src/graph.h | 21 ------------------- src/manifest_parser.cc | 6 +++--- src/manifest_parser_test.cc | 28 +++++++++++-------------- src/state.cc | 14 +------------ src/state.h | 8 +------ src/state_test.cc | 2 +- 10 files changed, 104 insertions(+), 87 deletions(-) diff --git a/src/clean.cc b/src/clean.cc index 98c638c..7b044c5 100644 --- 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 --- 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 + #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::iterator i = rules_.find(rule_name); + if (i == rules_.end()) + return NULL; + return i->second; +} + +const Rule* BindingEnv::LookupRule(const string& rule_name) { + map::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::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 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 --- 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 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 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 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 bindings_; + map rules_; Env* parent_; }; diff --git a/src/graph.cc b/src/graph.cc index 6b977eb..cbf7921 100644 --- 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::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 --- 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 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 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 --- 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 --- 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 --- 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::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 --- 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::Type Paths; Paths paths_; - /// All the rules used in the graph. - map rules_; - /// All the pools used in the graph. map pools_; diff --git a/src/state_test.cc b/src/state_test.cc index bd133b6..458b519 100644 --- 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); -- 2.7.4