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(
On 2014/04/24 08:08:06, marja wrote:
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.
The example you mention would be a comma-expression with two items, and
it
is a valid expression, equivalent to:
foo, (bar => { baz })
(foo, bar => { baz })
(foo, (bar => { baz }))
So the case you mention is valid syntax, and it is already handled by
the
grammar because comma has more precedence than expressions, and an arrow
function is an expression (the whole arrrow function, including the
parameter list).
There is no need to check for those cases here, because the input up
to the comma has already been parsed as a comma-expression when the
ParseArrowFunction() function is called.
https://codereview.chromium.org/160073006/diff/440001/src/parser.cc#newcode3220
src/parser.cc:3220: goto invalid_token;
On 2014/04/24 08:08:06, marja wrote:
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.
Sure thing, I will do that. The function will not be as terse, though.
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 {
On 2014/04/24 08:08:06, marja wrote:
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...
Fortunately I have this locally as a separate commit, I will submit a
new CL
which includes only the bits that move PreParserStatement around.
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode939
src/preparser.h:939: FunctionLiteral::IsGeneratorFlag is_generator,
On 2014/04/24 08:08:06, marja wrote:
Instead of IsGeneratorFlag and IsArrowFlag you should have only one
flag, which
takes values "generator", "arrow" and "normal".
Okay.
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode1188
src/preparser.h:1188: return Vector<const PreParserIdentifier>::empty();
On 2014/04/24 08:08:06, marja wrote:
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.
That is actually a neat idea, I will give it a try. Thank you for the
suggestion.
https://codereview.chromium.org/160073006/diff/440001/src/preparser.h#newcode2385
src/preparser.h:2385: if (peek() == Token::LPAREN) {
On 2014/04/24 08:08:06, marja wrote:
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...
Correct. In that case we know exactly where the parameter list starts,
so there
is no need to deduce the parameter names from the AST. Using the AST to
pick the
parameter names could be done in both cases, but parsing directly for
now provides
better detection and reporting of syntax errors.
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.
Sure, will do.
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.
