diff options
author | Nico Weber <nicolasweber@gmx.de> | 2015-03-09 04:44:41 (GMT) |
---|---|---|
committer | Nico Weber <nicolasweber@gmx.de> | 2015-03-09 04:44:41 (GMT) |
commit | b0c9eab3f67547eb0f6c24721fe002f414505e73 (patch) | |
tree | ea3f5c04a604e8bb8ce0f779d36c5d3dec1e9581 /src | |
parent | fbfa6921ca08bd96617745aa3da2cbf7bd68fad9 (diff) | |
parent | 2d645be0f0ffb1ad257f72ae89182e1d153ca617 (diff) | |
download | Ninja-b0c9eab3f67547eb0f6c24721fe002f414505e73.zip Ninja-b0c9eab3f67547eb0f6c24721fe002f414505e73.tar.gz Ninja-b0c9eab3f67547eb0f6c24721fe002f414505e73.tar.bz2 |
Merge pull request #921 from mohamed/master
Allow scoping rules through subninja
Diffstat (limited to 'src')
-rw-r--r-- | src/clean.cc | 4 | ||||
-rw-r--r-- | src/eval_env.cc | 51 | ||||
-rw-r--r-- | src/eval_env.h | 28 | ||||
-rw-r--r-- | src/graph.cc | 29 | ||||
-rw-r--r-- | src/graph.h | 21 | ||||
-rw-r--r-- | src/manifest_parser.cc | 6 | ||||
-rw-r--r-- | src/manifest_parser_test.cc | 41 | ||||
-rw-r--r-- | src/state.cc | 14 | ||||
-rw-r--r-- | src/state.h | 8 | ||||
-rw-r--r-- | src/state_test.cc | 2 |
10 files changed, 117 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 <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 --- 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 --- 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 --- 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 --- 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..e13d728 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,27 @@ 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, DuplicateRuleInDifferentSubninjasWithInclude) { + // Test that rules are scoped to subninjas even with includes. + files_["rules.ninja"] = "rule cat\n" + " command = cat\n"; + files_["test.ninja"] = "include rules.ninja\n" + "build x : cat\n"; + ManifestParser parser(&state, this); + string err; + EXPECT_TRUE(parser.ParseTest("include rules.ninja\n" + "subninja test.ninja\n" + "build y : cat\n", &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<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 --- 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 --- 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); |