482381 - Add a JS_GetPropertyDescriptorById

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Currently, getting a property's attributes, getter and setter anywhere on the prototype chain involves calling JS_LookupPropertyWithFlagsById, followed by JS_GetPropertyAttrsGetterAndSetter. Having to call two functions is less than ideal. Here's a patch to add it.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 366478 [details] [diff] [review]
patch v1

>+    JS_ASSERT((foundp != NULL) ^ (objp != NULL));

Some find != less obscure than ^ here. YMMV, look for other such assertions.

>+            if (vp)
>+                *vp = NULL;

Oops -- JSVAL_VOID, not NULL.

>+/*
>+ * Like JS_GetPropertyAttrsGetterAndSetterById but will return a property on
>+ * an object on the prototype chain (returned in objp). If objp is null, then
>+ * this property was not found on the prototype chain.
>+ */
>+extern JS_PUBLIC_API(JSBool)
>+JS_LookupPropertyInfoById(JSContext *cx, JSObject *obj, jsid id, uintN flags,
>+                          JSObject **objp, uintN *attrsp,
>+                          JSPropertyOp *getterp, JSPropertyOp *setterp,
>+                          jsval *vp);

My only misgiving about adding this now is that we might prefer JS_GetOwnPropertyDescriptor based on ES3.1 in the long run, and a C-ish version of it might look a lot like this, but with a struct out param (a la stat(2)). Or maybe not. Thoughts welcome from you, jorendorff, and bugwatchers.

/be

Comment 3

[Jeff Walden [:Waldo]]

This might be too cute, but C++ does have trigraphs (?), and xor would be clearer there than ^.

Comment 4

[Brendan Eich [:brendan]]

Waldo: your (?) came thru like that, but let's not use trigraphs. Either != or ^. This is not the obfuscated C programming contest you know :-P.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v2

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v3

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 366496 [details] [diff] [review]
patch v3

>+struct JSPropertyDescriptor {
>+    JSObject *obj;
>+    uintN attrs;
>+    JSPropertyOp getter;
>+    JSPropertyOp setter;
>+    jsval v;
>+};

Nits: s/v/value/ and tabulate so declarators (including * for obj) start in same column.

/be

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v3+

This is what I'll check in tomorrow.

Comment 9

[Jason Orendorff [:jorendorff]]

This is a fine idea.  It does seem weird to have structs named JSPropertyDesc and JSPropertyDescriptor though.

mrbkap, please document this on MDC.

Instead of "value" it could be "slotValue" or "storedValue" (which has the dubious advantage of being the term I use in the documentation; see https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/Stored_value and links to that page in places like https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_LookupProperty ).

Comment 10

[Brendan Eich [:brendan]]

JSPropertyDesc and JSPropertyDescArray hide in jsdbgapi.h. This is not the worst such name near-miss in the history of long-lived code (see Unix system calls, or the Windows API). But we could deprecate or rename them more easily. For ES3.1 brainprint reduction, the new API in this bug should use Property Descriptor as directly as possible.

With a C++ API we could lose the JS prefix and make a js namespace. Purty!

/be

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/14677a61f29d

Comment 12

[Brendan Eich [:brendan]]

I would rather just "value" -- docs can say "stored", value matches ES3.1's [[Value]]. It's the word the engine uses everywhere, and abbreviates via v and vp. The fact that it's stored becomes implicit if it isn't the first few times you run across it.

/be

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/924cf2bcec65 addresses comment 12.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/248ba0559930 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a81ce137b944 fixed this on the 1.9 branch.

Comment 15

[Shawn Wilsher :sdwilsh]

The documentation for this method is sadly lacking in usefulness (https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GetPropertyDescriptorById).

Comment 16

[Eric Shepherd [:sheppy]]

I've added an initial table here, but could use some help filling out details. Can anyone help?

https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GetPropertyDescriptorById#The_JSPropertyDescriptor_structure