Omit _Formals from duk_hcompfunc when possible

svaarala · svaarala · commit 0d36379451ef · 2016-12-05T00:36:41.000+02:00
diff --git a/src-input/duk_api_bytecode.c b/src-input/duk_api_bytecode.c
@@ -205,12 +205,15 @@ DUK_LOCAL duk_uint8_t *duk__dump_formals(duk_hthread *thr, duk_uint8_t *p, duk_b
 				 */
 				varname = DUK_TVAL_GET_STRING(tv_val);
 				DUK_ASSERT(varname != NULL);
+				DUK_ASSERT(DUK_HSTRING_GET_BYTELEN(varname) >= 1);  /* won't be confused with terminator */
 
 				DUK_ASSERT(DUK_HSTRING_MAX_BYTELEN <= 0x7fffffffUL);  /* ensures no overflow */
 				p = DUK_BW_ENSURE_RAW(thr, bw_ctx, 4 + DUK_HSTRING_GET_BYTELEN(varname), p);
 				p = duk__dump_hstring_raw(p, varname);
 			}
 		}
+	} else {
+		DUK_DD(DUK_DDPRINT("dumping function without _Formals, emit empty list"));
 	}
 	p = DUK_BW_ENSURE_RAW(thr, bw_ctx, 4, p);
 	DUK_RAW_WRITE_U32_BE(p, 0);  /* end of _Formals */
@@ -625,6 +628,11 @@ static duk_uint8_t *duk__load_func(duk_context *ctx, duk_uint8_t *p, duk_uint8_t
 	duk_compact(ctx, -1);
 	duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_VARMAP, DUK_PROPDESC_FLAGS_NONE);
 
+	/* If _Formals wasn't present in the original function, the list
+	 * here will be empty.  Same happens if _Formals was present but
+	 * had zero length.  We can omit _Formals from the result if its
+	 * length is zero and matches nargs.
+	 */
 	duk_push_array(ctx);  /* _Formals */
 	for (arr_idx = 0; ; arr_idx++) {
 		/* XXX: awkward */
@@ -635,8 +643,12 @@ static duk_uint8_t *duk__load_func(duk_context *ctx, duk_uint8_t *p, duk_uint8_t
 		}
 		duk_put_prop_index(ctx, -2, arr_idx);
 	}
-	duk_compact(ctx, -1);
-	duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_FORMALS, DUK_PROPDESC_FLAGS_NONE);
+	if (arr_idx == 0 && h_fun->nargs == 0) {
+		duk_pop(ctx);
+	} else {
+		duk_compact(ctx, -1);
+		duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_FORMALS, DUK_PROPDESC_FLAGS_NONE);
+	}
 
 	/* Return with final function pushed on stack top. */
 	DUK_DD(DUK_DDPRINT("final loaded function: %!iT", duk_get_tval(ctx, -1)));
