https://codereview.chromium.org/938443002/diff/80001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/938443002/diff/80001/src/bootstrapper.cc#newcode1619
src/bootstrapper.cc:1619:
EMPTY_NATIVE_FUNCTIONS_FOR_FEATURE(harmony_spreadcalls)
Why do we need a separate flag?
https://codereview.chromium.org/938443002/diff/80001/src/harmony-spread.js
File src/harmony-spread.js (right):
https://codereview.chromium.org/938443002/diff/80001/src/harmony-spread.js#newcode12
src/harmony-spread.js:12: var array = (new
InternalArray()).concat(%_Arguments(0));
On 2015/02/23 17:55:56, arv wrote:
Why not Push. concat creates a new array so we create one too many.
+1, use push
https://codereview.chromium.org/938443002/diff/80001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/938443002/diff/80001/src/preparser.h#newcode2320
src/preparser.h:2320: Scanner::Location* spread_arg_loc, bool* ok) {
Maybe name this first_spread_arg_loc to clarify that it points to the
first of possibly many spread args.
https://codereview.chromium.org/938443002/diff/80001/src/preparser.h#newcode2349
src/preparser.h:2349: unspread_sequences_count++;
Something seems wrong with the was_unspread logic. AFAICS, this only
increments unspread_sequences_count at most once.
https://codereview.chromium.org/938443002/diff/80001/src/runtime/runtime-function.cc
File src/runtime/runtime-function.cc (right):
https://codereview.chromium.org/938443002/diff/80001/src/runtime/runtime-function.cc#newcode578
src/runtime/runtime-function.cc:578: DCHECK(args.length() == 5 ||
args.length() == 3);
On 2015/02/23 17:55:56, arv wrote:
I think it would be cleaner to have to functions.
+1
https://codereview.chromium.org/938443002/diff/80001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/938443002/diff/80001/test/cctest/test-parsing.cc#newcode4993
test/cctest/test-parsing.cc:4993: "...'123', ...'456'",
Have some more tests with spreads interleaved with non-spreads.
https://codereview.chromium.org/938443002/diff/80001/test/mjsunit/harmony/spread-call.js
File test/mjsunit/harmony/spread-call.js (right):
https://codereview.chromium.org/938443002/diff/80001/test/mjsunit/harmony/spread-call.js#newcode247
test/mjsunit/harmony/spread-call.js:247: assertEquals("ABXYC", log);
Can you extend this test to have several spreads and non-spreads mixed?
https://codereview.chromium.org/938443002/
--
--
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.
