Merge pull request #1010 from svaarala/reorganize-internal-define-sites

Remove internal helper for defining property accessors

RELEASES.rst

@@ -1992,9 +1992,10 @@ Planned
   opcode handler optimizations (GH-903); refcount optimizations (GH-443,
   GH-973); minor RegExp compile/execute optimizations (GH-974)
 
-* Miscellaneous footprint improvements: RegExp compiler/executor (GH-977),
+* Miscellaneous footprint improvements: RegExp compiler/executor (GH-977);
-  internal duk_dup() variants (GH-990), allow stripping of (almost) all
+  internal duk_dup() variants (GH-990); allow stripping of (almost) all
-  built-ins for low memory builds (GH-989)
+  built-ins for low memory builds (GH-989); remove internal accessor setup
+  helper and use duk_def_prop() instead (GH-1010)
 
 * Internal change: rework shared internal string handling so that shared
   strings are plain string constants used in macro values, rather than

src-input/duk_api_internal.h

@@ -176,7 +176,7 @@ DUK_INTERNAL_DECL void duk_xdef_prop(duk_context *ctx, duk_idx_t obj_idx, duk_sm
 DUK_INTERNAL_DECL void duk_xdef_prop_index(duk_context *ctx, duk_idx_t obj_idx, duk_uarridx_t arr_idx, duk_small_uint_t desc_flags);  /* [val] -> [] */
 DUK_INTERNAL_DECL void duk_xdef_prop_stridx(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags);  /* [val] -> [] */
 DUK_INTERNAL_DECL void duk_xdef_prop_stridx_builtin(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_int_t builtin_idx, duk_small_uint_t desc_flags);  /* [] -> [] */
-DUK_INTERNAL_DECL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags);  /* [] -> [] */
+DUK_INTERNAL_DECL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx);  /* [] -> [] */
 
 /* These are macros for now, but could be separate functions to reduce code
  * footprint (check call site count before refactoring).

src-input/duk_api_object.c

@@ -374,11 +374,12 @@ DUK_INTERNAL void duk_xdef_prop_stridx_builtin(duk_context *ctx, duk_idx_t obj_i
  * object creation code, function instance creation code, and Function.prototype.bind().
  */
 
-DUK_INTERNAL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags) {
+DUK_INTERNAL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx) {
-	duk_hthread *thr = (duk_hthread *) ctx;
+	obj_idx = duk_require_normalize_index(ctx, obj_idx);
-	duk_hobject *obj = duk_require_hobject(ctx, obj_idx);
+	duk_push_hstring_stridx(ctx, stridx);
-	duk_hobject *thrower = thr->builtins[DUK_BIDX_TYPE_ERROR_THROWER];
+	duk_push_hobject_bidx(ctx, DUK_BIDX_TYPE_ERROR_THROWER);
-	duk_hobject_define_accessor_internal(thr, obj, DUK_HTHREAD_GET_STRING(thr, stridx), thrower, thrower, desc_flags);
+	duk_dup_top(ctx);
+	duk_def_prop(ctx, obj_idx, DUK_DEFPROP_HAVE_SETTER | DUK_DEFPROP_HAVE_GETTER | DUK_DEFPROP_FORCE);  /* attributes always 0 */
 }
 
 /* Object.defineProperty() equivalent C binding. */

src-input/duk_bi_function.c

@@ -319,8 +319,8 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) {
 	duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_LENGTH, DUK_PROPDESC_FLAGS_NONE);  /* attrs in E5 Section 15.3.5.1 */
 
 	/* caller and arguments must use the same thrower, [[ThrowTypeError]] */
-	duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE);
+	duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_CALLER);
-	duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_LC_ARGUMENTS, DUK_PROPDESC_FLAGS_NONE);
+	duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_LC_ARGUMENTS);
 
 	/* these non-standard properties are copied for convenience */
 	/* XXX: 'copy properties' API call? */

src-input/duk_hobject_props.c

@@ -4741,51 +4741,6 @@ DUK_INTERNAL void duk_hobject_define_property_internal_arridx(duk_hthread *thr,
 	duk_pop(ctx);  /* [ ... key ] -> [ ... ] */
 }
 
