FIXME cleanups and downgrades

Add value stack asserts to executor for the (somewhat messy) resetting of
value stack top in a few longjmp handling cases.

Downgrade a few other FIXMEs.  Ditz issues and XXX comments track whatever
improvements are left.

src/duk_api_public.h.in

@@ -381,7 +381,6 @@ void duk_pop_3(duk_context *ctx);
  *  is not needed; duk_is_valid_index() gives the same information.
  */
 
-/* FIXME: a duk_small_int_t suffices to represent type and type mask (at least now). */
 duk_int_t duk_get_type(duk_context *ctx, duk_idx_t index);
 duk_bool_t duk_check_type(duk_context *ctx, duk_idx_t index, duk_int_t type);
 duk_uint_t duk_get_type_mask(duk_context *ctx, duk_idx_t index);
@@ -491,8 +490,8 @@ void *duk_to_fixed_buffer(duk_context *ctx, duk_idx_t index, duk_size_t *out_siz
 void *duk_to_dynamic_buffer(duk_context *ctx, duk_idx_t index, duk_size_t *out_size);
 void *duk_to_pointer(duk_context *ctx, duk_idx_t index);
 void duk_to_object(duk_context *ctx, duk_idx_t index);
-void duk_to_defaultvalue(duk_context *ctx, duk_idx_t index, duk_int_t hint);  /* FIXME: small int? */
+void duk_to_defaultvalue(duk_context *ctx, duk_idx_t index, duk_int_t hint);
-void duk_to_primitive(duk_context *ctx, duk_idx_t index, duk_int_t hint);  /* FIXME: small int? */
+void duk_to_primitive(duk_context *ctx, duk_idx_t index, duk_int_t hint);
 
 /* safe variants of a few coercion operations */
 const char *duk_safe_to_lstring(duk_context *ctx, duk_idx_t index, duk_size_t *out_len);
@@ -572,7 +571,9 @@ void duk_put_number_list(duk_context *ctx, duk_idx_t obj_index, const duk_number
  *  Variable access
  */
 
-/* FIXME: incomplete, not usable now */
+/* XXX: These calls are incomplete and not usable now.  They are not (yet)
+ * part of the public API.
+ */
 void duk_get_var(duk_context *ctx);
 void duk_put_var(duk_context *ctx);
 duk_bool_t duk_del_var(duk_context *ctx);
@@ -753,7 +754,6 @@ duk_int_t duk_compile_raw(duk_context *ctx, const char *src_buffer, duk_size_t s
  *  Logging
  */
 
-/* FIXME: here a small integer type would be proper */
 void duk_log(duk_context *ctx, duk_int_t level, const char *fmt, ...);
 
 /*

src/duk_error_throw.c

@@ -47,7 +47,7 @@ void duk_err_create_and_throw(duk_hthread *thr, duk_errcode_t code) {
 	 *  to avoid further trouble.
 	 */
 
-	/* FIXME: if attempt to push beyond allocated valstack, this double fault
+	/* XXX: if attempt to push beyond allocated valstack, this double fault
 	 * handling fails miserably.  We should really write the double error
 	 * directly to thr->heap->lj.value1 and avoid valstack use entirely.
 	 */

src/duk_js_call.c

@@ -817,7 +817,7 @@ duk_int_t duk_handle_call(duk_hthread *thr,
 
 	/* [ ... func this (crud) errobj ] */
 
-	/* FIXME: is there space?  better implementation: write directly over
+	/* XXX: is there space?  better implementation: write directly over
 	 * 'func' slot to avoid valstack grow issues.
 	 */
 	duk_push_tval(ctx, &thr->heap->lj.value1);
@@ -1468,7 +1468,7 @@ static void duk__safe_call_adjust_valstack(duk_hthread *thr, duk_idx_t idx_retba
  *  handler if nothing else.
  */
 
-/* FIXME: bump preventcount by one for the duration of this call? */
+/* XXX: bump preventcount by one for the duration of this call? */
 
 duk_int_t duk_handle_safe_call(duk_hthread *thr,
                                duk_safe_call_function func,
@@ -1572,7 +1572,7 @@ duk_int_t duk_handle_safe_call(duk_hthread *thr,
 
 	/* [ ... | (crud) ] */
 
-	/* FIXME: space in valstack?  see discussion in duk_handle_call. */
+	/* XXX: space in valstack?  see discussion in duk_handle_call. */
 	duk_push_tval(ctx, &thr->heap->lj.value1);
 
 	/* [ ... | (crud) errobj ] */

src/duk_js_compiler.c

@@ -1989,17 +1989,24 @@ static void duk__ivalue_toplain_raw(duk_compiler_ctx *comp_ctx, duk_ivalue *x, d
 		duk_regconst_t arg2;
 		duk_reg_t dest;
 
-		/* need a short reg/const, does not have to be a mutable temp */
+		/* Need a short reg/const, does not have to be a mutable temp. */
 		arg1 = duk__ispec_toregconst_raw(comp_ctx, &x->x1, -1, DUK__IVAL_FLAG_ALLOW_CONST | DUK__IVAL_FLAG_REQUIRE_SHORT /*flags*/);
 		arg2 = duk__ispec_toregconst_raw(comp_ctx, &x->x2, -1, DUK__IVAL_FLAG_ALLOW_CONST | DUK__IVAL_FLAG_REQUIRE_SHORT /*flags*/);
 
+		/* Pick a destination register.  If either base value or key
+		 * happens to be a temp value, reuse it as the destination.
+		 *
+		 * XXX: The temp must be a "mutable" one, i.e. such that no
+		 * other expression is using it anymore.  Here this should be
+		 * the case because the value of a property access expression
+		 * is neither the base nor the key, but the lookup result.
+		 */
+
 		if (forced_reg >= 0) {
 			dest = forced_reg;
 		} else if (DUK__ISTEMP(comp_ctx, arg1)) {
-			/* FIXME: arg1 being used as a mutable temp? */
 			dest = (duk_reg_t) arg1;
 		} else if (DUK__ISTEMP(comp_ctx, arg2)) {
-			/* FIXME: arg1 being used as a mutable temp? */
 			dest = (duk_reg_t) arg2;
 		} else {
 			dest = DUK__ALLOCTEMP(comp_ctx);

src/duk_js_executor.c

@@ -643,14 +643,28 @@ static void duk__handle_catch_or_finally(duk_hthread *thr, duk_size_t cat_idx, d
 }
 
 static void duk__handle_label(duk_hthread *thr, duk_size_t cat_idx) {
+	duk_activation *act;
+
 	/* no callstack changes, no value stack changes */
 
+	DUK_ASSERT(thr != NULL);
+	DUK_ASSERT(thr->callstack_top >= 1);
+
+	act = thr->callstack + thr->callstack_top - 1;
+
+	DUK_ASSERT(act->func != NULL);
+	DUK_ASSERT(DUK_HOBJECT_HAS_COMPILEDFUNCTION(act->func));
+
 	/* +0 = break, +1 = continue */
 	(thr->callstack + thr->callstack_top - 1)->pc =
 		thr->catchstack[cat_idx].pc_base + (thr->heap->lj.type == DUK_LJ_TYPE_CONTINUE ? 1 : 0);
 
 	duk_hthread_catchstack_unwind(thr, cat_idx + 1);  /* keep label catcher */
 	/* no need to unwind callstack */
+
+	/* valstack should not need changes */
+	DUK_ASSERT((duk_size_t) (thr->valstack_top - thr->valstack_bottom) ==
+	           (duk_size_t) ((duk_hcompiledfunction *) act->func)->nregs);
 }
 
 /* Note: called for DUK_LJ_TYPE_YIELD and for DUK_LJ_TYPE_RETURN, when a
@@ -1108,8 +1122,6 @@ static duk_small_uint_t duk__handle_longjmp(duk_hthread *thr,
 				duk__handle_label(thr,
 				                  cat - thr->catchstack);
 
-				/* FIXME: reset valstack to 'nregs' (or assert it) */
-
 				DUK_DD(DUK_DDPRINT("-> break/continue caught by a label catcher (in the same function), restart execution"));
 				retval = DUK__LONGJMP_RESTART;
 				goto wipe_and_return;
@@ -1173,8 +1185,6 @@ static duk_small_uint_t duk__handle_longjmp(duk_hthread *thr,
 				                             cat - thr->catchstack,
 				                             1); /* is_finally */
 
-				/* FIXME: reset valstack to 'nregs' (or assert it) */
-
 				DUK_DD(DUK_DDPRINT("-> throw caught by a 'finally' clause, restart execution"));
 				retval = DUK__LONGJMP_RESTART;
 				goto wipe_and_return;