411968 - Finishing JS_AlreadyHasOwnProperty

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

The new JS_AlreadyHasOwnProperty API (bug 400793) isn't finished: it isn't documented.  I'm working on that, and I have a few questions.

* Why not call it JS_HasOwnProperty?  (I think we should.  We can still rename it; it hasn't been in a release yet.)

* The implementation is quite different from the implementation of obj_hasOwnProperty (in jsobj.c).  Can some code be shared here?

* Object.prototype.hasOwnProperty() handles inner/outer objects specially.  When searching for a property on an object x, if the property is found on x's outer object, the JSAPI says false; the JS method says true.  Is that intentional?

* Object.prototype.hasOwnProperty() also handles JSPROP_SHARED, JSPROP_PERMANENT properties specially.   JS_AlreadyHasOwnProperty doesn't, so it exposes an implementation detail about Function.length/arity.  Is that intentional?

Comment 1

[Jason Orendorff [:jorendorff]]

* It doesn't CHECK_REQUEST(cx).  Should it?

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #0)
> * Why not call it JS_HasOwnProperty?  (I think we should.  We can still rename
> it; it hasn't been in a release yet.)

Because ECMA specifies hasOwnProperty on Object.prototype, and that does something different from what the underpinning of this API does. We may want to expose that standard method as JS_HasOwnProperty. This API does not resolve lazily-created properties -- hence the "Already".

> * The implementation is quite different from the implementation of
> obj_hasOwnProperty (in jsobj.c).  Can some code be shared here?

No, see above (and see the other bug, I thought we discussed all this there, but it might have been more on IRC; IRC is a problem :-/).

> * Object.prototype.hasOwnProperty() handles inner/outer objects specially. 
> When searching for a property on an object x, if the property is found on x's
> outer object, the JSAPI says false; the JS method says true.  Is that
> intentional?

I think so. Again, this is *not* ECMA hasOwnProperty, reflected as a JS API.

> * Object.prototype.hasOwnProperty() also handles JSPROP_SHARED,
> JSPROP_PERMANENT properties specially.   JS_AlreadyHasOwnProperty doesn't, so
> it exposes an implementation detail about Function.length/arity.  Is that
> intentional?

Yes, see above.

I'm pretty sure you are barking up the wrong tree here, except for the doc writing goodness -- for which, thanks! :-).

/be

Comment 3

[Brendan Eich [:brendan]]

Cc'ing bz, since he is sole user of this new API. Jason, you are right we can change this still, but since it can't do what Object.prototype.hasOwnProperty, we may end up doc'ing it as is. But don't let me stop you from proposing better API impl and/or name, taking into account the intentionality of the original change.

/be

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: CHECK_REQUEST(cx) in JS_AlreadyHasOwn{,UC}Property

Thanks, Brendan.

I think this API should CHECK_REQUEST(cx), because it calls JS_LOCK_OBJ.  Patch attached.  I searched mxr for uses and only found <http://mxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLProtoImpl.cpp#258>, which is only called from within auto-requests.

Also please see http://developer.mozilla.org/en/docs/JS_AlreadyHasOwnProperty .

Comment 5

[Reed Loden [:reed]]

Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.385; previous revision: 3.384
done

Not sure if this bug is fixed completely, so leaving open.

Comment 6

[Jason Orendorff [:jorendorff]]

Thanks, Reed.  It's fixed.