ArraySplice builtin should return empty array and not alter receiver if invoked with no arguments.

ArraySplice builtin should return empty array and not alter receiver if invoked with no arguments.

Committed: http://code.google.com/p/v8/source/detail?r=6540

[antonm, 2011-01-28 19:22:08]

Mads,

may you have a look?

[Kevin Millikin (Chromium), 2011-01-31 07:55:11]

LGTM.

But, if we don't think it is performance critical to have the fast path, we
could also fix it by narrowing the application of the special case that follows
(I think...).

http://codereview.chromium.org/6357025/diff/2001/src/array.js
File src/array.js (right):

http://codereview.chromium.org/6357025/diff/2001/src/array.js#newcode612
src/array.js:612: if (num_arguments > 1) {
This is pretty weird.  It looks like the special case is only for not delete
count if start is given, i.e.:

if (num_arguments != 1) {
  // The normal thing according to the algorithm.
} else {
  del_count = len - start_i;
}

[antonm, 2011-01-31 11:33:07]

Kevin,

Case of no parameters call shouldn't be performance-critical I hope.  But early
return might be cleaner.

May you have a look at another attempt?  Alas, even though JS change is notably
smaller, C++ counterpart needs bigger changes (and I'd prefer to keep the logic
in both as parallel as possible)

[Kevin Millikin (Chromium), 2011-01-31 12:32:03]

Looks good.  I agree that we should keep the two implementations as close as
reasonable.

Maybe the comment about compatibility should actually say what behavior we're
trying to achieve (though spelling it out makes it seem like a silly behavior:
"An undefined 'deleteCount' argument deletes no elements, however an absent
'deleteCount' argument deletes all elements starting from the 'start' argument,
unless the 'start' argument is also absent.")

http://codereview.chromium.org/6357025/diff/6002/src/array.js
File src/array.js (right):

http://codereview.chromium.org/6357025/diff/6002/src/array.js#newcode610
src/array.js:610: if (num_arguments != 1) {
OK.  Now maybe it's clearer to change the comparison to (num_arguments == 1) and
flip the branches?  (Also in the C++ code.)

[Kevin Millikin (Chromium), 2011-01-31 14:51:56]

Yes.  Good.