Parser: Fix crash on stack overflow when lazy-parsing arrow functions

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 4bcdfdbda7e2233fd25a710caea8f23d9d6d0510..e8b2bc837bd29d165b7c2762cf6cb1b08bb296cc 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1145,8 +1145,19 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
       // from creating unresolved variables in already-resolved scopes.
       parsing_lazy_arrow_parameters_ = true;
       Expression* expression = ParseExpression(false, &ok);
-      DCHECK(expression->IsFunctionLiteral());
-      result = expression->AsFunctionLiteral();
+      if (ok) {
+        // Scanning must end at the same position that was recorded

[marja, 2015/03/19 11:00:26]

Hmm, this seems very indirect. Can't we know in a more direct way whether a
stack overflow has happened?

And why do we have ok == true in case of the stack overflow? Isn't that weird?
So we manage to parse a partial body successfully, but then... there should be
some part which reads the invalid token produced by the stack overflow and sets
ok to false. What's up with that?

[wingo, 2015/03/19 13:21:40]

I thought so too but to expand on what Adri found:

  - You go to parse an Assignment expression
  - You get the VariableProxy
  - You start to look ahead to see if the next token indicates a compound
expression, like = or + or something
  - The peeked token Token::ILLEGAL doesn't match any of the tokens that can
indicate a compoint expression
  - You return the VariableProxy instead, a valid expression, OK=true.

If this were a function(){} then we'd get an error expecting to consume the
right-bracket.  Since it's an arrow function, depending on where we are in
parsing it can succeed or not, as described in the CL message.

Agreed that it is very weird though!

+        // previously. If not, parsing has been interrupted due to a
+        // stack overflow, at which point the partially parsed arrow
+        // function concise body happens to be a valid expression.
+        if (scanner()->location().end_pos == shared_info->end_position()) {
+          DCHECK(expression->IsFunctionLiteral());

[wingo, 2015/03/19 07:48:45]

One thing I didn't get when first reading this code is that this DCHECK must be
true because the function was already pre-parsed, so we know what to expect. 
Perhaps a one-line comment here?

[aperez, 2015/03/19 12:42:30]

Acknowledged.

+          result = expression->AsFunctionLiteral();
+        } else {
+          result = NULL;
+          ok = false;
+        }
+      }
     } else if (shared_info->is_default_constructor()) {
       result = DefaultConstructor(IsSubclassConstructor(shared_info->kind()),
                                   scope, shared_info->start_position(),