include location of error in parse error messages in EvalEnv strings

E.g. when parsing "foo = ${bar", point at the correct location of
the missing curly brace.

6 files changed, 62 insertions, 8 deletions

diff --git a/src/eval_env.cc b/src/eval_env.cc
index 6b89b9c..8633f85 100644
--- a/src/eval_env.cc
+++ b/src/eval_env.cc
@@ -27,7 +27,7 @@ void BindingEnv::AddBinding(const string& key, const string& val) {
   bindings_[key] = val;
 }
 
-bool EvalString::Parse(const string& input, string* err) {
+bool EvalString::Parse(const string& input, string* err, size_t* err_index) {
   unparsed_ = input;
 
   string::size_type start, end;
@@ -49,6 +49,8 @@ bool EvalString::Parse(const string& input, string* err) {
       }
       if (end >= input.size()) {
         *err = "expected closing curly after ${";
+        if (err_index)
+          *err_index = end;
         return false;
       }
       parsed_.push_back(make_pair(input.substr(start, end - start), SPECIAL));
@@ -63,6 +65,8 @@ bool EvalString::Parse(const string& input, string* err) {
       }
       if (end == start) {
         *err = "expected variable after $";
+        if (err_index)
+          *err_index = start;
         return false;
       }
       parsed_.push_back(make_pair(input.substr(start, end - start), SPECIAL));
diff --git a/src/eval_env.h b/src/eval_env.h
index fd0e4eb..f0108ae 100644
--- a/src/eval_env.h
+++ b/src/eval_env.h
@@ -39,7 +39,7 @@ struct BindingEnv : public Env {
 // A tokenized string that contains variable references.
 // Can be evaluated relative to an Env.
 struct EvalString {
-  bool Parse(const string& input, string* err);
+  bool Parse(const string& input, string* err, size_t* err_index=NULL);
   string Evaluate(Env* env) const;
 
   const string& unparsed() const { return unparsed_; }
diff --git a/src/ninja_test.cc b/src/ninja_test.cc
index bd4546b..a4ec3a5 100644
--- a/src/ninja_test.cc
+++ b/src/ninja_test.cc
@@ -78,8 +78,18 @@ TEST(EvalString, OneVariableUpperCase) {
 TEST(EvalString, Error) {
   EvalString str;
   string err;
-  EXPECT_FALSE(str.Parse("bad $", &err));
+  size_t err_index;
+  EXPECT_FALSE(str.Parse("bad $", &err, &err_index));
   EXPECT_EQ("expected variable after $", err);
+  EXPECT_EQ(5, err_index);
+}
+TEST(EvalString, CurlyError) {
+  EvalString str;
+  string err;
+  size_t err_index;
+  EXPECT_FALSE(str.Parse("bad ${bar", &err, &err_index));
+  EXPECT_EQ("expected closing curly after ${", err);
+  EXPECT_EQ(9, err_index);
 }
 TEST(EvalString, Curlies) {
   EvalString str;
diff --git a/src/parsers.cc b/src/parsers.cc
index b9c1190..82b7111 100644
--- a/src/parsers.cc
+++ b/src/parsers.cc
@@ -52,7 +52,7 @@ void Tokenizer::Start(const char* start, const char* end) {
 
 bool Tokenizer::Error(const string& message, string* err) {
   char buf[1024];
-  sprintf(buf, "line %d, col %d: %s",
+  snprintf(buf, sizeof(buf), "line %d, col %d: %s",
           line_number_,
           (int)(token_.pos_ - cur_line_) + 1,
           message.c_str());
@@ -123,7 +123,7 @@ bool Tokenizer::ReadIdent(string* out) {
   return true;
 }
 
-bool Tokenizer::ReadToNewline(string* text, string* err) {
+bool Tokenizer::ReadToNewline(string *text, string* err, size_t max_length) {
   // XXX token_.clear();
   while (cur_ < end_ && *cur_ != '\n') {
     if (*cur_ == '\\') {
@@ -149,6 +149,10 @@ bool Tokenizer::ReadToNewline(string* text, string* err) {
       text->push_back(*cur_);
       ++cur_;
     }
+    if (text->size() >= max_length) {
+      token_.pos_ = cur_;
+      return false;
+    }
   }
   return Newline(err);
 }
@@ -367,6 +371,10 @@ bool ManifestParser::ParseLet(string* name, string* value, bool expand,
   if (!tokenizer_.ExpectToken(Token::EQUALS, err))
     return false;
 
+  // Backup the tokenizer state prior to consuming the line, for reporting
+  // the source location in case of a parse error later.
+  Tokenizer tokenizer_backup = tokenizer_;
+
   // XXX should we tokenize here?  it means we'll need to understand
   // command syntax, though...
   if (!tokenizer_.ReadToNewline(value, err))
@@ -375,8 +383,14 @@ bool ManifestParser::ParseLet(string* name, string* value, bool expand,
   if (expand) {
     EvalString eval;
     string eval_err;
-    if (!eval.Parse(*value, &eval_err))
-      return tokenizer_.Error(eval_err, err);
+    size_t err_index;
+    if (!eval.Parse(*value, &eval_err, &err_index)) {
+      string temp;
+      // Advance the saved tokenizer state up to the error index to report the
+      // error at the correct source location.
+      tokenizer_backup.ReadToNewline(&temp, err, err_index);
+      return tokenizer_backup.Error(eval_err, err);
+    }
     *value = eval.Evaluate(env_);
   }
 
diff --git a/src/parsers.h b/src/parsers.h
index 78865d1..055ba79 100644
--- a/src/parsers.h
+++ b/src/parsers.h
@@ -17,6 +17,7 @@
 
 #include <string>
 #include <vector>
+#include <limits>
 
 using namespace std;
 
@@ -67,7 +68,8 @@ struct Tokenizer {
   bool Newline(string* err);
   bool ExpectToken(Token::Type expected, string* err);
   bool ReadIdent(string* out);
-  bool ReadToNewline(string* text, string* err);
+  bool ReadToNewline(string* text, string* err,
+                     size_t max_length=std::numeric_limits<size_t>::max());
 
   Token::Type PeekToken();
   void ConsumeToken();
diff --git a/src/parsers_test.cc b/src/parsers_test.cc
index 73c1984..c20a812 100644
--- a/src/parsers_test.cc
+++ b/src/parsers_test.cc
@@ -208,6 +208,30 @@ TEST_F(ParserTest, Errors) {
     State state;
     ManifestParser parser(&state, NULL);
     string err;
+    EXPECT_FALSE(parser.Parse("x = $\n", &err));
+    EXPECT_EQ("line 1, col 6: expected variable after $", err);
+  }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
+    EXPECT_FALSE(parser.Parse("x = \\\n $[\n", &err));
+    EXPECT_EQ("line 2, col 3: expected variable after $", err);
+  }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
+    EXPECT_FALSE(parser.Parse("x = a\\\n b\\\n $\n", &err));
+    EXPECT_EQ("line 3, col 3: expected variable after $", err);
+  }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
     EXPECT_FALSE(parser.Parse("build x: y z\n", &err));
     EXPECT_EQ("line 1, col 10: unknown build rule 'y'", err);
   }