It would also be nice to provide a test to make sure we do not alias the
arguments.
function f(x, ...xs) {
// assert not strict
arguments[0] = 1;
assertEquals(3, x);
arguments[1] = 2;
assertArrayEquals([4, 5], xs);
}
f(3, 4, 5);
https://codereview.chromium.org/816913003/diff/240001/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/816913003/diff/240001/src/code-stubs.h#newcode1618
src/code-stubs.h:1618: virtual CallInterfaceDescriptor
GetCallInterfaceDescriptor() OVERRIDE {
we no longer use virtual when override is present.
https://codereview.chromium.org/816913003/diff/240001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/compiler/ast-graph-builder.cc#newcode2040
src/compiler/ast-graph-builder.cc:2040: // This hsould never lazy deopt,
so it is fine to send invalid bailout id.
typo
https://codereview.chromium.org/816913003/diff/240001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/816913003/diff/240001/src/objects-inl.h#newcode6169
src/objects-inl.h:6169: return
shared()->scope_info()->IsSimpleParameterList();
or
shared()->is_simple_parameter_list();
https://codereview.chromium.org/816913003/diff/240001/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/preparser.cc#newcode875
src/preparser.cc:875: is_rest = peek() == Token::ELLIPSIS &&
allow_harmony_rest_params();
Code duplication of preparser/parser makes me sad.
No need to fix though...
https://codereview.chromium.org/816913003/diff/240001/src/runtime/runtime-scopes.cc
File src/runtime/runtime-scopes.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/runtime/runtime-scopes.cc#newcode494
src/runtime/runtime-scopes.cc:494:
DCHECK(callee->is_simple_parameter_list());
Should be
DCHECK(!callee->is_simple_parameter_list());
If you run the test in debug mode you might be able to find the case
where you end up with a sloppy arg where you expect a strict arg.
https://codereview.chromium.org/816913003/diff/240001/src/runtime/runtime-scopes.cc#newcode513
src/runtime/runtime-scopes.cc:513: parameters -= rest_index;
I still think this would be cleaner if you didn't do pointer
arithmetics.
https://codereview.chromium.org/816913003/diff/240001/src/runtime/runtime-scopes.cc#newcode547
src/runtime/runtime-scopes.cc:547: // Determine parameter location on
the stack and dispatch on language mode.
" dispatch on language mode"
What does that mean?
https://codereview.chromium.org/816913003/diff/240001/src/scanner.cc
File src/scanner.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/scanner.cc#newcode602
src/scanner.cc:602: if (c0_ == '.') {
We could do the feature flag check here as well?
https://codereview.chromium.org/816913003/diff/240001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/scopes.cc#newcode185
src/scopes.cc:185: rest_parameter_ = nullptr;
NULL for consistency?
https://codereview.chromium.org/816913003/diff/240001/src/scopes.cc#newcode1309
src/scopes.cc:1309: if (rest_parameter_ &&
!MustAllocate(rest_parameter_)) {
I'm curious. What is going on here?
https://codereview.chromium.org/816913003/diff/240001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/x64/code-stubs-x64.cc#newcode867
src/x64/code-stubs-x64.cc:867: // Patch the arguments.length and the
parameters pointer.
Maybe we should refactor this to not duplicate this code?
https://codereview.chromium.org/816913003/diff/240001/src/x64/code-stubs-x64.cc#newcode873
src/x64/code-stubs-x64.cc:873:
StandardFrameConstants::kCallerSPOffset));
wrong indentation
https://codereview.chromium.org/816913003/diff/240001/src/x64/code-stubs-x64.cc#newcode883
src/x64/code-stubs-x64.cc:883: /*(__ movp(rdx, rcx);
Remove?
https://codereview.chromium.org/816913003/diff/240001/src/x64/code-stubs-x64.cc#newcode894
src/x64/code-stubs-x64.cc:894: // __ Allocate(rcx, rax, rdx, rbx,
&runtime, TAG_OBJECT);
remove?
https://codereview.chromium.org/816913003/diff/240001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):
https://codereview.chromium.org/816913003/diff/240001/src/x64/full-codegen-x64.cc#newcode237
src/x64/full-codegen-x64.cc:237: int rest_index;
Is rest_index -1 if not set? Maybe do the test for that before creating
rest_param?
https://codereview.chromium.org/816913003/diff/240001/src/x64/full-codegen-x64.cc#newcode251
src/x64/full-codegen-x64.cc:251: Operand(rbp,
StandardFrameConstants::kCallerSPOffset + offset));
indentation
https://codereview.chromium.org/816913003/diff/240001/test/mjsunit/harmony/rest-params.js
File test/mjsunit/harmony/rest-params.js (right):
https://codereview.chromium.org/816913003/diff/240001/test/mjsunit/harmony/rest-params.js#newcode40
test/mjsunit/harmony/rest-params.js:40: function sloppySlowTest(a, a,
...c) {
This should be a SyntaxError.
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-function-definitions-static-semantics-early-errors
Maybe file a bug so we don't forget about this?
https://codereview.chromium.org/816913003/
--
--
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.
