From adaa91a33eb2cf23b88333b5cc2e2e456a5f1803 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Wed, 10 Jul 2013 17:30:16 -0700
Subject: Reuse ManifestParser::Load() in ManifestParser::ParseFileInclude().

ParseFileInclude() was doing the work that Load() was doing. Instead, just
make it call Load().

Also, remove a FIXME about using ReadPath() in ParseFileInclude() -- it's
already being used. (The FIXME was added in the same commit that added the
call to ReadPath() -- 8a0c96075786c19 -- it probably should've been deleted
before that commit.)

Also, remove a `contents.resize(contents.size() + 10);`. It's not clear what
it's for, but if it was important then ManifestParser::ParseFileInclude()
would have needed that call too, and it didn't have it.

No intended behavior change.
---
 src/manifest_parser.cc | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index d4f0007..88c16ab 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -28,6 +28,7 @@ ManifestParser::ManifestParser(State* state, FileReader* file_reader)
   : state_(state), file_reader_(file_reader) {
   env_ = &state->bindings_;
 }
+
 bool ManifestParser::Load(const string& filename, string* err) {
   string contents;
   string read_err;
@@ -35,7 +36,6 @@ bool ManifestParser::Load(const string& filename, string* err) {
     *err = "loading '" + filename + "': " + read_err;
     return false;
   }
-  contents.resize(contents.size() + 10);
   return Parse(filename, contents, err);
 }
 
@@ -338,17 +338,11 @@ bool ManifestParser::ParseEdge(string* err) {
 }
 
 bool ManifestParser::ParseFileInclude(bool new_scope, string* err) {
-  // XXX this should use ReadPath!
   EvalString eval;
   if (!lexer_.ReadPath(&eval, err))
     return false;
   string path = eval.Evaluate(env_);
 
-  string contents;
-  string read_err;
-  if (!file_reader_->ReadFile(path, &contents, &read_err))
-    return lexer_.Error("loading '" + path + "': " + read_err, err);
-
   ManifestParser subparser(state_, file_reader_);
   if (new_scope) {
     subparser.env_ = new BindingEnv(env_);
@@ -356,8 +350,8 @@ bool ManifestParser::ParseFileInclude(bool new_scope, string* err) {
     subparser.env_ = env_;
   }
 
-  if (!subparser.Parse(path, contents, err))
-    return false;
+  if (!subparser.Load(path, err))
+    return lexer_.Error(string(*err), err);
 
   if (!ExpectToken(Lexer::NEWLINE, err))
     return false;
-- 
cgit v0.12


From 4c7802baf8ddead0cbfddefb1f9fa3587c6dd089 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Wed, 10 Jul 2013 17:43:29 -0700
Subject: Let the ".ninja parse" metric include the time to read the toplevel
 ninja file.

Loads of included ninja files were covered by the ".ninja parse" in Parse()
further up the stack from the toplevel file, but the load of the toplevel
file wasn't counted. Fix that.
---
 src/manifest_parser.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 88c16ab..f5e7eac 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -30,6 +30,7 @@ ManifestParser::ManifestParser(State* state, FileReader* file_reader)
 }
 
 bool ManifestParser::Load(const string& filename, string* err) {
+  METRIC_RECORD(".ninja parse");
   string contents;
   string read_err;
   if (!file_reader_->ReadFile(filename, &contents, &read_err)) {
@@ -41,7 +42,6 @@ bool ManifestParser::Load(const string& filename, string* err) {
 
 bool ManifestParser::Parse(const string& filename, const string& input,
                            string* err) {
-  METRIC_RECORD(".ninja parse");
   lexer_.Start(filename, input);
 
   for (;;) {
-- 
cgit v0.12


From cce4807703a0595fd8db8c57db61184b0d00a187 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Wed, 17 Jul 2013 17:55:36 -0700
Subject: Add back contents.resize(), but with a comment and just 1 instead of
 10.

---
 src/manifest_parser.cc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index f5e7eac..a1bb904 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -37,6 +37,14 @@ bool ManifestParser::Load(const string& filename, string* err) {
     *err = "loading '" + filename + "': " + read_err;
     return false;
   }
+
+  // The lexer needs a nul byte at the end of its input, to know when it's done.
+  // It takes a StringPiece, and StringPiece's string constructor uses
+  // string::data().  data()'s return value isn't guaranteed to be
+  // null-terminated (although in practice - libc++, libstdc++, msvc's stl --
+  // it is, and C++11 demands that too), so add an explicit nul byte.
+  contents.resize(contents.size() + 1);
+
   return Parse(filename, contents, err);
 }
 
-- 
cgit v0.12