Thanks for working on this!
Some questions which popped in my mind while reading through this (but I
didn't
do much searching to find the answers):
1) Why do Identifiers now need to know their positions?
2) Why is the added FunctionState constructor needed?
https://codereview.chromium.org/160073006/diff/440001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/160073006/diff/440001/src/parser.cc#newcode3202
src/parser.cc:3202: Vector<VariableProxy*>
ParserTraits::ParameterListFromExpression(
It looks like this doesn't check that the expression is parenthesized,
so this would accept foo, bar => { baz }. You should add tests for such
cases in the style of test-parsing.cc.
https://codereview.chromium.org/160073006/diff/440001/src/parser.cc#newcode3220
src/parser.cc:3220: goto invalid_token;
Afaics, it would be possible to write this in a goto-less way; just set
some error condition and break from the while loop, after the loop,
check the error conditions.
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode736
src/preparser.h:736: class PreParserStatement {
I'd be more than willing to take this kind of changes as a separate CL.
A bit unfortunate that this will be the first unified statement parsing
function...
I wonder if this CL would get easier if I unify some easy statement
parsing funcs (like ParseIfStatement), so that the machinery for unified
statement parsing would be there already...
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode939
src/preparser.h:939: FunctionLiteral::IsGeneratorFlag is_generator,
Instead of IsGeneratorFlag and IsArrowFlag you should have only one
flag, which takes values "generator", "arrow" and "normal".
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode1188
src/preparser.h:1188: return Vector<const PreParserIdentifier>::empty();
This makes PreParser accept a different language than Parser (well, it
already does, but I don't want to make them diverge any further).
PreParserExpression could maintain a "is this a valid parenthesized
comma-separated identifier list" property which can be used here for
checking the validity.
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode2385
src/preparser.h:2385: if (peek() == Token::LPAREN) {
Hmm, so this function can be called in two ways, first by
ParseAssignmentExpression, where the next token is always arrow, and
then by Parser::ParseLazy where it's the beginning of the param list...
Maybe you could split this up into two different functions. The last
part, parsing the body after the =>, can be a third function which the
two then call.
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.
