Fix memory leak of Rules, use bool phony_ member to identify phony edges

src/eval_env.cc

@@ -31,22 +31,22 @@ void BindingEnv::AddBinding(const string& key, const string& val) {
   bindings_[key] = val;
 }
 
-void BindingEnv::AddRule(const Rule* rule) {
+void BindingEnv::AddRule(std::unique_ptr<const Rule> rule) {
   assert(LookupRuleCurrentScope(rule->name()) == NULL);
-  rules_[rule->name()] = rule;
+  rules_[rule->name()] = std::move(rule);
 }
 
 const Rule* BindingEnv::LookupRuleCurrentScope(const string& rule_name) {
-  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+  auto i = rules_.find(rule_name);
   if (i == rules_.end())
     return NULL;
-  return i->second;
+  return i->second.get();
 }
 
 const Rule* BindingEnv::LookupRule(const string& rule_name) {
-  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+  auto i = rules_.find(rule_name);
   if (i != rules_.end())
-    return i->second;
+    return i->second.get();
   if (parent_)
     return parent_->LookupRule(rule_name);
   return NULL;
@@ -63,6 +63,16 @@ const EvalString* Rule::GetBinding(const string& key) const {
   return &i->second;
 }
 
+std::unique_ptr<Rule> Rule::Phony() {
+  auto rule = std::unique_ptr<Rule>(new Rule("phony"));
+  rule->phony_ = true;
+  return rule;
+}
+
+bool Rule::IsPhony() const {
+  return phony_;
+}
+
 // static
 bool Rule::IsReservedBinding(const string& var) {
   return var == "command" ||
@@ -78,7 +88,7 @@ bool Rule::IsReservedBinding(const string& var) {
       var == "msvc_deps_prefix";
 }
 
-const map<string, const Rule*>& BindingEnv::GetRules() const {
+const map<string, std::unique_ptr<const Rule>>& BindingEnv::GetRules() const {
   return rules_;
 }
 

src/eval_env.h

@@ -16,6 +16,7 @@
 #define NINJA_EVAL_ENV_H_
 
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -59,6 +60,10 @@ struct EvalString {
 struct Rule {
   explicit Rule(const std::string& name) : name_(name) {}
 
+  static std::unique_ptr<Rule> Phony();
+
+  bool IsPhony() const;
+
   const std::string& name() const { return name_; }
 
   void AddBinding(const std::string& key, const EvalString& val);
@@ -74,6 +79,7 @@ struct Rule {
   std::string name_;
   typedef std::map<std::string, EvalString> Bindings;
   Bindings bindings_;
+  bool phony_ = false;
 };
 
 /// An Env which contains a mapping of variables to values
@@ -85,10 +91,10 @@ struct BindingEnv : public Env {
   virtual ~BindingEnv() {}
   virtual std::string LookupVariable(const std::string& var);
 
-  void AddRule(const Rule* rule);
+  void AddRule(std::unique_ptr<const Rule> rule);
   const Rule* LookupRule(const std::string& rule_name);
   const Rule* LookupRuleCurrentScope(const std::string& rule_name);
-  const std::map<std::string, const Rule*>& GetRules() const;
+  const std::map<std::string, std::unique_ptr<const Rule>>& GetRules() const;
 
   void AddBinding(const std::string& key, const std::string& val);
 
@@ -102,7 +108,7 @@ struct BindingEnv : public Env {
 
 private:
   std::map<std::string, std::string> bindings_;
-  std::map<std::string, const Rule*> rules_;
+  std::map<std::string, std::unique_ptr<const Rule>> rules_;
   BindingEnv* parent_;
 };
 

src/graph.cc

@@ -559,7 +559,7 @@ void Edge::Dump(const char* prefix) const {
 }
 
 bool Edge::is_phony() const {
-  return rule_ == &State::kPhonyRule;
+  return rule_->IsPhony();
 }
 
 bool Edge::use_console() const {

src/manifest_parser.cc

@@ -18,6 +18,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <memory>
 #include <vector>
 
 #include "graph.h"
@@ -143,7 +144,7 @@ bool ManifestParser::ParseRule(string* err) {
   if (env_->LookupRuleCurrentScope(name) != NULL)
     return lexer_.Error("duplicate rule '" + name + "'", err);
 
-  Rule* rule = new Rule(name);  // XXX scoped_ptr
+  auto rule = std::unique_ptr<Rule>(new Rule(name));
 
   while (lexer_.PeekToken(Lexer::INDENT)) {
     string key;
@@ -169,7 +170,7 @@ bool ManifestParser::ParseRule(string* err) {
   if (rule->bindings_["command"].empty())
     return lexer_.Error("expected 'command =' line", err);
 
-  env_->AddRule(rule);
+  env_->AddRule(std::move(rule));
   return true;
 }
 

src/manifest_parser_test.cc

@@ -51,7 +51,7 @@ TEST_F(ParserTest, Rules) {
 "build result: cat in_1.cc in-2.O\n"));
 
   ASSERT_EQ(3u, state.bindings_.GetRules().size());
-  const Rule* rule = state.bindings_.GetRules().begin()->second;
+  const auto& rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat", rule->name());
   EXPECT_EQ("[cat ][$in][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -84,7 +84,7 @@ TEST_F(ParserTest, IgnoreIndentedComments) {
 "  #comment\n"));
 
   ASSERT_EQ(2u, state.bindings_.GetRules().size());
-  const Rule* rule = state.bindings_.GetRules().begin()->second;
+  const auto& rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat", rule->name());
   Edge* edge = state.GetNode("result", 0)->in_edge();
   EXPECT_TRUE(edge->GetBindingBool("restat"));
@@ -117,7 +117,7 @@ TEST_F(ParserTest, ResponseFiles) {
 "  rspfile=out.rsp\n"));
 
   ASSERT_EQ(2u, state.bindings_.GetRules().size());
-  const Rule* rule = state.bindings_.GetRules().begin()->second;
+  const auto& rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
   EXPECT_EQ("[cat ][$rspfile][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -134,7 +134,7 @@ TEST_F(ParserTest, InNewline) {
 "  rspfile=out.rsp\n"));
 
   ASSERT_EQ(2u, state.bindings_.GetRules().size());
-  const Rule* rule = state.bindings_.GetRules().begin()->second;
+  const auto& rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("cat_rsp", rule->name());
   EXPECT_EQ("[cat ][$in_newline][ > ][$out]",
             rule->GetBinding("command")->Serialize());
@@ -195,7 +195,7 @@ TEST_F(ParserTest, Continuation) {
 " d e f\n"));
 
   ASSERT_EQ(2u, state.bindings_.GetRules().size());
-  const Rule* rule = state.bindings_.GetRules().begin()->second;
+  const auto& rule = state.bindings_.GetRules().begin()->second;
   EXPECT_EQ("link", rule->name());
   EXPECT_EQ("[foo bar baz]", rule->GetBinding("command")->Serialize());
 }

src/ninja.cc

@@ -674,12 +674,12 @@ int NinjaMain::ToolRules(const Options* options, int argc, char* argv[]) {
 
   // Print rules
 
-  typedef map<string, const Rule*> Rules;
+  typedef map<string, std::unique_ptr<const Rule>> Rules;
   const Rules& rules = state_.bindings_.GetRules();
   for (Rules::const_iterator i = rules.begin(); i != rules.end(); ++i) {
     printf("%s", i->first.c_str());
     if (print_description) {
-      const Rule* rule = i->second;
+      const Rule* rule = i->second.get();
       const EvalString* description = rule->GetBinding("description");
       if (description != NULL) {
         printf(": %s", description->Unparse().c_str());

src/state.cc

@@ -63,10 +63,9 @@ void Pool::Dump() const {
 
 Pool State::kDefaultPool("", 0);
 Pool State::kConsolePool("console", 1);
-const Rule State::kPhonyRule("phony");
 
 State::State() {
-  bindings_.AddRule(&kPhonyRule);
+  bindings_.AddRule(Rule::Phony());
   AddPool(&kDefaultPool);
   AddPool(&kConsolePool);
 }

src/state.h

@@ -95,7 +95,6 @@ struct Pool {
 struct State {
   static Pool kDefaultPool;
   static Pool kConsolePool;
-  static const Rule kPhonyRule;
 
   State();
 

src/state_test.cc

@@ -31,7 +31,7 @@ TEST(State, Basic) {
 
   Rule* rule = new Rule("cat");
   rule->AddBinding("command", command);
-  state.bindings_.AddRule(rule);
+  state.bindings_.AddRule(std::unique_ptr<Rule>(rule));
 
   Edge* edge = state.AddEdge(rule);
   state.AddIn(edge, "in1", 0);