Re: Dynamic modules: emacs-module.c and signaling errors

[Eli Zaretskii]

> From: Tom Tromey <address@hidden>
> Cc: Eli Zaretskii <address@hidden>,  Tom Tromey <address@hidden>,  
> address@hidden,  address@hidden,  address@hidden,  address@hidden,  
> address@hidden
> Date: Tue, 24 Nov 2015 15:21:20 -0700
> 
> Perhaps module APIs should just fail when the environment is already
> in the error state.

Thanks, I think this is a very simple measure that goes a long way
towards resolving the issue.

Any objections to installing the patch below?  With it, I can get the
expected Lisp error when invoking the unchanged mod-test-sum with
non-integer arguments.

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 1388e53..c75ddeb 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -198,7 +198,8 @@ static void module_wrong_type (emacs_env *, Lisp_Object, 
Lisp_Object);
    its local variable C must stay live in later code.  */
 
 #define MODULE_SETJMP_1(handlertype, handlerfunc, retval, c, dummy)    \
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); \
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)  \
+    return retval;                                                     \
   struct handler *c = push_handler_nosignal (Qt, handlertype);         \
   if (!c)                                                              \
     {                                                                  \
@@ -256,7 +288,8 @@ static emacs_value
 module_make_global_ref (emacs_env *env, emacs_value ref)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
   struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash);
   Lisp_Object new_obj = value_to_lisp (ref);
@@ -287,7 +320,8 @@ static void
 module_free_global_ref (emacs_env *env, emacs_value ref)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return;
   /* TODO: This probably never signals.  */
   /* FIXME: Wait a minute.  Shouldn't this function report an error if
      the hash lookup fails?  */
