Some more comments :) Sorry for the back and forth but there are a lot of
edge
cases now!
https://codereview.chromium.org/553623004/diff/100001/test/mjsunit/array-length.js
File test/mjsunit/array-length.js (right):
https://codereview.chromium.org/553623004/diff/100001/test/mjsunit/array-length.js#newcode172
test/mjsunit/array-length.js:172: assertEquals(o.length,
Number.MAX_SAFE_INTEGER + 1);
This test is valid -- it will create a sparse array. Keep it.
https://codereview.chromium.org/553623004/diff/100001/test/mjsunit/array-length.js#newcode186
test/mjsunit/array-length.js:186: assertEquals(o.length,
Number.MAX_SAFE_INTEGER - 1);
This test is also valid, I think.
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js
File test/mjsunit/array-length.js (right):
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode125
test/mjsunit/array-length.js:125: assertEquals(Number.MIN_VALUE,
o.length);
TBH I don't find this test to be useful -- toString() isn't going to
mutate the array-like that it receives, right? Here the right thing to
do is to test that the result is equal to "" or whatever the spec says
it should be..
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode131
test/mjsunit/array-length.js:131: assertEquals(Number.MIN_VALUE,
o.length);
Likewise, just check that the result is what you expect.
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode137
test/mjsunit/array-length.js:137: assertEquals(Number.MIN_VALUE,
o.length);
Likewise
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode144
test/mjsunit/array-length.js:144: assertEquals(1, o.length);
Here OTOH it does make sense to check the length, cool. Perhaps check
that o[0] is 1.
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode200
test/mjsunit/array-length.js:200: assertEquals(1, o.length);
need to test negative indexes with >2^32 length; see the spec
https://codereview.chromium.org/553623004/diff/140001/test/mjsunit/array-length.js#newcode201
test/mjsunit/array-length.js:201:
need to test also:
*slice* with large indexes (a small slice at the end)
slice with large indexes and big arrays (take a small slice at the end)
lastIndexOf and indexOf with a large array and a large fromIndex (close
to the length of the array)
fill with large array, negative start offset, and/or negative end offset
https://codereview.chromium.org/553623004/
--
--
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.
