Fixes bug in Array.prototype.lastIndexOf when called with null or undefined a...

Fixes bug in Array.prototype.lastIndexOf when called with null or undefined as fromIndex argument. (fixes issue 754).


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

[Lasse Reichstein, 2010-06-25 09:10:32]

LGTM

http://codereview.chromium.org/2840021/diff/1/2
File src/array.js (right):

http://codereview.chromium.org/2840021/diff/1/2#newcode957
src/array.js:957: if (index == null) {
Use
 if (IS_UNDEFINED(index)) {
instead. It'll catch the case where there is only on argument  just as well, but
won't waste time comparing to null.

http://codereview.chromium.org/2840021/diff/1/3
File test/mjsunit/regress/regress-754.js (right):

http://codereview.chromium.org/2840021/diff/1/3#newcode37
test/mjsunit/regress/regress-754.js:37: assertEquals(a.lastIndexOf(2, null),
-1);
Add the cases with only one argument as well.
Put expected value as first arguemtn to assertEquals, and actual as second.
Always give a third argument to assertEquals, so you can see which test failed
(if the expected values aren't unique in the file). They don't have to be fancy
or saying, just unique and searchable.

[Lasse Reichstein, 2010-06-25 09:10:32]

LGTM

http://codereview.chromium.org/2840021/diff/1/2
File src/array.js (right):

http://codereview.chromium.org/2840021/diff/1/2#newcode957
src/array.js:957: if (index == null) {
Use
 if (IS_UNDEFINED(index)) {
instead. It'll catch the case where there is only on argument  just as well, but
won't waste time comparing to null.

http://codereview.chromium.org/2840021/diff/1/3
File test/mjsunit/regress/regress-754.js (right):

http://codereview.chromium.org/2840021/diff/1/3#newcode37
test/mjsunit/regress/regress-754.js:37: assertEquals(a.lastIndexOf(2, null),
-1);
Add the cases with only one argument as well.
Put expected value as first arguemtn to assertEquals, and actual as second.
Always give a third argument to assertEquals, so you can see which test failed
(if the expected values aren't unique in the file). They don't have to be fancy
or saying, just unique and searchable.

[Rico, 2010-06-25 09:27:10]

http://codereview.chromium.org/2840021/diff/1/2
File src/array.js (right):

http://codereview.chromium.org/2840021/diff/1/2#newcode957
src/array.js:957: if (index == null) {
On 2010/06/25 09:10:32, Lasse Reichstein wrote:
> Use
>  if (IS_UNDEFINED(index)) {
> instead. It'll catch the case where there is only on argument  just as well,
but
> won't waste time comparing to null.

Done.

http://codereview.chromium.org/2840021/diff/1/3
File test/mjsunit/regress/regress-754.js (right):

http://codereview.chromium.org/2840021/diff/1/3#newcode37
test/mjsunit/regress/regress-754.js:37: assertEquals(a.lastIndexOf(2, null),
-1);
On 2010/06/25 09:10:32, Lasse Reichstein wrote:
> Add the cases with only one argument as well.
> Put expected value as first arguemtn to assertEquals, and actual as second.
> Always give a third argument to assertEquals, so you can see which test failed
> (if the expected values aren't unique in the file). They don't have to be
fancy
> or saying, just unique and searchable.
Single argument tests added.
With the new test script the actual line number of the failure for mjsunit tests
is provided (i.e., the unique dummy values are not needed anymore)