638997 - Remove PropDesc::id

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

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).

Comment 1

[Luke Wagner [: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!

Comment 2

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/320432649bc7

Comment 3

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/320432649bc7