Merge pull request #1507 from svaarala/boundfunc-cleanups

Misc follow-ups to duk_hboundfunc change

RELEASES.rst

@@ -2866,7 +2866,7 @@ Planned
 
 * Add an internal type for representing bound functions (duk_hboundfunc) and
   "collapse" bound function chains so that the target of a duk_hboundfunc is
-  always a non-bound function (GH-1503)
+  always a non-bound function (GH-1503, GH-1507)
 
 * Make call stack and value stack limits configurable via config options
   (DUK_USE_CALLSTACK_LIMIT, DUK_USE_VALSTACK_LIMIT) (GH-1526)

doc/release-notes-v2-2.rst

@@ -9,7 +9,7 @@ Main changes in this release (see RELEASES.rst for full details):
 
 * TBD.
 
-Upgrading from Duktape 2.0
+Upgrading from Duktape 2.1
 ==========================
 
 No action (other than recompiling) should be needed for most users to upgrade
@@ -36,3 +36,9 @@ from Duktape v2.1.x.  Note the following:
   ``duk_def_prop()``.  The inherited getters can also be replaced if necessary.
   The intermediate prototype doesn't have a named global binding, but you can
   access it by reading the prototype of a pushed function.
+
+* The bound 'this', bound arguments, and target of a duk_hboundfunc are no
+  longer internal properties (but duk_hboundfunc struct members).  The 'this'
+  binding, target, and bound argument count are now visible as artificial
+  properties; the bound argument values are not visible in the debugger
+  protocol for now.

src-input/duk_api_internal.h

@@ -24,7 +24,7 @@ DUK_INTERNAL_DECL void duk_valstack_shrink_check_nothrow(duk_hthread *thr, duk_b
 
 DUK_INTERNAL_DECL void duk_copy_tvals_incref(duk_hthread *thr, duk_tval *tv_dst, duk_tval *tv_src, duk_size_t count);
 
-DUK_INTERNAL_DECL duk_tval *duk_create_gap(duk_context *ctx, duk_idx_t idx_base, duk_idx_t count);
+DUK_INTERNAL_DECL duk_tval *duk_reserve_gap(duk_context *ctx, duk_idx_t idx_base, duk_idx_t count);
 
 DUK_INTERNAL_DECL void duk_set_top_and_wipe(duk_context *ctx, duk_idx_t top, duk_idx_t idx_wipe_start);
 

src-input/duk_api_stack.c

@@ -1252,12 +1252,12 @@ DUK_EXTERNAL void duk_xcopymove_raw(duk_context *to_ctx, duk_context *from_ctx,
 	}
 }
 
-/* Internal helper: create a gap of 'count' elements at 'idx_base' and return a
+/* Internal helper: reserve a gap of 'count' elements at 'idx_base' and return a
  * pointer to the gap.  Values in the gap are garbage and MUST be initialized by
  * the caller before any side effects may occur.  The caller must ensure there's
  * enough stack reserve for 'count' values.
  */
