https://codereview.chromium.org/1238593003/diff/40001/src/array.js
File src/array.js (left):
https://codereview.chromium.org/1238593003/diff/40001/src/array.js#oldcode571
src/array.js:571: if (!IS_UNDEFINED(current_i) || low in array) {
As discussed offline, I don't think SparseReverse needs to be changed at
all, since it's guaranteed that [[Get]] and [[HasProperty]] are both
side-effect free in here.
https://codereview.chromium.org/1238593003/diff/40001/src/array.js
File src/array.js (right):
https://codereview.chromium.org/1238593003/diff/40001/src/array.js#newcode637
src/array.js:637: } else if (%_HasFastPackedElements(array)) {
This needs an IS_ARRAY check (you can cache this above). Please add a
test covering this. This object violates the contract
PackedArrayReverse() is expecting:
{ length: 2, 0: 'foo', __proto__: { get 1() { delete this[1] } } }
Arrays can't have this problem because of their exotic 'length'.
https://codereview.chromium.org/1238593003/diff/40001/src/array.js#newcode640
src/array.js:640: return InnerArrayReverse(array, len);
how about "GenericArrayReverse" as the name for this instead?
https://codereview.chromium.org/1238593003/diff/40001/test/mjsunit/es6/array-reverse-order.js
File test/mjsunit/es6/array-reverse-order.js (right):
https://codereview.chromium.org/1238593003/diff/40001/test/mjsunit/es6/array-reverse-order.js#newcode10
test/mjsunit/es6/array-reverse-order.js:10: {length:2, get 0(){delete
this[0];}, 1: "b"}))
Can you spread this out over multiple statements to make it a bit more
readable? I.e., assign the object to a variable, assign the result of
reverse.
Also, "order" in the name of this test doesn't help me much. I'd just
drop it from the name, leaving this as es6/array-reverse.js
https://codereview.chromium.org/1238593003/
--
--
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.
