Last Comment Bug 492840 - ES5: Implement Object.create
: ES5: Implement Object.create
Status: RESOLVED FIXED
fixed-in-tracemonkey
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on: 430133
Blocks: es5
  Show dependency treegraph
 
Reported: 2009-05-13 13:37 PDT by Jim Blandy :jimb
Modified: 2010-08-18 15:38 PDT (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch atop latest in bug 430133 (5.42 KB, patch)
2009-09-23 18:42 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Give a better error message for Object.create(2), one other uber-nit (6.59 KB, patch)
2009-09-25 14:42 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Updated atop latest patch in bug 430133, add another test or two (6.84 KB, patch)
2009-09-25 20:53 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
mrbkap: review+
Details | Diff | Splinter Review
Unbitrotted, possibly with previous comments addressed, atop and waiting upon bug 430133 (6.95 KB, patch)
2009-12-10 17:36 PST, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review

Description Jim Blandy :jimb 2009-05-13 13:37:28 PDT
Object.create is specified by 15.2.3.5 in the ES5 draft:
http://wiki.ecmascript.org/doku.php?id=es3.1:es3.1_proposal_working_draft
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-23 18:42:02 PDT
Created attachment 402512 [details] [diff] [review]
Patch atop latest in bug 430133

This doesn't provide any new functionality, but newborn-proto-setting capability is an important step toward removing __proto__ support at some (likely distant) time in the future.
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-25 14:42:16 PDT
Created attachment 402921 [details] [diff] [review]
Give a better error message for Object.create(2), one other uber-nit
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-25 20:53:01 PDT
Created attachment 402997 [details] [diff] [review]
Updated atop latest patch in bug 430133, add another test or two
Comment 4 Bob Clary [:bc:] 2009-09-27 09:28:25 PDT
Waldo, please fix the test to call reportCompare() and update ecma_5/Object/jstests.list to remove the skip. Thanks.
Comment 5 Blake Kaplan (:mrbkap) 2009-09-30 13:00:51 PDT
Comment on attachment 402997 [details] [diff] [review]
Updated atop latest patch in bug 430133, add another test or two

>+MSG_DEF(JSMSG_NOT_OBJECT_OR_NULL,     244, 0, JSEXN_TYPEERR, "value is not null or an object")

You can use JSMSG_UNEXPECTED_TYPE with type being "null or an object" instead of adding a new message here.

>diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp
>--- a/js/src/jsobj.cpp
>+++ b/js/src/jsobj.cpp
>@@ -2673,16 +2673,80 @@ obj_defineProperties(JSContext* cx, uint
>     for (size_t i = 0; i < len; i++) {
>         if (!DefineProperty(cx, obj, (*descs)[i], true, dummy))
>             return JS_FALSE;
>     }
> 
>     return JS_TRUE;
> }
> 
>+/* ES5 15.2.3.5: Object.create(O [, Properties]) */
>+static JSBool
>+obj_create(JSContext *cx, uintN argc, jsval *vp)
>+{
>+    jsval v = argc == 0 ? JSVAL_VOID : vp[2];

Nit: parentheses around argc == 0.

>+    if (argc < 1 || !JSVAL_IS_OBJECT(v)) {

The argc < 1 in this check is redundant with the above assignment (especially since we assign a non-object to v if argc is less than 1). I guess it's a little tricky, but I'd say to remove the redundant test.

>+    /*
>+     * Notwithstanding that Object.create seems more likely to be in run-once
>+     * code than performance-critical code, JS_GetScopeChain(cx) is going to
>+     * throw us off trace -- is there an alternative?

Hmm, so js_NewObjectWithGivenProto will use the parent of the prototype (if it's not null) if parent is null. You could use that if the given prototype is non-null. I don't know what we want the lexical scope of these objects to be. Parented to the prototype object's parent (which may be a different lexical scope)? Always parented to the static scope of Object.create? I assume the spec is silent on this issue because it doesn't address the possibility of multiple global objects...
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-30 16:54:41 PDT
(reversing dependency on Object.defineProperty, whose patch this patch depends on)
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2009-12-10 17:36:04 PST
Created attachment 417018 [details] [diff] [review]
Unbitrotted, possibly with previous comments addressed, atop and waiting upon bug 430133
Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2010-01-11 11:34:15 PST
I wrote up a little bit here, but we certainly need to make the information more visible than simply buried in the JS reference:

https://developer.mozilla.org/En/Core_JavaScript_1.5_Reference/Global_Objects/Object/create

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