On 2015/08/31 13:43:34, conradw wrote:
On 2015/08/31 13:13:30, wingo wrote:
> On 2015/08/31 11:22:53, conradw wrote:
> > On 2015/08/28 09:54:13, wingo wrote:
> > >
>
https://codereview.chromium.org/1309813007/diff/1/src/expression-classifier.h
> > > File src/expression-classifier.h (right):
> > >
> > >
> >
>

https://codereview.chromium.org/1309813007/diff/1/src/expression-classifier.h#newcode158
> > > src/expression-classifier.h:158: void RecordPatternError(const
> > > Scanner::Location& loc,
> > > On 2015/08/27 15:32:24, conradw wrote:
> > > > I was thinking it might be possible to eliminate/combine the
> > > > assignment_pattern_error_ object, since every time
> assignment_pattern_error_
> > > is
> > > > set, binding_pattern_error_ is set identically. From what you're
saying,
> > there
> > > > would still need to be two separate bits in invalid_productions_
however.
> > >
> > > Not sure that this works.  Consider:
> > >
> > >   ({x.y, 2}) => 45
> > >
> > > We're going to want to raise the binding pattern error, not the
assignment
> > > pattern error.
> >
> > My impression was that this is the reason that {assignment,
binding}_pattern
> > errors are both recorded in the case of a potential assignment pattern
error.
> > Since the message is identical, I feel like it's possible to eliminate a
copy
> of
> > the error object in this case (as long as the distinction of validating > > specifically for an assignment pattern error, for the case caitp mentioned
> > above, still exists in the invalid_productions_ pseudo-bitfield).
>
> Do you expect the same error for these productions?
>
>   ({x.y, 2});
>   ({x.y, 2}) => 45;
>
> I would expect the first to give:
>
>   ({x.y, 2});
>             ^ Unexpected token: 2
>
> and the second:
>
>   ({x.y, 2}) => 45;
>      ^ Unexpected token: .

Ah, I see, I completely missed the point you were trying to make first time
around, sorry :P

I suppose it could still be made to work by reporting the shared error
preferentially but I can see how that could make things unintuitive. Seems
like
a bad idea then!

I haven't changed the error reporting just yet (need some thought for that), but
I've made some changes:

- destructuring assignment is behind --harmony-destructuring-assignment flag
rather than --harmony-destructuring
- AST node stuff is simplified a bit for preparser's benefit
(IsPatternOrLiteral() is added to full AST and PreParserExpression to support
this for the preparser)

The arrow function test that relies on destructuring assignment isn't fixed
here, I'd like to see if that can be fixed without depending on destructuring
assignment

https://codereview.chromium.org/1309813007/

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