bug#69953: [PATCH] Remove duplicated asserts and checks

[Sergey Vinokurov <serg.foo@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

Hello,

I noticed that emacs-module.c contains duplicate 
module_non_local_exit_check() checks and 
module_assert_thread/module_assert_env asserts, mostly performed at the 
same point in program sequentially.

The module_non_local_exit_check() checks happen in 
MODULE_HANDLE_NONLOCAL_EXIT and MODULE_FUNCTION_BEGIN_NO_CATCH macros. 
The MODULE_HANDLE_NONLOCAL_EXIT is never used by itself, only as part of 
MODULE_FUNCTION_BEGIN which starts with MODULE_FUNCTION_BEGIN_NO_CATCH 
that performs the check.

In addition, there're 6 "Implementation of runtime and environment 
functions" rules outlined where MODULE_HANDLE_NONLOCAL_EXIT should be 
called at step 4 but module_non_local_exit_check() is supposed to have 
already happened at step 3 so documentation does not seem to intend for 
the check to be repeated in MODULE_HANDLE_NONLOCAL_EXIT.

Regarding asserts my observation is that module_non_local_exit_check() 
already contains module_assert_thread and module_assert_env so there's 
no need to do asserts if first thing we do is call 
module_non_local_exit_check.

Regards,
Sergey

[-- Attachment #2: 0001-Remove-duplicated-asserts-and-checks.patch --]
[-- Type: text/x-patch, Size: 2592 bytes --]

From 8c8516ee4869690d0d9418b26d4fae90520c9860 Mon Sep 17 00:00:00 2001
From: Sergey Vinokurov <serg.foo@gmail.com>
Date: Sat, 23 Mar 2024 02:15:06 +0000
Subject: [PATCH] Remove duplicated asserts and checks

* src/emacs-module.c (MODULE_HANDLE_NONLOCAL_EXIT): Remove redundant check
* src/emacs-module.c (MODULE_FUNCTION_BEGIN_NO_CATCH): Remove redundant assert
* src/emacs-module.c (module_non_local_exit_signal): Remove redundant assert
* src/emacs-module.c (module_non_local_exit_throw): Remove redundant assert
---
 src/emacs-module.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 08db39b0b0d..fbeeb146a68 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -264,8 +264,6 @@ module_decode_utf_8 (const char *str, ptrdiff_t len)
 /* TODO: Make backtraces work if this macro is used.  */
 
 #define MODULE_HANDLE_NONLOCAL_EXIT(retval)                             \
-  if (module_non_local_exit_check (env) != emacs_funcall_exit_return)	\
-    return retval;							\
   struct handler *internal_handler =                                    \
     push_handler_nosignal (Qt, CATCHER_ALL);                            \
   if (!internal_handler)                                                \
@@ -332,8 +330,6 @@ #define MODULE_INTERNAL_CLEANUP()		\
 
 #define MODULE_FUNCTION_BEGIN_NO_CATCH(error_retval)                    \
   do {                                                                  \
-    module_assert_thread ();                                            \
-    module_assert_env (env);                                            \
     if (module_non_local_exit_check (env) != emacs_funcall_exit_return) \
       return error_retval;                                              \
   } while (false)
@@ -523,8 +519,6 @@ module_non_local_exit_get (emacs_env *env,
 module_non_local_exit_signal (emacs_env *env,
                               emacs_value symbol, emacs_value data)
 {
-  module_assert_thread ();
-  module_assert_env (env);
   if (module_non_local_exit_check (env) == emacs_funcall_exit_return)
     module_non_local_exit_signal_1 (env, value_to_lisp (symbol),
 				    value_to_lisp (data));
@@ -533,8 +527,6 @@ module_non_local_exit_signal (emacs_env *env,
 static void
 module_non_local_exit_throw (emacs_env *env, emacs_value tag, emacs_value value)
 {
-  module_assert_thread ();
-  module_assert_env (env);
   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));
-- 
2.43.1