-DUK_INTERNAL duk_tval *duk_create_gap(duk_context *ctx, duk_idx_t idx_base, duk_idx_t count) {
+DUK_INTERNAL duk_tval *duk_reserve_gap(duk_context *ctx, duk_idx_t idx_base, duk_idx_t count) {
 	duk_hthread *thr = (duk_hthread *) ctx;
 	duk_tval *tv_src;
 	duk_tval *tv_dst;

src-input/duk_bi_function.c

@@ -220,8 +220,7 @@ DUK_INTERNAL duk_ret_t duk_bi_reflect_construct(duk_context *ctx) {
 DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	duk_hthread *thr = (duk_hthread *) ctx;
 	duk_hboundfunc *h_bound;
-	duk_hobject *h_target;
+	duk_idx_t nargs;  /* bound args, not counting 'this' binding */
-	duk_idx_t nargs;
 	duk_idx_t bound_nargs;
 	duk_int_t bound_len;
 	duk_tval *tv_prevbound;
@@ -236,27 +235,25 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	/* Vararg function, careful arg handling, e.g. thisArg may not
 	 * be present.
 	 */
-	nargs = duk_get_top(ctx);  /* = 1 + arg count */
+	nargs = duk_get_top(ctx) - 1;  /* actual args, not counting 'this' binding */
-	if (nargs == 0) {
+	if (nargs < 0) {
-		duk_push_undefined(ctx);
 		nargs++;
+		duk_push_undefined(ctx);
 	}
-	DUK_ASSERT(nargs >= 1);
+	DUK_ASSERT(nargs >= 0);
 
 	/* Limit 'nargs' for bound functions to guarantee arithmetic
 	 * below will never wrap.
 	 */
-	if (nargs - 1 > (duk_idx_t) DUK_HBOUNDFUNC_MAX_ARGS) {
+	if (nargs > (duk_idx_t) DUK_HBOUNDFUNC_MAX_ARGS) {
 		DUK_DCERROR_RANGE_INVALID_COUNT(thr);
 	}
 
 	duk_push_this(ctx);
 	duk_require_callable(ctx, -1);
-	h_target = duk_get_hobject(ctx, -1);
-	/* h_target may be NULL for lightfuncs. */
 
-	/* [ thisArg arg1 ... argN func ]  (thisArg+args == nargs total) */
+	/* [ thisArg arg1 ... argN func ]  (thisArg+args == nargs+1 total) */
-	DUK_ASSERT_TOP(ctx, nargs + 1);
+	DUK_ASSERT_TOP(ctx, nargs + 2);
 
 	/* Create bound function object. */
 	h_bound = duk_push_hboundfunc(ctx);
@@ -275,14 +272,18 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	 */
 	tv_prevbound = NULL;
 	n_prevbound = 0;
-	tv_tmp = DUK_GET_TVAL_NEGIDX(ctx, -2);
-	DUK_TVAL_SET_TVAL(&h_bound->target, tv_tmp);
 	tv_tmp = DUK_GET_TVAL_POSIDX(ctx, 0);
 	DUK_TVAL_SET_TVAL(&h_bound->this_binding, tv_tmp);
+	tv_tmp = DUK_GET_TVAL_NEGIDX(ctx, -2);
+	DUK_TVAL_SET_TVAL(&h_bound->target, tv_tmp);
 
-	if (h_target != NULL) {
+	if (DUK_TVAL_IS_OBJECT(tv_tmp)) {
+		duk_hobject *h_target;
 		duk_hobject *bound_proto;
 
+		h_target = DUK_TVAL_GET_OBJECT(tv_tmp);
+		DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(h_target));
+
 		/* Internal prototype must be copied from the target.
 		 * For lightfuncs Function.prototype is used and is already
 		 * in place.
@@ -323,6 +324,7 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 		/* Lightfuncs are always strict. */
 		duk_hobject *bound_proto;
 
+		DUK_ASSERT(DUK_TVAL_IS_LIGHTFUNC(tv_tmp));
 		DUK_HOBJECT_SET_STRICT((duk_hobject *) h_bound);
 		bound_proto = thr->builtins[DUK_BIDX_FUNCTION_PROTOTYPE];
 		DUK_HOBJECT_SET_PROTOTYPE_INIT_INCREF(thr, (duk_hobject *) h_bound, bound_proto);
@@ -331,7 +333,7 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	DUK_TVAL_INCREF(thr, &h_bound->target);  /* old values undefined, no decref needed */
 	DUK_TVAL_INCREF(thr, &h_bound->this_binding);
 
-	bound_nargs = n_prevbound + (nargs - 1);
+	bound_nargs = n_prevbound + nargs;
 	if (bound_nargs > (duk_idx_t) DUK_HBOUNDFUNC_MAX_ARGS) {
 		DUK_DCERROR_RANGE_INVALID_COUNT(thr);
 	}
@@ -343,22 +345,29 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	h_bound->nargs = bound_nargs;
 
 	duk_copy_tvals_incref(thr, tv_res, tv_prevbound, n_prevbound);
-	duk_copy_tvals_incref(thr, tv_res + n_prevbound, DUK_GET_TVAL_POSIDX(ctx, 1), nargs - 1);
+	duk_copy_tvals_incref(thr, tv_res + n_prevbound, DUK_GET_TVAL_POSIDX(ctx, 1), nargs);
 
 	/* [ thisArg arg1 ... argN func boundFunc ] */
 
-	/* bound function 'length' property is interesting */
+	/* Bound function 'length' property is interesting.
-	bound_len = 0;
+	 * For lightfuncs, simply read the virtual property.
-	if (h_target == NULL ||  /* lightfunc */
+	 */
-	    DUK_HOBJECT_GET_CLASS_NUMBER(h_target) == DUK_HOBJECT_CLASS_FUNCTION) {
+	duk_get_prop_stridx_short(ctx, -2, DUK_STRIDX_LENGTH);
-		/* For lightfuncs, simply read the virtual property. */
+	bound_len = duk_get_int(ctx, -1);  /* ES2015: no coercion */
-		duk_int_t tmp;
+	if (bound_len < nargs) {
-		duk_get_prop_stridx_short(ctx, -2, DUK_STRIDX_LENGTH);
+		bound_len = 0;
-		tmp = duk_to_int(ctx, -1) - (nargs - 1);  /* step 15.a */
+	} else {
-		duk_pop(ctx);
+		bound_len -= nargs;
-		bound_len = (tmp >= 0 ? tmp : 0);
+	}
+	if (sizeof(duk_int_t) > 4 && bound_len > (duk_int_t) DUK_UINT32_MAX) {
+		bound_len = (duk_int_t) DUK_UINT32_MAX;
 	}
-	duk_push_int(ctx, bound_len);
+	duk_pop(ctx);
+	DUK_ASSERT(bound_len >= 0);
+	tv_tmp = thr->valstack_top++;
+	DUK_ASSERT(DUK_TVAL_IS_UNDEFINED(tv_tmp));
+	DUK_ASSERT(!DUK_TVAL_NEEDS_REFCOUNT_UPDATE(tv_tmp));
+	DUK_TVAL_SET_U32(tv_tmp, (duk_uint32_t) bound_len);  /* in-place update, fastint */
 	duk_xdef_prop_stridx_short(ctx, -2, DUK_STRIDX_LENGTH, DUK_PROPDESC_FLAGS_C);  /* attrs in E6 Section 9.2.4 */
 
 	/* XXX: could these be virtual? */

src-input/duk_debugger.c

@@ -2199,6 +2199,19 @@ DUK_LOCAL void duk__debug_handle_get_heap_obj_info(duk_hthread *thr, duk_heap *h
 			}
 		}
 
+		if (DUK_HOBJECT_IS_BOUNDFUNC(h_obj)) {
+			duk_hboundfunc *h_bfun;
+			h_bfun = (duk_hboundfunc *) h_obj;
+
+			duk__debug_getinfo_flags_key(thr, "target");
+			duk_debug_write_tval(thr, &h_bfun->target);
+			duk__debug_getinfo_flags_key(thr, "this_binding");
+			duk_debug_write_tval(thr, &h_bfun->this_binding);
+			duk__debug_getinfo_flags_key(thr, "nargs");
+			duk_debug_write_int(thr, h_bfun->nargs);
+			/* h_bfun->args not exposed now */
+		}
+
 		if (DUK_HOBJECT_IS_THREAD(h_obj)) {
 			/* XXX: Currently no inspection of threads, e.g. value stack, call
 			 * stack, catch stack, etc.

src-input/duk_js_call.c

@@ -546,7 +546,7 @@ DUK_LOCAL void duk__handle_bound_chain_for_call(duk_hthread *thr,
 
 			duk_require_stack(ctx, len);
 
-			tv_gap = duk_create_gap(ctx, idx_func + 2, len);
+			tv_gap = duk_reserve_gap(ctx, idx_func + 2, len);
 			duk_copy_tvals_incref(thr, tv_gap, tv_args, len);
 
 			/* [ ... func this <bound args> arg1 ... argN ] */

tests/ecmascript/test-bi-function-proto-apply-gaps.js

@@ -0,0 +1,33 @@
+/*
+ *  Gaps in .apply() args array.
+ */
+
+/*===
+undefined undefined
+undefined undefined
+undefined undefined
+string foo
+undefined undefined
+===*/
+
+function test() {
+    function f(a,b,c,d,e) {
+        print(typeof a, a);
+        print(typeof b, b);
+        print(typeof c, c);
+        print(typeof d, d);
+        print(typeof e, e);
+    }
+
+    var args = [];
+    args.length = 10;
+    args[3] = 'foo';
+
+    f.apply(null, args);
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

tests/ecmascript/test-bi-function-proto-bind-length.js

@@ -0,0 +1,30 @@
+/*===
+123
+0
+dummy
+0
+===*/
+
+function test() {
+    var f, g;
+
+    // In ES2015 .bind() doesn't coerce the length so '123' is treated as 0.
+    f = function f() {};
+    Object.defineProperty(f, 'length', { value: '123' });
+    print(f.length);
+    g = f.bind(null, 123);
+    print(g.length);
+
+    // Same for a string which isn't number like.
+    f = function f() {};
+    Object.defineProperty(f, 'length', { value: 'dummy' });
+    print(f.length);
+    g = f.bind(null, 123);
+    print(g.length);
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

tests/perf/test-misc-1dcell.js

@@ -0,0 +1,72 @@
+if (typeof print !== 'function') { print = console.log; }
+
+function evolve(input, rule) {
+    var res = [];
+    var i;
+    var bits;
+
+    for (i = 0; i < input.length; i++) {
+        bits = 0;
+        if (i - 1 >= 0) {
+            bits += input[i - 1] * 4;
+        }
+        if (i - 1 < input.length) {
+            bits += input[i + 1];
+        }
+        if (i - 2 >= 0) {
+            bits += input[i - 2] * 8;
+        }
+        if (i + 2 < input.length) {
+            bits += input[i + 2] * 16;
+        }
+        bits += input[i] * 2;
+
+        if ((1 << bits) & rule) {
+            res[i] = 0;
+        } else {
+            res[i] = 1;
+        }
+    }
+
+    res[(Math.random() * input.length) >>> 0] = (Math.random() > 0.5 ? 1 : 0);
+
+    return res;
+}
+
+function mapchar(v) {
+    return v ? ' ' : '#';
+}
+
+function dump(input) {
+    var line = '|' + input.map(mapchar).join('') + '|';
+    //print(line);
+}
+
+function test() {
+    var input;
+    var rulenum;
+    var round;
+
+    //rulenum = (Math.random() * 0x100000000) >>> 0;
+    rulenum = 0xdeadbeef;
+
+    input = [];
+    while (input.length < 512) {
+        input.push(1);
+    }
+    input[256] = 0;
+
+    for (round = 0; round < 1e4; round++) {
+        dump(input);
+        input = evolve(input, rulenum);
+    }
+
+    print('done');
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+    throw e;
+}