Kill JSProperty and replace with rooted shapes

RESOLVED FIXED in mozilla16

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

Trunk
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 639201 [details] [diff] [review]
patch

As far as I can tell, JSProperty always holds a Shape or the values 0 or 1. When it holds 1, we're always dealing with non-native objects where we never look at the shape anyway. So we could replace 1 with anything as long as it's not 0, which denotes "not found."

This patch converts every instance of JSProperty to Shape. And it roots the Shape and passes it through handles. In the places where we used to use (JSProperty*)1, I instead just stuck an arbitrary Shape. I think this is okay.
Attachment #639201 - Flags: review?(bhackett1024)
Comment on attachment 639201 [details] [diff] [review]
patch

Review of attachment 639201 [details] [diff] [review]:
-----------------------------------------------------------------

Double yay!  We should have holiday traditions of shoveling obsolete code out of the engine.

::: js/src/jsarray.cpp
@@ +728,5 @@
>      if (!obj->isDenseArray())
>          return baseops::LookupElement(cx, obj, index, objp, propp);
>  
>      if (IsDenseArrayIndex(obj, index)) {
> +        propp.set(obj->lastProperty()); /* just need to set to something non-null */

This and other non-native lookups should use some unwieldy common function like MarkNonNativeLookupSuccess, with a comment explaining what's going on.
Attachment #639201 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/fe305819d2f2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Shouldn't this be reopened?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla16 → ---
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/f88cf68c7fcd
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.