On 2015/08/20 15:47:57, wingo wrote:
https://codereview.chromium.org/1306583002/diff/60001/src/preparser.h
File src/preparser.h (right):


https://codereview.chromium.org/1306583002/diff/60001/src/preparser.h#newcode3604
src/preparser.h:3604: ArrowFormalParametersUnexpectedToken(classifier);
On 2015/08/20 15:22:57, caitp wrote:
> probably want to make sure destructuring still works properly, `({ x: x = > foo.bar }) => {}` for example. I don't expect `({ x = foo.bar }) => {}` to
work
> properly because CoverInitializedName parsing is pretty messed up in
> maybe-expressions, might be worth marking that as a TODO though in the test
case
>
> This also applies to other tokens which can legally occur in binding
patterns
> within arrow formals, I guess.
>
> If this is covered by other tests, then that's ok

Thanks for having a look!

I'm pretty sure that destructuring should still work properly, and the
reasoning
is a bit convoluted: an assignmentexpression is a valid arrow formal parameter
list if it is a bare identifier, or if it starts with a parenthesis, only
contains binding patterns separated by commas, possibly ending in a rest
pattern
(which, oddly, can't contain patterns).  OK.  That is what was already the
case
before and is covered by DestructuringPositiveTests and
DestructuringNegativeTests in test-parser.cc.

The way the above classifier works is that when parsing a parenthesized list
in
ParsePrimaryExpression, the expression as a whole remains a valid
ArrowFormalParameterList iff the inner productions are valid BindingPatterns. Specifically the invalid-ArrowFormalParameterList *does not* accumulate out
from
the individual arguments -- instead it's the BindingPattern flag that
propagates
out. See the gnarly special case in ExpressionClassifier::Accumulate. So I
don't think additional tests are necessary for existing destructuring foo.

What we missed is that the classifier would accept things as valid
ArrowFormalParameterLists that weren't valid. That's what this patch fixes. These additional flags won't cause problems for arrow function formals because
they are explicitly ignored there.

Cool. Since default parameters are implemented,a similar test case might be
useful there. It mIght end up working out the same way, it's just hard to see if
there's a test case like that in test-parsing by grepping, though.

https://codereview.chromium.org/1306583002/

--
--
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.

Reply via email to