487930 - removal of JSSLOT_ARRAY_LOOKUP_HOLDER

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

Currently array_lookupProperty from jsarray.cpp uses a special slot, JSSLOT_ARRAY_LOOKUP_HOLDER, to store the index passed to the function. But this is not necessary since to satisfy JSObjectOps requirements it is sufficient to convert the id itself to JSProperty* and cast it back in array_dropProperty and array_getAttributes.

It not only shrinks the code but also free the slot for other uses.

Comment 1

[Igor Bukanov]

When creating a patch for this bug I noticed a regression that I introduced in bug 486106. There I missed that array_lookupProperty also takes care about the length property. So I will use this bug to fix that as well.

I also nominate for 1.9.1  - the bug does not affect browser or js shell, but it could bite other SpiderMonkey users.

Comment 2

[Igor Bukanov]

Attachment: v1

The essence of the patch is casting jsid to (JSProperty *) in array_lookupProperty. The rest is fixes to make sure that js_GetDenseArrayElementValue returns the correct result for any id that was accepted in array_lookupProperty. The change required to add JSContext *cx parameter to js_GetDenseArrayElementValue triggering a cascade of changes for Lookup* API in jsapi.cpp.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 372236 [details] [diff] [review]
v1

>Index: tm/js/src/jsarray.cpp
>  * In dense mode, holes in the array are represented by JSVAL_HOLE.  The final
>- * slot in fslots (JSSLOT_ARRAY_LOOKUP_HOLDER) is used to store the single jsid
>- * "in use" by a lookupProperty caller.
>+ * slot in fslots is unused..

Nit: extra period.

Comment 4

[Igor Bukanov]

landed to TM with the nit addressed - http://hg.mozilla.org/tracemonkey/rev/ccb029897983

Comment 5

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/ccb029897983

Comment 6

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4c644e6fd58c