-/*
- *  Internal helper for defining an accessor property, ignoring
- *  normal semantics such as extensibility, write protection etc.
- *  Overwrites any existing value and attributes.  This is called
- *  very rarely, so the implementation first sets a value to undefined
- *  and then changes the entry to an accessor (this is to save code space).
- */
-
-DUK_INTERNAL void duk_hobject_define_accessor_internal(duk_hthread *thr, duk_hobject *obj, duk_hstring *key, duk_hobject *getter, duk_hobject *setter, duk_small_uint_t propflags) {
-	duk_context *ctx = (duk_context *) thr;
-	duk_int_t e_idx;
-	duk_int_t h_idx;
-
-	DUK_DDD(DUK_DDDPRINT("define new accessor (internal): thr=%p, obj=%!O, key=%!O, "
-	                     "getter=%!O, setter=%!O, flags=0x%02lx",
-	                     (void *) thr, (duk_heaphdr *) obj, (duk_heaphdr *) key,
-	                     (duk_heaphdr *) getter, (duk_heaphdr *) setter,
-	                     (unsigned long) propflags));
-
-	DUK_ASSERT(thr != NULL);
-	DUK_ASSERT(thr->heap != NULL);
-	DUK_ASSERT(obj != NULL);
-	DUK_ASSERT(key != NULL);
-	DUK_ASSERT((propflags & ~DUK_PROPDESC_FLAGS_MASK) == 0);
-	/* setter and/or getter may be NULL */
-	DUK_ASSERT(!DUK_HEAPHDR_HAS_READONLY((duk_heaphdr *) obj));
-
-	DUK_ASSERT_VALSTACK_SPACE(thr, DUK__VALSTACK_SPACE);
-
-	/* force the property to 'undefined' to create a slot for it */
-	duk_push_undefined(ctx);
-	duk_hobject_define_property_internal(thr, obj, key, propflags);
-	duk_hobject_find_existing_entry(thr->heap, obj, key, &e_idx, &h_idx);
-	DUK_DDD(DUK_DDDPRINT("accessor slot: e_idx=%ld, h_idx=%ld", (long) e_idx, (long) h_idx));
-	DUK_ASSERT(e_idx >= 0);
-	DUK_ASSERT((duk_uint32_t) e_idx < DUK_HOBJECT_GET_ENEXT(obj));
-
-	/* no need to decref, as previous value is 'undefined' */
-	DUK_HOBJECT_E_SLOT_SET_ACCESSOR(thr->heap, obj, e_idx);
-	DUK_HOBJECT_E_SET_VALUE_GETTER(thr->heap, obj, e_idx, getter);
-	DUK_HOBJECT_E_SET_VALUE_SETTER(thr->heap, obj, e_idx, setter);
-	DUK_HOBJECT_INCREF_ALLOWNULL(thr, getter);
-	DUK_HOBJECT_INCREF_ALLOWNULL(thr, setter);
-}
-
 /*
  *  Internal helpers for managing object 'length'
  */

src-input/duk_hthread_builtins.c

