Njs: bound constructors

[drsm]

https://tc39.es/ecma262/#sec-bound-function-exotic-objects-construct-argumentslist-newtarget

njs:

>> var f = function(a, b, c) { this.a = a; this.b = b; this.c = c; }
undefined
>> var bf = f.bind(null, 1, 2, 3);
undefined
>> var o = new bf()
TypeError: property set on primitive null type
    at anonymous (shell:1)
    at main (native)

node:

> var f = function(a, b, c) { this.a = a; this.b = b; this.c = c; }
undefined
> var bf = f.bind(null, 1, 2, 3);
undefined
> var o = new bf()
undefined
> o
f { a: 1, b: 2, c: 3 }
// https://tc39.es/ecma262/#sec-ordinaryhasinstance
> (o instanceof f) && (o instanceof bf)
true
// https://tc39.es/ecma262/#sec-function-instances-prototype (note)
> typeof bf.prototype
'undefined'

[xeioex]

@drsm

Take a looks:

# HG changeset patch
# User Dmitry Volyntsev <[email protected]>
# Date 1573740634 -10800
#      Thu Nov 14 17:10:34 2019 +0300
# Node ID 911b4b9feafc8b267d4cd3d87a9ec055b46b43a5
# Parent  6b33cb8b6df6110e081db6ba6739bbcd90ffd9eb
Reimplemented "bound" functions according to specification.

This closes #202 and closes #204 issues on Github.

