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

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
17 years ago
13 years ago

People

(Reporter: Phil Schwartau, Assigned: Kenton Hanson (gone))

Tracking

({js1.5})

Trunk
x86
All
js1.5
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [QA note: verify any fix interactively])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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)
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

17 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

17 years ago
Created attachment 50895 [details] [diff] [review]
proposed fix
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

17 years ago
Attachment #50895 - Attachment is obsolete: true
(Assignee)

Comment 5

17 years ago
Created attachment 51085 [details] [diff] [review]
revised patch, eliminates comment
(Assignee)

Comment 6

17 years ago
Looking for r and sr
Status: NEW → ASSIGNED

Comment 7

17 years ago
r=rogerl
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+
I also removed the bogus lengthp null test in the JSVAL_IS_INT "fast path" early
return case.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

17 years ago
Verified interactively in debug and optimized JS shells built 2001-10-26
on WinNT, Linux, and Mac. 
Status: RESOLVED → VERIFIED

Updated

13 years ago
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.