505587 - Implement ES5 Object.getOwnPropertyDescriptor

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jeff Walden [:Waldo]]

Needed for verifying correctness of Object.defineProperty...

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Still needs tests, will start formally writing them shortly...

Comment 2

[Jeff Walden [:Waldo]]

Attachment: A decent set of tests for a start

I also ran the patch against the ES5 tests at <http://es5conform.codeplex.com/>, and things seem to work perfectly fine.

This is currently limited to what's exposed by ES5 itself; bug 430133 and Object.defineProperty will allow access to the real corner cases that can't be examined in the current specified APIs.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 397398 [details] [diff] [review]
Patch

>+static JSBool
>+obj_getOwnPropertyDescriptor(JSContext *cx, uintN argc, jsval *vp)
>+{
>+    if (argc == 0 || JSVAL_IS_PRIMITIVE(vp[2])) {
>+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                             JSMSG_NOT_NONNULL_OBJECT,
>+                             js_getter_str);
>+        return JS_FALSE;
>+    }
>+    JSObject *obj = JSVAL_TO_OBJECT(vp[2]);

Nit: blank line after the }.

>+        JSScopeProperty *sprop = reinterpret_cast<JSScopeProperty *>(prop);
>+        if (!desc->defineProperty(cx, ATOM_TO_JSID(atomState.getAtom),
>+                                  ((attrs & JSPROP_GETTER) && OBJ_IS_NATIVE(obj))
>+                                  ? js_CastAsObjectJSVal(sprop->getter)
>+                                  : JSVAL_VOID,
>+                                  JS_PropertyStub, JS_PropertyStub,
>+                                  JSPROP_ENUMERATE, NULL) ||
>+            !desc->defineProperty(cx, ATOM_TO_JSID(atomState.setAtom),
>+                                  ((attrs & JSPROP_SETTER) && OBJ_IS_NATIVE(obj))
>+                                  ? js_CastAsObjectJSVal(sprop->setter)
>+                                  : JSVAL_VOID,
>+                                  JS_PropertyStub, JS_PropertyStub,
>+                                  JSPROP_ENUMERATE, NULL)) {

I'd prefer to move the OBJ_IS_NATIVE checks (and the cast to JSScopeProperty) into a common place.

Comment 4

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/b7ee9263cf96

with the requested changes, also with the attached tests in the new ecma_5/Object/ directory.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/b7ee9263cf96

Comment 6

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/87fcc6cb16af to update tests to use reportCompare once

Comment 7

[Jeff Walden [:Waldo]]

I wrote up a little bit of documentation, but it needs to be made more visible than simply exposing it deep within the JS reference:

https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Objects/Object/getOwnPropertyDescriptor