Closed Bug 638997 Opened 9 years ago Closed 9 years ago

Remove PropDesc::id

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: Waldo, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Attached patch PatchSplinter Review
Property descriptors in the spec don't have an id field.  Moreover, storing the id is unnecessary excess baggage every place it's needed.  I'm not sure why I ever put it in the property descriptor struct.

This gets rid of a bogus bit of info that an ES5-like MOP won't need to set/worry about.  It also (in passing, because I have to touch two places that use descriptor arrays to fix both) commons out code used by Object.defineProperties and Object.create, a nice simplification (one even more like the spec, actually, which says for .create "as if by" Object.defineProperties).
Attachment #517057 - Flags: review?
Attachment #517057 - Flags: review? → review?(luke)
Comment on attachment 517057 [details] [diff] [review]
Patch

> static bool
> DefineProperties(JSContext *cx, JSObject *obj, JSObject *props)
> {
>-    AutoIdArray ida(cx, JS_Enumerate(cx, props));
>-    if (!ida)
>+    AutoIdVector ids(cx);
>+    if (!GetPropertyNames(cx, props, JSITER_OWNONLY, &ids))
>         return false;

>@@ -2598,27 +2592,8 @@ obj_create(JSContext *cx, uintN argc, Va
>-        JSObject *props = &vp[3].toObject();
>-        AutoIdArray ida(cx, JS_Enumerate(cx, props));
>-        if (!ida)
>+        if (!DefineProperties(cx, obj, &vp[3].toObject()))
>             return JS_FALSE;
>-
>-        AutoPropDescArrayRooter descs(cx);
>-        size_t len = ida.length();
>-        for (size_t i = 0; i < len; i++) {
>-            jsid id = ida[i];
>-            PropDesc *desc = descs.append();
>-            if (!desc || !JS_GetPropertyById(cx, props, id, Jsvalify(&vp[1])) ||
>-                !desc->initialize(cx, id, vp[1])) {
>-                return JS_FALSE;
>-            }
>-        }

Nice; IIUC, this also skips a malloc/memcpy of the id array!
Attachment #517057 - Flags: review?(luke) → review+
http://hg.mozilla.org/tracemonkey/rev/320432649bc7
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/320432649bc7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.