@@ -418,7 +418,7 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {
 		num = (duk_small_uint_t) duk_bd_decode(bd, DUK__NUM_NORMAL_PROPS_BITS);
 		DUK_DDD(DUK_DDDPRINT("built-in object %ld, %ld normal valued properties", (long) i, (long) num));
 		for (j = 0; j < num; j++) {
-			duk_small_uint_t prop_flags;
+			duk_small_uint_t defprop_flags;
 
 			duk__push_stridx_or_string(ctx, bd);
 
@@ -430,15 +430,27 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {
 			 */
 
 			if (duk_bd_decode_flag(bd)) {
-				prop_flags = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_FLAGS_BITS);
+				defprop_flags = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_FLAGS_BITS);
 			} else {
-				prop_flags = DUK_PROPDESC_FLAGS_WC;
+				defprop_flags = DUK_PROPDESC_FLAGS_WC;
 			}
+			defprop_flags |= DUK_DEFPROP_FORCE |
+			                 DUK_DEFPROP_HAVE_VALUE |
+			                 DUK_DEFPROP_HAVE_WRITABLE |
+			                 DUK_DEFPROP_HAVE_ENUMERABLE |
+			                 DUK_DEFPROP_HAVE_CONFIGURABLE;  /* Defaults for data properties. */
+
+			/* The writable, enumerable, configurable flags in prop_flags
+			 * match both duk_def_prop() and internal property flags.
+			 */
+			DUK_ASSERT(DUK_PROPDESC_FLAG_WRITABLE == DUK_DEFPROP_WRITABLE);
+			DUK_ASSERT(DUK_PROPDESC_FLAG_ENUMERABLE == DUK_DEFPROP_ENUMERABLE);
+			DUK_ASSERT(DUK_PROPDESC_FLAG_CONFIGURABLE == DUK_DEFPROP_CONFIGURABLE);
 
 			t = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_TYPE_BITS);
 
 			DUK_DDD(DUK_DDDPRINT("built-in %ld, normal-valued property %ld, key %!T, flags 0x%02lx, type %ld",
-			                     (long) i, (long) j, duk_get_tval(ctx, -1), (unsigned long) prop_flags, (long) t));
+			                     (long) i, (long) j, duk_get_tval(ctx, -1), (unsigned long) defprop_flags, (long) t));
 
 			switch (t) {
 			case DUK__PROP_TYPE_DOUBLE: {
@@ -479,38 +491,28 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {
 				duk_c_function c_func_getter;
 				duk_c_function c_func_setter;
 
-				/* XXX: this is a bit awkward because there is no exposed helper
-				 * in the API style, only this internal helper.
-				 */
 				DUK_DDD(DUK_DDDPRINT("built-in accessor property: objidx=%ld, key=%!T, getteridx=%ld, setteridx=%ld, flags=0x%04lx",
-				                     (long) i, duk_get_tval(ctx, -1), (long) natidx_getter, (long) natidx_setter, (unsigned long) prop_flags));
+				                     (long) i, duk_get_tval(ctx, -1), (long) natidx_getter, (long) natidx_setter, (unsigned long) defprop_flags));
 
 				c_func_getter = duk_bi_native_functions[natidx_getter];
-				c_func_setter = duk_bi_native_functions[natidx_setter];
 				if (c_func_getter != NULL) {
 					duk_push_c_function_noconstruct_noexotic(ctx, c_func_getter, 0);  /* always 0 args */
-				} else {
+					defprop_flags |= DUK_DEFPROP_HAVE_GETTER;
-					duk_push_undefined(ctx);
 				}
+				c_func_setter = duk_bi_native_functions[natidx_setter];
 				if (c_func_setter != NULL) {
 					duk_push_c_function_noconstruct_noexotic(ctx, c_func_setter, 1);  /* always 1 arg */
-				} else {
+					defprop_flags |= DUK_DEFPROP_HAVE_SETTER;
-					duk_push_undefined(ctx);
 				}
 
-				/* XXX: magic for getter/setter? use duk_def_prop()? */
+				/* XXX: magic for getter/setter? */
 
-				DUK_ASSERT((prop_flags & DUK_PROPDESC_FLAG_WRITABLE) == 0);  /* genbuiltins.py ensures */
+				/* Writable flag doesn't make sense for an accessor. */
+				DUK_ASSERT((defprop_flags & DUK_PROPDESC_FLAG_WRITABLE) == 0);  /* genbuiltins.py ensures */
 
-				prop_flags |= DUK_PROPDESC_FLAG_ACCESSOR;  /* accessor flag not encoded explicitly */
+				defprop_flags &= ~(DUK_DEFPROP_HAVE_VALUE | DUK_DEFPROP_HAVE_WRITABLE);
-				duk_hobject_define_accessor_internal(thr,
+				defprop_flags |= DUK_DEFPROP_HAVE_ENUMERABLE | DUK_DEFPROP_HAVE_CONFIGURABLE;
-				                                     duk_require_hobject(ctx, i),
+				break;
-				                                     duk_get_hstring(ctx, -3),
-				                                     duk_get_hobject(ctx, -2),
-				                                     duk_get_hobject(ctx, -1),
-				                                     prop_flags);
-				duk_pop_3(ctx);  /* key, getter and setter, now reachable through object */
-				goto skip_value;
 			}
 			default: {
 				/* exhaustive */
@@ -518,11 +520,8 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {
 			}
 			}
 
-			DUK_ASSERT((prop_flags & DUK_PROPDESC_FLAG_ACCESSOR) == 0);
+			duk_def_prop(ctx, i, defprop_flags);
-			duk_xdef_prop(ctx, i, prop_flags);
+			DUK_ASSERT_TOP(ctx, DUK_NUM_ALL_BUILTINS);
-
-		 skip_value:
-			continue;  /* avoid empty label at the end of a compound statement */
 		}
 
 		/* native function properties */

src-input/duk_js_call.c

@@ -308,8 +308,8 @@ DUK_LOCAL void duk__create_arguments_object(duk_hthread *thr,
 
 		DUK_DDD(DUK_DDDPRINT("strict function, setting caller/callee to throwers"));
 
-		duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE);
+		duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLER);
-		duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLEE, DUK_PROPDESC_FLAGS_NONE);
+		duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLEE);
 	} else {
 		DUK_DDD(DUK_DDDPRINT("non-strict function, setting callee to actual value"));
 		duk_push_hobject(ctx, func);

src-input/duk_js_var.c

@@ -393,8 +393,8 @@ void duk_js_push_closure(duk_hthread *thr,
 	/* [ ... closure template ] */
 
 	if (DUK_HOBJECT_HAS_STRICT(&fun_clos->obj)) {
-		duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE);
+		duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_CALLER);
-		duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_LC_ARGUMENTS, DUK_PROPDESC_FLAGS_NONE);
+		duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_LC_ARGUMENTS);
 	} else {
 #ifdef DUK_USE_NONSTD_FUNC_CALLER_PROPERTY
 		DUK_DDD(DUK_DDDPRINT("function is non-strict and non-standard 'caller' property in use, add initial 'null' value"));