@@ -343,18 +377,18 @@ static void
 module_non_local_exit_signal (emacs_env *env, emacs_value sym, emacs_value 
data)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
-  module_non_local_exit_signal_1 (env, value_to_lisp (sym),
-                                 value_to_lisp (data));
+  if (module_non_local_exit_check (env) == emacs_funcall_exit_return)
+    module_non_local_exit_signal_1 (env, value_to_lisp (sym),
+                                   value_to_lisp (data));
 }
 
 static void
 module_non_local_exit_throw (emacs_env *env, emacs_value tag, emacs_value 
value)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
-  module_non_local_exit_throw_1 (env, value_to_lisp (tag),
-                                value_to_lisp (value));
+  if (module_non_local_exit_check (env) == emacs_funcall_exit_return)
+    module_non_local_exit_throw_1 (env, value_to_lisp (tag),
+                                  value_to_lisp (value));
 }
 
 /* A module function is lambda function that calls `module-call',
@@ -370,7 +404,8 @@ module_make_function (emacs_env *env, ptrdiff_t min_arity, 
ptrdiff_t max_arity,
                      void *data)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
 
   if (! (0 <= min_arity
@@ -407,7 +442,8 @@ module_funcall (emacs_env *env, emacs_value fun, ptrdiff_t 
nargs,
                emacs_value args[])
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
   MODULE_HANDLE_THROW;
 
@@ -428,7 +464,8 @@ static emacs_value
 module_intern (emacs_env *env, const char *name)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
   return lisp_to_value (env, intern (name));
 }
@@ -437,7 +474,8 @@ static emacs_value
 module_type_of (emacs_env *env, emacs_value value)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   return lisp_to_value (env, Ftype_of (value_to_lisp (value)));
 }
 
@@ -445,7 +483,8 @@ static bool
 module_is_not_nil (emacs_env *env, emacs_value value)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return false;
   return ! NILP (value_to_lisp (value));
 }
 
@@ -453,7 +492,8 @@ static bool
 module_eq (emacs_env *env, emacs_value a, emacs_value b)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return false;
   return EQ (value_to_lisp (a), value_to_lisp (b));
 }
 
@@ -461,7 +501,8 @@ static intmax_t
 module_extract_integer (emacs_env *env, emacs_value n)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return 0;
   Lisp_Object l = value_to_lisp (n);
   if (! INTEGERP (l))
     {
@@ -475,7 +516,8 @@ static emacs_value
 module_make_integer (emacs_env *env, intmax_t n)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   if (! (MOST_NEGATIVE_FIXNUM <= n && n <= MOST_POSITIVE_FIXNUM))
     {
       module_non_local_exit_signal_1 (env, Qoverflow_error, Qnil);
@@ -488,7 +530,8 @@ static double
 module_extract_float (emacs_env *env, emacs_value f)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return 0;
   Lisp_Object lisp = value_to_lisp (f);
   if (! FLOATP (lisp))
     {
@@ -502,7 +545,8 @@ static emacs_value
 module_make_float (emacs_env *env, double d)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
   return lisp_to_value (env, make_float (d));
 }
@@ -512,7 +556,8 @@ module_copy_string_contents (emacs_env *env, emacs_value 
value, char *buffer,
                             ptrdiff_t *length)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return false;
   MODULE_HANDLE_SIGNALS;
   Lisp_Object lisp_str = value_to_lisp (value);
   if (! STRINGP (lisp_str))
@@ -557,7 +602,8 @@ static emacs_value
 module_make_string (emacs_env *env, const char *str, ptrdiff_t length)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   MODULE_HANDLE_SIGNALS;
   if (length > STRING_BYTES_BOUND)
     {
@@ -573,6 +619,8 @@ static emacs_value
 module_make_user_ptr (emacs_env *env, emacs_finalizer_function fin, void *ptr)
 {
   check_main_thread ();
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   return lisp_to_value (env, make_user_ptr (fin, ptr));
 }
 
@@ -580,7 +628,8 @@ static void *
 module_get_user_ptr (emacs_env *env, emacs_value uptr)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   Lisp_Object lisp = value_to_lisp (uptr);
   if (! USER_PTRP (lisp))
     {
@@ -594,7 +643,8 @@ static void
 module_set_user_ptr (emacs_env *env, emacs_value uptr, void *ptr)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return;
   Lisp_Object lisp = value_to_lisp (uptr);
   if (! USER_PTRP (lisp))
     module_wrong_type (env, Quser_ptr, lisp);
@@ -605,7 +655,8 @@ static emacs_finalizer_function
 module_get_user_finalizer (emacs_env *env, emacs_value uptr)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   Lisp_Object lisp = value_to_lisp (uptr);
   if (! USER_PTRP (lisp))
     {
@@ -620,7 +671,8 @@ module_set_user_finalizer (emacs_env *env, emacs_value uptr,
                           emacs_finalizer_function fin)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return;
   Lisp_Object lisp = value_to_lisp (uptr);
   if (! USER_PTRP (lisp))
     module_wrong_type (env, Quser_ptr, lisp);
@@ -631,7 +683,8 @@ static void
 module_vec_set (emacs_env *env, emacs_value vec, ptrdiff_t i, emacs_value val)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return;
   Lisp_Object lvec = value_to_lisp (vec);
   if (! VECTORP (lvec))
     {
@@ -653,7 +706,8 @@ static emacs_value
 module_vec_get (emacs_env *env, emacs_value vec, ptrdiff_t i)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return NULL;
   Lisp_Object lvec = value_to_lisp (vec);
   if (! VECTORP (lvec))
     {
@@ -675,7 +729,8 @@ static ptrdiff_t
 module_vec_size (emacs_env *env, emacs_value vec)
 {
   check_main_thread ();
-  eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
+  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
+    return 0;
   Lisp_Object lvec = value_to_lisp (vec);
   if (! VECTORP (lvec))
     {
@@ -806,10 +861,12 @@ module_non_local_exit_signal_1 (emacs_env *env, 
Lisp_Object sym,
                                Lisp_Object data)
 {
   struct emacs_env_private *p = env->private_members;
-  eassert (p->pending_non_local_exit == emacs_funcall_exit_return);
-  p->pending_non_local_exit = emacs_funcall_exit_signal;
-  p->non_local_exit_symbol.v = sym;
-  p->non_local_exit_data.v = data;
+  if (p->pending_non_local_exit == emacs_funcall_exit_return)
+    {
+      p->pending_non_local_exit = emacs_funcall_exit_signal;
+      p->non_local_exit_symbol.v = sym;
+      p->non_local_exit_data.v = data;
+    }
 }
 
 static void
@@ -817,10 +874,12 @@ module_non_local_exit_throw_1 (emacs_env *env, 
Lisp_Object tag,
                               Lisp_Object value)
 {
   struct emacs_env_private *p = env->private_members;
-  eassert (p->pending_non_local_exit == emacs_funcall_exit_return);
-  p->pending_non_local_exit = emacs_funcall_exit_throw;
-  p->non_local_exit_symbol.v = tag;
-  p->non_local_exit_data.v = value;
+  if (p->pending_non_local_exit == emacs_funcall_exit_return)
+    {
+      p->pending_non_local_exit = emacs_funcall_exit_throw;
+      p->non_local_exit_symbol.v = tag;
+      p->non_local_exit_data.v = value;
+    }
 }
 
 /* Module version of `wrong_type_argument'.  */