101488 - Array(m).length = new Number(n) failing (silently)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Phil Schwartau]

Assigning Array(m).length = Number(n) is working correctly:

js> var arr = Array(5); arr.length = Number(1); print(arr.length);
1
js> arr.length
1


But with the "new" keyword before Number(), we get no output and no error: 

js> var arr = Array(5); arr.length = new Number(1); print(arr.length);
js>

In addition, the length has not changed:

js> arr.length
5


(This testcase is due to [e-mail])

Comment 1

[Brendan Eich [:brendan]]

mccabe goofed ValueIsLength in jsarray.c -- it fails to convert objects and
other types than number (internally represented by a 31-bit tagged int, or by a
jsdouble) to number, then to a uint32 -- and it throws a silent error with that
final 'return JS_FALSE;' statement (without a call to JS_ReportError or one of
its siblings).

khanson, this is pretty easy to fix, worth doing for 0.9.5?  Let me know if I
can help -- I'll look for a patch to review.

/be

Comment 2

[Phil Schwartau]

Testcase added to JS testsuite:

           mozilla/js/tests/ecma_3/Array/regress-101488.js

However, since the bug involves a silent exit with no error, 
I will verify any fix interactively, via the steps given above -

Comment 3

[Kenton Hanson (gone)]

Attachment: proposed fix

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 50895 [details] [diff] [review]
proposed fix

Looks good, maybe remove the "mccabe gets his wish" comment?

Also, how about a diff -u for applying via patch?  Thanks.

/be

Comment 5

[Kenton Hanson (gone)]

Attachment: revised patch, eliminates comment

Comment 6

[Kenton Hanson (gone)]

Looking for r and sr

Comment 7

[rogerl (gone)]

r=rogerl

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 51085 [details] [diff] [review]
revised patch, eliminates comment

Thanks -- I'll check in.

/be

Comment 9

[Brendan Eich [:brendan]]

I also removed the bogus lengthp null test in the JSVAL_IS_INT "fast path" early
return case.

/be

Comment 10

[Phil Schwartau]

Verified interactively in debug and optimized JS shells built 2001-10-26
on WinNT, Linux, and Mac.