Looks like a good approach.

Some high level comments

- Use macro for visitor
- Can you use a non inner class and provide an .h file for PatternMatcher?
Parser.h is already huge.


https://codereview.chromium.org/1130623004/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1130623004/diff/1/src/parser.cc#newcode2496
src/parser.cc:2496: // statements).
I'm not sure what this means?

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc
File src/pattern-matcher.cc (right):

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode9
src/pattern-matcher.cc:9:
Can I has .h file?

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode146
src/pattern-matcher.cc:146: //
stest-parsing/InitializedDeclarationsInStrictForOfError tart context for
typo

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode151
src/pattern-matcher.cc:151: DCHECK(proxy != NULL);
DCHECK_NOT_NULL

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode190
src/pattern-matcher.cc:190: for (auto i = properties->begin(); i !=
properties->end(); i++) {
Can we do?

for (auto property : properties) {

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode196
src/pattern-matcher.cc:196: property->key(), RelocInfo::kNoPosition));
TODO(dslomov): Computed property names

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode207
src/pattern-matcher.cc:207: void
Parser::PatternMatcher::VisitProperty(Property* node) { UNREACHABLE(); }
This is only unreachable in binding patterns:

var o = {x: 1};
({y: o.x} = {y: 2});
assert(o.x === 2);

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode320
src/pattern-matcher.cc:320: void
Parser::PatternMatcher::VisitDoWhileStatement(DoWhileStatement* node) {
Maybe a macro for all of these?

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode341
src/pattern-matcher.cc:341: void
Parser::PatternMatcher::VisitAssignment(Assignment* node) {
TODO(dslomov): Implement

var {x: y = 42} = {};
assert(y === 42);

https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode356
src/pattern-matcher.cc:356: void
Parser::PatternMatcher::VisitSpread(Spread* node) { UNREACHABLE(); }
TODO(dslomov): Implement

var [...xs] = []

https://codereview.chromium.org/1130623004/diff/1/test/mjsunit/harmony/destructuring.js
File test/mjsunit/harmony/destructuring.js (right):

https://codereview.chromium.org/1130623004/diff/1/test/mjsunit/harmony/destructuring.js#newcode8
test/mjsunit/harmony/destructuring.js:8: var { x : x, y : y } = { x : 1,
y : 2 };
Also:

var {z} = {z: 3};

Should work for free since the parser desugars it before it gets to the
destructuring.

https://codereview.chromium.org/1130623004/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to