Here are some comments (but I didn't yet look at all the code in detail).
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3141
src/parser.cc:3141: if (expression == NULL)
Normally having expression == NULL would be an error case; pls add a
comment here that this can actually happen in non-error cases. (Afaics
it happens when the argument list is empty.)
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3312
src/parser.cc:3312: // FormalParameterList ::
This ParseFunctionLiteral is pretty monstrous as is, can it be split
first before adding another dimension for array funcs?
https://codereview.chromium.org/160073006/diff/380001/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/160073006/diff/380001/src/preparser.cc#newcode844
src/preparser.cc:844: PreParser::Expression
PreParser::ParseFunctionLiteral(
... this doesn't handle arrow funcs yet? (Like, single statement body
and so on.)
You should add tests to cctest/test-parsing to check that the parser and
preparser accept the same set of arrow functions.
https://codereview.chromium.org/160073006/diff/380001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/160073006/diff/380001/src/preparser.h#newcode1464
src/preparser.h:1464: if (allow_arrow_functions() && peek() ==
Token::RPAREN) {
This doesn't sound right. I guess the idea is that we see the (, then
parse the expression inside the () with ParseExpression. But what if we
see a lonely ) when we expect an expression? That should be an error,
but this looks like it'll accept it.
https://codereview.chromium.org/160073006/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
