771026 - Kill JSProperty and replace with rooted shapes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: 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.

Comment 1

[Brian Hackett [Laid off!]]

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.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/mozilla-central/rev/fe305819d2f2

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Had to back this out.
https://hg.mozilla.org/mozilla-central/rev/76fd67373e89

Comment 4

[Stefan Fleiter (:sfleiter)]

Shouldn't this be reopened?

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f88cf68c7fcd

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f88cf68c7fcd