The default bug view has changed. See this FAQ.

Kill JSProperty and replace with rooted shapes

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

Trunk
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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+
(Assignee)

Comment 2

5 years ago
https://hg.mozilla.org/mozilla-central/rev/fe305819d2f2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
(Assignee)

Comment 3

5 years ago
Had to back this out.
https://hg.mozilla.org/mozilla-central/rev/76fd67373e89
Shouldn't this be reopened?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla16 → ---
Version: unspecified → Trunk
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f88cf68c7fcd
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/f88cf68c7fcd
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.