In addition to the comments below, this is lacking tests. You should be
able to
treat arrow functions like ordinary functions for now, such that you can
already
write JS tests that check correct parsing.
Example tests I'd like to see include
() => 0
x => x
x => y => x + y
(x, y) => (u, v) => x*u + y*v
(x, y) => x.a = y
var f = x => x, y // y is undefined
(x, y) => x, y // fails with y unbound
=> 0 // fails
) => 0 // fails
(()) => 0 // fails
((x)) => x // fails
((x, y)) => x + y // fails
); // fails
Ideally, write tests that use eval to programmatically generate and check
many
different combinations of these, in different syntactic contexts.
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#newcode1013
src/parser.cc:1013: FunctionParsingMode parsing_mode =
shared_info->is_generator()
Nit: I'd prefer formatting this like
FunctionParsingMode parsing_mode =
shared_info->is_generator() ? kGeneratorFunction :
shared_info->is_arrow() ? kArrowFunction : kNormalFunction;
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3139
src/parser.cc:3139: Vector<VariableProxy*>
Parser::ParameterListFromExpression(
How does this rule out illegal parenthesis? For example, ((x)) or ((x,
y)) would seem to (incorrectly) parse as the parameter lists (x) and (x,
y), wouldn't it?
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3151
src/parser.cc:3151: ParserTraits::ReportMessageAt(Scanner::Location(pos,
pos),
Can we refactor this a little to avoid all the error reporting code
duplication?
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3312
src/parser.cc:3312: // FormalParameterList ::
On 2014/03/24 09:04:06, marja wrote:
This ParseFunctionLiteral is pretty monstrous as is, can it be split
first
before adding another dimension for array funcs?
+1. The arrow function case is parsing quite different syntax, so I'd
rather have it as a separate function. You can still factor out
commonalities into third functions (e.g. ParseFunctionParameterList,
ParseFunctionBody, etc.).
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3315
src/parser.cc:3315: if (func_parsing_mode == kArrowFunction && peek() ==
Token::ARROW) {
Can it ever happen that the first condition is true but the second
isn't? Where do you invoke ParseFunctionLiteral with kArrowFunction but
have not parsed the parameters yet?
https://codereview.chromium.org/160073006/diff/380001/src/parser.cc#newcode3400
src/parser.cc:3400: Expression* expression = ParseExpression(true,
CHECK_OK);
Shouldn't this be ParseAssignmentExpression?
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.