diff --git a/src/njs_builtin.c b/src/njs_builtin.c
--- a/src/njs_builtin.c
+++ b/src/njs_builtin.c
@@ -771,6 +771,10 @@ njs_builtin_match_native_function(njs_vm
     njs_lvlhsh_each_t       lhe;
     njs_builtin_traverse_t  ctx;

+    if (function->bound != NULL) {
+        function = function->u.bound_target;
+    }
+
     ctx.type = NJS_BUILTIN_TRAVERSE_MATCH;
     ctx.native = function->u.native;

diff --git a/src/njs_function.c b/src/njs_function.c
--- a/src/njs_function.c
+++ b/src/njs_function.c
@@ -111,6 +111,54 @@ njs_function_closures(njs_vm_t *vm, njs_
 }


+njs_int_t
+njs_function_name_set(njs_vm_t *vm, njs_function_t *function,
+    njs_value_t *name, njs_bool_t bound)
+{
+    u_char              *start;
+    njs_int_t           ret;
+    njs_string_prop_t   string;
+    njs_object_prop_t   *prop;
+    njs_lvlhsh_query_t  lhq;
+
+    prop = njs_object_prop_alloc(vm, &njs_string_name, name, 0);
+    if (njs_slow_path(name == NULL)) {
+        return NJS_ERROR;
+    }
+
+    if (bound) {
+        (void) njs_string_prop(&string, name);
+        string.length += njs_length("bound ");
+
+        start = njs_string_alloc(vm, &prop->value, string.size + 6,
+                                 string.length);
+        if (njs_slow_path(start == NULL)) {
+            return NJS_ERROR;
+        }
+
+        start = njs_cpymem(start, "bound ", 6);
+        memcpy(start, string.start, string.size);
+    }
+
+    prop->configurable = 1;
+
+    lhq.key_hash = NJS_NAME_HASH;
+    lhq.key = njs_str_value("name");
+    lhq.replace = 0;
+    lhq.value = prop;
+    lhq.proto = &njs_object_hash_proto;
+    lhq.pool = vm->mem_pool;
+
+    ret = njs_lvlhsh_insert(&function->object.hash, &lhq);
+    if (njs_slow_path(ret != NJS_OK)) {
+        njs_internal_error(vm, "lvlhsh insert failed");
+        return NJS_ERROR;
+    }
+
+    return NJS_OK;
+}
+
+
 static njs_function_t *
 njs_function_copy(njs_vm_t *vm, njs_function_t *function)
 {
@@ -347,10 +395,17 @@ njs_function_lambda_frame(njs_vm_t *vm, 
     njs_uint_t             n, max_args, closures;
     njs_value_t            *value, *bound;
     njs_frame_t            *frame;
+    njs_function_t         *target;
     njs_native_frame_t     *native_frame;
     njs_function_lambda_t  *lambda;

-    lambda = function->u.lambda;
+    if (njs_fast_path(function->bound == NULL)) {
+        lambda = function->u.lambda;
+
+    } else {
+        target = function->u.bound_target;
+        lambda = target->u.lambda;
+    }

     max_args = njs_max(nargs, lambda->nargs);

@@ -384,10 +439,16 @@ njs_function_lambda_frame(njs_vm_t *vm, 
         n = function->args_offset;
         native_frame->nargs += n - 1;

-        do {
+        if (ctor) {
+            *value++ = *this;
+            bound++;
+            n--;
+        }
+
+        while (n != 0) {
             *value++ = *bound++;
             n--;
-        } while (n != 0);
+        };
     }

     vm->scopes[NJS_SCOPE_CALLEE_ARGUMENTS] = value;
@@ -497,13 +558,19 @@ njs_function_lambda_call(njs_vm_t *vm)
     njs_frame_t            *frame;
     njs_value_t            *dst, *src;
     njs_closure_t          *closure, **closures;
-    njs_function_t         *function;
+    njs_function_t         *function, *target;
     njs_function_lambda_t  *lambda;

     frame = (njs_frame_t *) vm->top_frame;
     function = frame->native.function;

-    lambda = function->u.lambda;
+    if (njs_fast_path(function->bound == NULL)) {
+        lambda = function->u.lambda;
+
+    } else {
+        target = function->u.bound_target;
+        lambda = target->u.lambda;
+    }

 #if (NJS_DEBUG)
     vm->scopes[NJS_SCOPE_CALLEE_ARGUMENTS] = NULL;
@@ -580,18 +647,26 @@ njs_function_lambda_call(njs_vm_t *vm)
 njs_int_t
 njs_function_native_call(njs_vm_t *vm)
 {
-    njs_int_t           ret;
-    njs_value_t         *value;
-    njs_frame_t         *frame;
-    njs_function_t      *function;
-    njs_native_frame_t  *native, *previous;
+    njs_int_t              ret;
+    njs_value_t            *value;
+    njs_frame_t            *frame;
+    njs_function_t         *function, *target;
+    njs_native_frame_t     *native, *previous;
+    njs_function_native_t  call;

     native = vm->top_frame;
     frame = (njs_frame_t *) native;
     function = native->function;

-    ret = function->u.native(vm, native->arguments, native->nargs,
-                             function->magic);
+    if (njs_fast_path(function->bound == NULL)) {
+        call = function->u.native;
+
+    } else {
+        target = function->u.bound_target;
+        call = target->u.native;
+    }
+
+    ret = call(vm, native->arguments, native->nargs, function->magic);
     if (njs_slow_path(ret == NJS_ERROR)) {
         return ret;
     }
@@ -1044,9 +1119,11 @@ static njs_int_t
 njs_function_prototype_bind(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused)
 {
-    size_t          size;
-    njs_value_t     *values;
-    njs_function_t  *function;
+    size_t              size;
+    njs_int_t           ret;
+    njs_value_t         *values, name;
+    njs_function_t      *function;
+    njs_lvlhsh_query_t  lhq;

     if (!njs_is_function(&args[0])) {
         njs_type_error(vm, "\"this\" argument is not a function");
@@ -1059,6 +1136,29 @@ njs_function_prototype_bind(njs_vm_t *vm
         return NJS_ERROR;
     }

+    njs_object_property_init(&lhq, "name", NJS_NAME_HASH);
+
+    ret = njs_object_property(vm, &args[0], &lhq, &name);
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
+
+    if (!njs_is_string(&name)) {
+        name = njs_string_empty;
+    }
+
+    njs_lvlhsh_init(&function->object.hash);
+
+    ret = njs_function_name_set(vm, function, &name, 1);
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
+
+    /* Bound functions have no "prototype" property. */
+    function->object.shared_hash = vm->shared->arrow_instance_hash;
+
+    function->u.bound_target = njs_function(&args[0]);
+
     if (nargs == 1) {
         args = njs_value_arg(&njs_value_undefined);

diff --git a/src/njs_function.h b/src/njs_function.h
--- a/src/njs_function.h
+++ b/src/njs_function.h
@@ -101,6 +101,8 @@ struct njs_frame_s {
 njs_function_t *njs_function_alloc(njs_vm_t *vm, njs_function_lambda_t *lambda,
     njs_closure_t *closures[], njs_bool_t shared);
 njs_function_t *njs_function_value_copy(njs_vm_t *vm, njs_value_t *value);
+njs_int_t njs_function_name_set(njs_vm_t *vm, njs_function_t *function,
+    njs_value_t *name, njs_bool_t bound);
 njs_int_t njs_function_arguments_object_init(njs_vm_t *vm,
     njs_native_frame_t *frame);
 njs_int_t njs_function_rest_parameters_init(njs_vm_t *vm,
diff --git a/src/njs_object_prop.c b/src/njs_object_prop.c
--- a/src/njs_object_prop.c
+++ b/src/njs_object_prop.c
@@ -376,10 +376,9 @@ exception:
 njs_int_t
 njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq)
 {
-    njs_int_t           ret;
-    njs_function_t      *function;
-    njs_object_prop_t   *prop, *shared, *name;
-    njs_lvlhsh_query_t  lhq;
+    njs_int_t          ret;
+    njs_function_t     *function;
+    njs_object_prop_t  *prop, *shared;

     prop = njs_mp_align(vm->mem_pool, sizeof(njs_value_t),
                         sizeof(njs_object_prop_t));
@@ -436,27 +435,7 @@ njs_prop_private_copy(njs_vm_t *vm, njs_
         return NJS_ERROR;
     }

-    name = njs_object_prop_alloc(vm, &njs_string_name, &prop->name, 0);
-    if (njs_slow_path(name == NULL)) {
-        return NJS_ERROR;
-    }
-
-    name->configurable = 1;
-
-    lhq.key_hash = NJS_NAME_HASH;
-    lhq.key = njs_str_value("name");
-    lhq.replace = 0;
-    lhq.value = name;
-    lhq.proto = &njs_object_hash_proto;
-    lhq.pool = vm->mem_pool;
-
-    ret = njs_lvlhsh_insert(&function->object.hash, &lhq);
-    if (njs_slow_path(ret != NJS_OK)) {
-        njs_internal_error(vm, "lvlhsh insert failed");
-        return NJS_ERROR;
-    }
-
-    return NJS_OK;
+    return njs_function_name_set(vm, function, &prop->name, 0);
 }


diff --git a/src/njs_value.h b/src/njs_value.h
--- a/src/njs_value.h
+++ b/src/njs_value.h
@@ -245,6 +245,7 @@ struct njs_function_s {
     union {
         njs_function_lambda_t         *lambda;
         njs_function_native_t         native;
+        njs_function_t                *bound_target;
     } u;

     njs_value_t                       *bound;
diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -1328,22 +1328,30 @@ static njs_jump_off_t
 njs_vmcode_instance_of(njs_vm_t *vm, njs_value_t *object,
     njs_value_t *constructor)
 {
-    njs_value_t        value;
+    njs_value_t        value, bound;
     njs_object_t       *prototype, *proto;
+    njs_function_t     *function;
     njs_jump_off_t     ret;
     const njs_value_t  *retval;

-    const njs_value_t prototype_string = njs_string("prototype");
+    static const njs_value_t prototype_string = njs_string("prototype");

     if (!njs_is_function(constructor)) {
         njs_type_error(vm, "right argument is not callable");
         return NJS_ERROR;
     }

+    function = njs_function(constructor);
+
+    if (function->bound != NULL) {
+        function = function->u.bound_target;
+        njs_set_function(&bound, function);
+        constructor = &bound;
+    }
+
     retval = &njs_value_false;

     if (njs_is_object(object)) {
-        njs_set_undefined(&value);
         ret = njs_value_property(vm, constructor,
                                  njs_value_arg(&prototype_string), &value);

@@ -1607,8 +1615,9 @@ njs_function_frame_create(njs_vm_t *vm, 
 static njs_object_t *
 njs_function_new_object(njs_vm_t *vm, njs_value_t *constructor)
 {
-    njs_value_t     proto;
+    njs_value_t     proto, bound;
     njs_object_t    *object;
+    njs_function_t  *function;
     njs_jump_off_t  ret;

     const njs_value_t prototype_string = njs_string("prototype");
@@ -1618,6 +1627,13 @@ njs_function_new_object(njs_vm_t *vm, nj
         return NULL;
     }

+    function = njs_function(constructor);
+    if (function->bound != NULL) {
+        function = function->u.bound_target;
+        njs_set_function(&bound, function);
+        constructor = &bound;
+    }
+
     ret = njs_value_property(vm, constructor, njs_value_arg(&prototype_string),
                              &proto);

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -5528,6 +5528,46 @@ static njs_unit_test_t  njs_test[] =
                  "var o = { toString: f }; o"),
       njs_str("0,1,2") },

+    { njs_str("var f = Object.defineProperty(function() {}, 'name', {value: 'F'});"
+              "[f.name, f.bind().name, f.bind().bind().name]"),
+      njs_str("F,bound F,bound bound F") },
+
+    { njs_str("var f = Object.defineProperty(function() {}, 'name', {value: undefined});"
+              "[f.name, f.bind().name, f.bind().bind().name]"),
+      njs_str(",bound ,bound bound ") },
+
+    { njs_str("var f = Object.defineProperty(function() {}, 'name', {get:()=>'F'});"
+              "[f.name, f.bind().name]"),
+      njs_str("F,bound F") },
+
+    { njs_str("var f = Object.defineProperty(function() {}, 'name', {get:()=>{throw Error('Oops')}});"
+              "f.bind().name"),
+      njs_str("Error: Oops") },
+
+    { njs_str("var f = function() {}; f.a = 'a'; [f.bind().a, f.a]"),
+      njs_str(",a") },
+
+    { njs_str("var f = function() {}; var bf = f.bind(); bf.b = 'b'; "
+              "[f.b, bf.b]"),
+      njs_str(",b") },
+
+    { njs_str("function f(x,y) {return {args:arguments,length:arguments.length}};"
+              "var nf = Function.prototype.bind.call(f, {}, 'a', 'b');"
+              "var o = new nf();[o.length, o.args[0]]"),
+      njs_str("2,a") },
+
+    { njs_str("function f(x,y) {return {args:arguments,length:arguments.length}};"
+              "var nf = Function.prototype.bind.call(f, {});"
+              "var o = new nf('a', 'b');[o.length, o.args[0]]"),
+      njs_str("2,a") },
+
+    { njs_str("var f = function(a,b) { this.a = a; this.b = b; };"
+              "f.prototype.X = 'X';"
+              "var bf = f.bind(null, 1,2);"
+              "var o = new bf(); "
+              "[Object.keys(o), o.X,(o instanceof f) && (o instanceof bf),bf.prototype]"),
+      njs_str("a,b,X,true,") },
+
     { njs_str("var s = { toString: function() { return '123' } };"
                  "var a = 'abc'; a.concat('абв', s)"),
       njs_str("abcабв123") },

[drsm]

@xeioex
Looks good to me!

[xeioex]

@drsm BTW, chains of bound calls still does not work as expected.

+#if 0 /* FIMXE: refactor Bound calls. */
+    { njs_str("function f(x,y) {return {args:arguments,length:arguments.length}};"
+              "var bf = f.bind({}, 'a'); var bbf = bf.bind({},'b'); var o = bbf('c');"),
+              "[o.args[0], o.args[2], o.length]"
+      njs_str("a,c,3") },
+#endif

But I think this should be another issue.