Change jsint index arguments to JS_*Element functions to uint32 index

RESOLVED FIXED in mozilla8

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments)

Sparse/dense property storage will want this API split to make indexed accesses more efficient.

I queried the newsgroup about making the change (versus adding yet another kind of property-access API), and no one expressed concern.  It's also worth noting that every use of these methods that I found in mozilla-central was using only the non-negative half of the index space.  I expect this is the common case.  I wonder, even, how much anyone ever used negative elements -- I'd guess element stuff is mostly used for arrays and arraylike things, and you're not going to hit negative elements there.

(I did end up changing some Mozilla callers to avoid possible compiler warnings.  Technically I probably could have gotten away without doing this, but it seemed a good time to clean up signage usage in that code when I already had to audit them to make sure none depended on being able to pass negative indexes.)
Created attachment 550917 [details] [diff] [review]
Change JS_DefineElement and adjust users

I'm doing one patch per method changed, to keep things bite-sized.  There's no particular reason it couldn't all be concatenated together into one patch, it just might get (a little bit) big.
Attachment #550917 - Flags: review?(jst)
Attachment #550917 - Flags: review?(dmandelin)
Created attachment 550919 [details] [diff] [review]
Change JS_AlreadyHasOwnElement
Attachment #550919 - Flags: review?(dmandelin)
Created attachment 550920 [details] [diff] [review]
Change JS_HasElement
Attachment #550920 - Flags: review?(dmandelin)
Created attachment 550921 [details] [diff] [review]
Change JS_LookupElement
Attachment #550921 - Flags: review?(dmandelin)
Attachment #550921 - Attachment description: dmandelin@moz → Change JS_LookupElement
Created attachment 550922 [details] [diff] [review]
Change JS_GetElement
Attachment #550922 - Flags: review?(dmandelin)
Created attachment 550923 [details] [diff] [review]
Change JS_SetElement
Attachment #550923 - Flags: review?(dmandelin)
Created attachment 550924 [details] [diff] [review]
Change JS_DeleteElement{,2}
Attachment #550924 - Flags: review?(dmandelin)

Updated

6 years ago
Attachment #550917 - Flags: review?(jst) → review+
Attachment #550917 - Flags: review?(dmandelin) → review+
Attachment #550919 - Flags: review?(dmandelin) → review+
Attachment #550920 - Flags: review?(dmandelin) → review+
Attachment #550921 - Flags: review?(dmandelin) → review+
Attachment #550922 - Flags: review?(dmandelin) → review+
Attachment #550923 - Flags: review?(dmandelin) → review+
Attachment #550924 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/f2ff60137bb3
http://hg.mozilla.org/integration/mozilla-inbound/rev/bcc124e86068
http://hg.mozilla.org/integration/mozilla-inbound/rev/43dbc22aa5fa
http://hg.mozilla.org/integration/mozilla-inbound/rev/cacc7fdcba7b
http://hg.mozilla.org/integration/mozilla-inbound/rev/77ea7ae6ce08
http://hg.mozilla.org/integration/mozilla-inbound/rev/30dd110a4ed6 (not part of this bug, but required for the last two patches to be definitely comprehensive)
http://hg.mozilla.org/integration/mozilla-inbound/rev/2d818a0b3cd1
http://hg.mozilla.org/integration/mozilla-inbound/rev/6030c9f7b3ca
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/f2ff60137bb3
http://hg.mozilla.org/mozilla-central/rev/bcc124e86068
http://hg.mozilla.org/mozilla-central/rev/43dbc22aa5fa
http://hg.mozilla.org/mozilla-central/rev/cacc7fdcba7b
http://hg.mozilla.org/mozilla-central/rev/77ea7ae6ce08
http://hg.mozilla.org/mozilla-central/rev/30dd110a4ed6
http://hg.mozilla.org/mozilla-central/rev/2d818a0b3cd1
http://hg.mozilla.org/mozilla-central/rev/6030c9f7b3ca
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.