331429 - Implement an apply that can be composed with new

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P4)

Description

[Blake Kaplan (:mrbkap) (inactive)]

In Narcissus, we currently have this absolutely awful switch statement in Function.prototype.__construct__ to get around the lack of something like apply/call for construction. It seems like it would be cleaner and easier to just implement the special semantics around |new| (e.g., creating a new object with the proper prototype, etc.) and use __call__ to finish it off (like global.Function.__construct__).

If this won't work for some reason that I'm not seeing, then we should implement a native __construct__ type function that does what apply or call do for regular calls for construction.

Comment 1

[Brendan Eich [:brendan]]

__construct__ ain't __call__.  See Date and Array, oh and one other I forget all the time.

/be

Comment 2

[Brendan Eich [:brendan]]

I've copped to this before: what JS really wants is an apply operator that can be composed with new.  So you could 'apply new C(obj, args)' and get the equivalent of 'new C(args[0], args[1], ...)' where obj is the newly created object of C's class, and args is an array or arguments object.

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Right, those and RegExp. The core problem is (of course) that Narcissus can't control whether fp->flags has the JSFRAME_CONSTRUCTING bit set, that's the core of the problem. I don't seem to recall there being a proposal that covers this for ECMAScript v4, but for Narcissus' purposes, instead of adding a new keyword (apply), why don't we add Function.prototype.__construct__ (Narcissus only) that does the Right Thing.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: First pass at spidermonkey changes

This implements Function.prototype.__applyConstruct__.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Narcissus changes

...using __applyConstruct__...

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Second pass at SpiderMonkey changes

These are mainly cosmetic changes.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 218585 [details] [diff] [review]
Narcissus changes

With your nit-picky renaming :-P.

/be

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 218732 [details] [diff] [review]
Second pass at SpiderMonkey changes

> JSBool
>+js_InvokeConstructor(JSContext *cx, uintN argc)

Please define this right after js_StrictlyEqual's definition.

>+{
>+    jsval *vp;
>+    JSFunction *fun;
>+    JSObject *obj, *obj2, *proto, *parent;
>+    JSClass *clasp, *funclasp;
>+    jsval lval, rval;

Nit: swap last two lines to match first-def order.

>+    vp = cx->fp->sp - (2 + argc);

D'oh, how about making vp a param, so the common cases (JSOP_NEW/JSOP_NEWINIT) don't have to pay a gratuitous recomputation cost?  Make the odd case (fun_applyConstructor) pay (once only in that path too).

>+#ifdef NARCISSUS
>+static JSBool
>+fun_applyConstructor(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
>+                     jsval *rval)
>+{
>+    JSObject *args;
>+    uintN len, i;

Minor whining about deviation from aobj/length in fun_apply :-P.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Updated

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 218740 [details] [diff] [review]
Updated


>+    /* Check the return value and update obj from it. */
>+    rval = *vp;
>+    if (JSVAL_IS_PRIMITIVE(rval)) {
>+        if (!fun && JS_VERSION_IS_ECMA(cx)) {
>+            /* native [[Construct]] returning primitive is error */
>+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                                 JSMSG_BAD_NEW_RESULT,
>+                                 js_ValueToPrintableString(cx, rval));
>+            return JS_FALSE;
>+        }
>+        *vp = OBJECT_TO_JSVAL(obj);
>+    } else {
>+        obj = JSVAL_TO_OBJECT(rval);
>+    }

No need for else clause here (also change comment at top of this cited hunk to say "Check the return value and if primitive, force it to be obj" or some such).

Fix that, and r=me -- thanks.

/be

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Checked in.