Closed Bug 492845 Opened 15 years ago Closed 14 years ago

ES5: Implement Object.seal, Object.isSealed

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: jimb, Assigned: jimb)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 file, 1 obsolete file)

Object.seal and Object.isSealed are specified by 15.2.3.8 and 15.2.3.11 in the ES5 draft:
http://wiki.ecmascript.org/doku.php?id=es3.1:es3.1_proposal_working_draft
Assignee: general → jwalden+bmo
Blocks: es5
blocking2.0: --- → ?
blocking2.0: ? → beta1+
blocking2.0: beta1+ → beta2+
blocking2.0: beta2+ → betaN+
This bug depends on Brendan's scope-removal work in bug 558451.
Depends on: 558451
blocking2.0: betaN+ → beta5+
blocking2.0: beta5+ → beta6+
Attached patch Patch (obsolete) — Splinter Review
This goes atop bug 492849 (extensibility bug), implements both seal and freeze primitives.  Same time/reviewer caveats apply as with that bug/patch.
Attachment #470866 - Flags: review?(jorendorff)
Attached patch RebasedSplinter Review
Attachment #470866 - Attachment is obsolete: true
Attachment #473779 - Flags: review?(brendan)
Attachment #470866 - Flags: review?(jorendorff)
Comment on attachment 473779 [details] [diff] [review]
Rebased

># HG changeset patch
># User Jeff Walden <jwalden@mit.edu>
># Date 1277925101 25200
># Node ID 48df17e99d8632f4e6a3fd58a32e9ec78875bf4e
># Parent  284cb62bbb72c525e7638c86fbfc64e904410883
>Implement Object.isSealed, Object.isFrozen, Object.seal, Object.freeze.  NOT REVIEWED YET
>
>diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
>--- a/js/src/jsapi.cpp
>+++ b/js/src/jsapi.cpp
>@@ -2928,6 +2928,12 @@ JS_NewObjectForConstructor(JSContext *cx
> }
> 
> JS_PUBLIC_API(JSBool)
>+JS_IsExtensible(JSObject *obj)
>+{
>+    return obj->isExtensible();
>+}
>+
>+JS_PUBLIC_API(JSBool)
> JS_FreezeObject(JSContext *cx, JSObject *obj)
> {
>     CHECK_REQUEST(cx);
>diff --git a/js/src/jsapi.h b/js/src/jsapi.h
>--- a/js/src/jsapi.h
>+++ b/js/src/jsapi.h
>@@ -1887,6 +1887,10 @@ extern JS_PUBLIC_API(JSObject *)
> JS_NewObjectWithGivenProto(JSContext *cx, JSClass *clasp, JSObject *proto,
>                            JSObject *parent);
> 
>+/* Queries the [[Extensible]] property of the object. */
>+extern JS_PUBLIC_API(JSBool)
>+JS_IsExtensible(JSObject *obj);
>+
> /*
>  * Equivalent to Object.freeze (called recursively, if needed).  Deprecated
>  * as this previously-existing name is a clear false cognate of ES5's
>diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp
>--- a/js/src/jsobj.cpp
>+++ b/js/src/jsobj.cpp
>@@ -2560,7 +2560,7 @@ JSObject::modifyAllPropertyAttributes(JS
>                 return false;
> 
>             if ((attrs & JSPROP_PERMANENT) &&
>-                (!(attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
>+                ((attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
>                 continue;
>             }
> 

Oops, missed this -- it looks like a bug in the patch for bug 492849, so should go in a new patch for that bug, not this one.

>+static JSBool
>+obj_seal(JSContext *cx, uintN argc, Value *vp)
>+{
>+    JSObject *obj;
>+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.seal", &obj))
>+        return false;
>+    vp->setObject(*obj);
>+
>+    return obj->seal(cx);

Nit: simplest style for control flow like this would put the blank line after the return false; line, not after the vp->setObject(*obj).

>+obj_freeze(JSContext *cx, uintN argc, Value *vp)
>+{
>+    JSObject *obj;
>+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.freeze", &obj))
>+        return false;
>+    vp->setObject(*obj);
>+
>+    return obj->freeze(cx);

Ditto.

Looks great otherwise -- r=me with fixes.

/be
Attachment #473779 - Flags: review?(brendan) → review+
Assignee: jwalden+bmo → jim
(In reply to comment #6)
> >             if ((attrs & JSPROP_PERMANENT) &&
> >-                (!(attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
> >+                ((attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
> >                 continue;
> >             }
> > 
> 
> Oops, missed this -- it looks like a bug in the patch for bug 492849, so should
> go in a new patch for that bug, not this one.

Already noticed and fixed in the patch for that bug.

> >+static JSBool
> >+obj_seal(JSContext *cx, uintN argc, Value *vp)
> >+{
> >+    JSObject *obj;
> >+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.seal", &obj))
> >+        return false;
> >+    vp->setObject(*obj);
> >+
> >+    return obj->seal(cx);
> 
> Nit: simplest style for control flow like this would put the blank line after
> the return false; line, not after the vp->setObject(*obj).

Fixed.
http://hg.mozilla.org/tracemonkey/rev/441f83a81fb8
Status: NEW → ASSIGNED
Whiteboard: [fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/441f83a81fb8
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 613452
You need to log in before you can comment on or make changes to this bug.