Thanks for the look

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 09:40:26, conradw wrote:
BindingPatternErrors and AssignmentPatternErrors being separate seems
odd to me,
from scanning the parser code it looks like the latter implies the
former. This
might be an opportunity to unify them somewhat.

There are cases where they do need to be separate. EG, `[ a.b.c ]` is a
valid AssignmentPattern, but not a valid BindingPattern. I think it's
likely that in the majority of cases, they are the same though.

https://codereview.chromium.org/1309813007/diff/1/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/1309813007/diff/1/src/preparser.h#newcode1134
src/preparser.h:1134: kArrayLiteralExpression
On 2015/08/27 09:40:26, conradw wrote:
Do Object and Array literals need to be distinguished in the
preparser? It seems
like the added checks are satisfied by either.

They would be satisfied by either. If folks prefer, they can be unified
into a single flag.

https://codereview.chromium.org/1309813007/diff/1/src/preparser.h#newcode3008
src/preparser.h:3008: if (is_pattern_element) {
On 2015/08/27 09:40:26, conradw wrote:
It seems to me that the classifier should always record the error, and
it should
be the responsibility of the caller to only convert this to a "real"
parser
error if it's appropriate. I appreciate this might not be possible to
arrange,
however.

The context this flag provides is important. It allows the parser to
treat references like `(a => a).b` as a valid destructuring assignment
target (which is, per spec, correct --- despite not being very useful)

https://codereview.chromium.org/1309813007/diff/1/src/preparser.h#newcode3048
src/preparser.h:3048: } else if (assignment_pattern) {
On 2015/08/27 09:40:26, conradw wrote:
unifying the two errors would clean up this code

The BindingPattern error doesn't really need to be here, thought I had
deleted it. It's marked for cleanup and will be removed in the next
iteration

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