Thanks Andy, few more cleanup points here =)
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode3947
src/parser.cc:3947: ReportMessageAt(params_loc,
MessageTemplate::kMalformedArrowFunParamList);
On 2015/08/31 13:07:31, wingo wrote:
Why is this necessary? ParseArrowFunctionFormalParameters should only
be called
on a valid production.
You're right, I realized that this morning but haven't fixed it yet.
Going to airport soon, but I'll update with a new patchset with a fix
when I get back
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4187
src/parser.cc:4187: }
On 2015/08/31 13:07:31, wingo wrote:
Is this bit specific to this patch or is it a general fix?
Well, it's a mix. See https://code.google.com/p/v8/issues/detail?id=4400
for other bugs related to this.
I'm hoping the various literal counter strategies can be sort of unified
later on to make things simpler, but this was the simplest way to do it
for this CL
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4430
src/parser.cc:4430: empty_values, parameters.rest_array_literal_index,
On 2015/08/31 13:07:31, wingo wrote:
Is it necessary to reserve a literal index in the reindexer? Since we
make it
here and we're in the context of the function, seems to me we might be
able to
reserve the index with the normal mechanism.
It seems to be necessary. Without it, an extra slot is allocated in the
context's literals array, which makes things hard for lazy parsing, and
is potentially a bug farm too. I haven't gotten it working without
saving the literal index (either in an AST node or here) and rewriting
it correctly
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h#newcode1044
src/preparser.h:1044: (IsBinaryOperation() &&
HasRestField::decode(code_));
On 2015/08/31 13:07:31, wingo wrote:
Need more comments about the preconditions, i.e. the result is only
valid on the
result of ParseConditionalExpression when the conditional expression
stared with
an LPAREN and the classifier returns true for
is_valid_arrow_formal_parameters(). Actually I think this would be
better
inlined into the trait-specific ParseArrowFunctionFormalParameters.
I'll inline it
https://codereview.chromium.org/1272673003/diff/110001/test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js
File test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js (right):
https://codereview.chromium.org/1272673003/diff/110001/test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js#newcode1
test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js:1: // Copyright
2015 the V8 project authors. All rights reserved.
On 2015/08/31 13:07:31, wingo wrote:
IIRC we don't update these lines; Adam?
This isn't really an update, it's a new file
https://codereview.chromium.org/1272673003/diff/110001/test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js#newcode22
test/mjsunit/harmony/arrow-rest-params-lazy-parsing.js:22: var
strictTest = (function() {
On 2015/08/31 13:07:31, wingo wrote:
Why this change?
It's not legal to have a language mode directive in a function with a
rest parameter
https://codereview.chromium.org/1272673003/
--
--
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.
