Closed
Bug 101488
Opened 23 years ago
Closed 23 years ago
Array(m).length = new Number(n) failing (silently)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: pschwartau, Assigned: khanson)
Details
(Keywords: js1.5, Whiteboard: [QA note: verify any fix interactively])
Attachments
(1 file, 1 obsolete file)
1.16 KB,
patch
|
brendan
:
review+
brendan
:
superreview+
|
Details | Diff | Splinter Review |
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 igor@icesoft.no)
Comment 1•23 years ago
|
||
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
Keywords: js1.5,
mozilla0.9.5
Reporter | ||
Comment 2•23 years ago
|
||
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 -
Whiteboard: [QA note: verify any fix interactively]
Assignee | ||
Comment 3•23 years ago
|
||
Comment 4•23 years ago
|
||
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
Assignee | ||
Updated•23 years ago
|
Attachment #50895 -
Attachment is obsolete: true
Assignee | ||
Comment 5•23 years ago
|
||
Comment 7•23 years ago
|
||
r=rogerl
Comment 8•23 years ago
|
||
Comment on attachment 51085 [details] [diff] [review] revised patch, eliminates comment Thanks -- I'll check in. /be
Attachment #51085 -
Flags: superreview+
Attachment #51085 -
Flags: review+
Comment 9•23 years ago
|
||
I also removed the bogus lengthp null test in the JSVAL_IS_INT "fast path" early return case. /be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 10•23 years ago
|
||
Verified interactively in debug and optimized JS shells built 2001-10-26 on WinNT, Linux, and Mac.
Status: RESOLVED → VERIFIED
Updated•19 years ago
|
Flags: testcase+
You need to log in
before you can comment on or make changes to this bug.
Description
•