https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode2982
src/parser.cc:2982: factory()->NewBlock(labels, 2, true,
RelocInfo::kNoPosition);
Seems weird that you pass labels both here and to the switch statement.
Are they both needed? Given that all the blocks end at the same place,
it seems like you should only need the labels (and the target) in one
place. Probably on the switch?
Can you add some tests with labeled breaks inside switch, with labels at
various places (on a block containing a switch, on the switch itself, on
a block contained in the switch)?
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode2983
src/parser.cc:2983: Scope* switch_scope = NewScope(scope_, BLOCK_SCOPE);
As discussed, I don't think you need this scope (and in fact it'll
already be dropped for you). It's fine to have a block without a scope.
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode2988
src/parser.cc:2988: // This target exists in case the tag has a
do-expression
This comment likely goes away if you simplify the way labels/targets are
used (and it should go away, as do-expressions are not yet a thing)
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode3010
src/parser.cc:3010: factory()->NewSwitchStatement(labels, switch_pos);
See above, I'd drop the labels here (pass NULL).
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode3021
src/parser.cc:3021: cases = new (zone()) ZoneList<CaseClause*>(4,
zone());
Can you move this initialization up to where cases is declared? Seems
like that'd be simpler.
https://codereview.chromium.org/1293283002/diff/100001/src/parser.cc#newcode3034
src/parser.cc:3034: Expect(Token::RBRACE, CHECK_OK);
I think this should move above the scope stuff just above, the closing
brace is effectively the end of the scope.
Can you update the comment about end positions in scopes.h to mention
switch while you're at it?
https://codereview.chromium.org/1293283002/
--
--
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.
