Last Comment Bug 492845 - ES5: Implement Object.seal, Object.isSealed
: ES5: Implement Object.seal, Object.isSealed
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Jim Blandy :jimb
:
Mentors:
: 563205 (view as bug list)
Depends on: 558451 599464 600132 600135 613452 624421 624426
Blocks: 430133 es5
  Show dependency treegraph
 
Reported: 2009-05-13 13:42 PDT by Jim Blandy :jimb
Modified: 2011-01-18 23:24 PST (History)
25 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta7+


Attachments
Patch (8.38 KB, patch)
2010-08-31 12:53 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Review
Rebased (8.38 KB, patch)
2010-09-09 15:34 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
brendan: review+
Details | Diff | Review

Description Jim Blandy :jimb 2009-05-13 13:42:32 PDT
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
Comment 1 Brendan Eich [:brendan] 2010-05-01 21:14:08 PDT
*** Bug 563205 has been marked as a duplicate of this bug. ***
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2010-08-18 15:25:20 PDT
This bug depends on Brendan's scope-removal work in bug 558451.
Comment 3 Brendan Eich [:brendan] 2010-08-18 16:09:25 PDT
See bug 492849 comment 9.

/be
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2010-08-31 12:53:53 PDT
Created attachment 470866 [details] [diff] [review]
Patch

This goes atop bug 492849 (extensibility bug), implements both seal and freeze primitives.  Same time/reviewer caveats apply as with that bug/patch.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2010-09-09 15:34:44 PDT
Created attachment 473779 [details] [diff] [review]
Rebased
Comment 6 Brendan Eich [:brendan] 2010-09-09 17:54:18 PDT
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
Comment 7 Jim Blandy :jimb 2010-09-17 00:17:05 PDT
(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.
Comment 8 Jim Blandy :jimb 2010-09-21 11:43:04 PDT
http://hg.mozilla.org/tracemonkey/rev/441f83a81fb8

Note You need to log in before you can comment on or make changes to this bug.