diff --git a/src-input/duk_js_call.c b/src-input/duk_js_call.c
@@ -144,11 +144,16 @@ DUK_LOCAL void duk__create_arguments_object(duk_hthread *thr,
 	duk_push_hobject(ctx, func);
 	duk_get_prop_stridx(ctx, -1, DUK_STRIDX_INT_FORMALS);
 	formals = duk_get_hobject(ctx, -1);
-	n_formals = 0;
 	if (formals) {
-		duk_get_prop_stridx(ctx, -1, DUK_STRIDX_LENGTH);
-		n_formals = (duk_idx_t) duk_require_int(ctx, -1);
-		duk_pop(ctx);
+		n_formals = (duk_idx_t) duk_get_length(ctx, -1);
+	} else {
+		/* This shouldn't happen without tampering of internal
+		 * properties: if a function accesses 'arguments', _Formals
+		 * is kept.  Check for the case anyway in case internal
+		 * properties have been modified manually.
+		 */
+		DUK_D(DUK_DPRINT("_Formals is undefined when creating arguments, use n_formals == 0"));
+		n_formals = 0;
 	}
 	duk_remove(ctx, -2);  /* leave formals on stack for later use */
 	i_formals = duk_require_top_index(ctx);
diff --git a/src-input/duk_js_compiler.c b/src-input/duk_js_compiler.c
@@ -658,6 +658,8 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
 	duk_instr_t *p_instr;
 	duk_compiler_instr *q_instr;
 	duk_tval *tv;
+	duk_bool_t keep_varmap;
+	duk_bool_t keep_formals;
 
 	DUK_DDD(DUK_DDDPRINT("converting duk_compiler_func to function/template"));
 
@@ -791,6 +793,25 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
 
 	duk_pop(ctx);  /* 'data' (and everything in it) is reachable through h_res now */
 
+	/*
+	 *  Init non-property result fields
+	 *
+	 *  'nregs' controls how large a register frame is allocated.
+	 *
+	 *  'nargs' controls how many formal arguments are written to registers:
+	 *  r0, ... r(nargs-1).  The remaining registers are initialized to
+	 *  undefined.
+	 */
+
+	DUK_ASSERT(func->temp_max >= 0);
+	h_res->nregs = (duk_uint16_t) func->temp_max;
+	h_res->nargs = (duk_uint16_t) duk_hobject_get_length(thr, func->h_argnames);
+	DUK_ASSERT(h_res->nregs >= h_res->nargs);  /* pass2 allocation handles this */
+#if defined(DUK_USE_DEBUGGER_SUPPORT)
+	h_res->start_line = (duk_uint32_t) func->min_line;
+	h_res->end_line = (duk_uint32_t) func->max_line;
+#endif
+
 	/*
 	 *  Init object properties
 	 *
@@ -807,14 +828,20 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
 	 * after a cleanup.  When debugging is enabled, we always need the varmap to
 	 * be able to lookup variables at any point.
 	 */
+
 #if defined(DUK_USE_DEBUGGER_SUPPORT)
-	if (1) {
+	DUK_DD(DUK_DDPRINT("keeping _Varmap because debugger support is enabled"));
+	keep_varmap = 1;
 #else
-	if (func->id_access_slow ||     /* directly uses slow accesses */
-	    func->may_direct_eval ||    /* may indirectly slow access through a direct eval */
-	    funcs_count > 0) {          /* has inner functions which may slow access (XXX: this can be optimized by looking at the inner functions) */
+	keep_varmap =
+	    func->id_access_slow ||   /* directly uses slow accesses */
+	    func->may_direct_eval ||  /* may indirectly slow access through a direct eval */
+	    funcs_count > 0;          /* has inner functions which may slow access (XXX: this can be optimized by looking at the inner functions) */
 #endif
+
+	if (keep_varmap) {
 		duk_int_t num_used;
+		DUK_DD(DUK_DDPRINT("keeping _Varmap"));
 		duk_dup(ctx, func->varmap_idx);
 		num_used = duk__cleanup_varmap(comp_ctx);
 		DUK_DDD(DUK_DDDPRINT("cleaned up varmap: %!T (num_used=%ld)",
@@ -823,20 +850,35 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
 		if (num_used > 0) {
 			duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_VARMAP, DUK_PROPDESC_FLAGS_NONE);
 		} else {
-			DUK_DDD(DUK_DDDPRINT("varmap is empty after cleanup -> no need to add"));
+			DUK_DD(DUK_DDPRINT("varmap is empty after cleanup -> no need to add"));
 			duk_pop(ctx);
 		}
 	}
 
-	/* _Formals: omitted if function is guaranteed not to need a (non-strict) arguments object */
-	if (1) {
-		/* XXX: Add a proper condition.  If formals list is omitted, recheck
-		 * handling for 'length' in duk_js_push_closure(); it currently relies
-		 * on _Formals being set.  Removal may need to be conditional to debugging
-		 * being enabled/disabled too.
-		 */
+	/* _Formals: omitted if function is guaranteed not to need a (non-strict)
+	 * arguments object, and _Formals.length matches nargs exactly.
+	 *
+	 * Non-arrow functions can't see an outer function's 'argument' binding
+	 * (because they have their own), but arrow functions can.  When arrow
+	 * functions are added, this condition would need to be added:
+	 *     inner_arrow_funcs_count > 0   inner arrow functions may access 'arguments'
+	 */
+#if defined(DUK_USE_DEBUGGER_SUPPORT)
+	DUK_DD(DUK_DDPRINT("keeping _Formals because debugger support is enabled"));
+	keep_formals = 1;
+#else
+	keep_formals =
+	    func->id_access_arguments ||   /* accesses 'arguments', main reason to keep, could check strictness too */
+	    func->may_direct_eval ||       /* direct eval -> may access 'arguments' via eval */
+	    duk_get_length(ctx, func->argnames_idx) != (duk_size_t) h_res->nargs;  /* nargs not enough for closure .length */
+#endif
+
+	if (keep_formals) {
+		DUK_DD(DUK_DDPRINT("keeping _Formals"));
 		duk_dup(ctx, func->argnames_idx);
 		duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_INT_FORMALS, DUK_PROPDESC_FLAGS_NONE);
+	} else {
+		DUK_DD(DUK_DDPRINT("omitting _Formals, nargs matches _Formals.length, so no properties added"));
 	}
 
 	/* name */
@@ -920,25 +962,6 @@ DUK_LOCAL void duk__convert_to_func_template(duk_compiler_ctx *comp_ctx, duk_boo
 		duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_FILE_NAME, DUK_PROPDESC_FLAGS_NONE);
 	}
 
-	/*
-	 *  Init remaining result fields
-	 *
-	 *  'nregs' controls how large a register frame is allocated.
-	 *
-	 *  'nargs' controls how many formal arguments are written to registers:
-	 *  r0, ... r(nargs-1).  The remaining registers are initialized to
-	 *  undefined.
-	 */
-
-	DUK_ASSERT(func->temp_max >= 0);
-	h_res->nregs = (duk_uint16_t) func->temp_max;
-	h_res->nargs = (duk_uint16_t) duk_hobject_get_length(thr, func->h_argnames);
-	DUK_ASSERT(h_res->nregs >= h_res->nargs);  /* pass2 allocation handles this */
-#if defined(DUK_USE_DEBUGGER_SUPPORT)
-	h_res->start_line = (duk_uint32_t) func->min_line;
-	h_res->end_line = (duk_uint32_t) func->max_line;
-#endif
-
 	DUK_DD(DUK_DDPRINT("converted function: %!ixT",
 	                   (duk_tval *) duk_get_tval(ctx, -1)));
 
diff --git a/src-input/duk_js_var.c b/src-input/duk_js_var.c
@@ -337,24 +337,22 @@ void duk_js_push_closure(duk_hthread *thr,
 	/*
 	 *  "length" maps to number of formals (E5 Section 13.2) for function
 	 *  declarations/expressions (non-bound functions).  Note that 'nargs'
-	 *  is NOT necessarily equal to the number of arguments.
+	 *  is NOT necessarily equal to the number of arguments.  Use length
+	 *  of _Formals; if missing, assume nargs matches .length.
 	 */
 
 	/* [ ... closure template ] */
 
-	len_value = 0;
-
-	/* XXX: use helper for size optimization */
-	if (duk_get_prop_stridx(ctx, -2, DUK_STRIDX_INT_FORMALS)) {
+	/* XXX: these lookups should be just own property lookups instead of
+	 * looking up the inheritance chain.
+	 */
+	if (duk_get_prop_stridx(ctx, -1, DUK_STRIDX_INT_FORMALS)) {
 		/* [ ... closure template formals ] */
-		DUK_ASSERT(duk_has_prop_stridx(ctx, -1, DUK_STRIDX_LENGTH));
-		DUK_ASSERT(duk_get_length(ctx, -1) <= DUK_UINT_MAX);  /* formal arg limits */
 		len_value = (duk_uint_t) duk_get_length(ctx, -1);  /* could access duk_harray directly, not important */
+		DUK_DD(DUK_DDPRINT("closure length from _Formals -> %ld", (long) len_value));
 	} else {
-		/* XXX: what to do if _Formals is not empty but compiler has
-		 * optimized it away -- read length from an explicit property
-		 * then?
-		 */
+		len_value = fun_temp->nargs;
+		DUK_DD(DUK_DDPRINT("closure length defaulted from nargs -> %ld", (long) len_value));
 	}
 	duk_pop(ctx);
 
