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

From: Sergey Vinokurov <[email-masked]>
Date: Thu, 9 May 2024 15:16:05 +0100

On 09/05/2024 08:24, Eli Zaretskii wrote:
> tags 69953 wontfix
> close 69953
> thanks
> 
>> Cc: [email-masked], [email-masked]
>> Date: Sat, 27 Apr 2024 11:27:41 +0300
>> From: Eli Zaretskii <[email-masked]>
>>
>> Ping! Ping!  Philipp and Daniel, any comments?
>>
>>> Cc: [email-masked], [email-masked]
>>> Date: Sat, 13 Apr 2024 10:42:50 +0300
>>> From: Eli Zaretskii <[email-masked]>
>>>
>>> Ping! Philipp and Daniel, do you have any comments on this?
>>>
>>>> Cc: Philipp Stephani <[email-masked]>, [email-masked],
>>>>   Daniel Colascione <[email-masked]>
>>>> Date: Sat, 23 Mar 2024 09:15:04 +0200
>>>> From: Eli Zaretskii <[email-masked]>
>>>>
>>>>> Date: Sat, 23 Mar 2024 03:27:34 +0000
>>>>> From: Sergey Vinokurov <[email-masked]>
>>>>>
>>>>> 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.
>>>>
>>>> Thanks, but why is that a problem?  module_assertions is false by
>>>> default, and the function to turn on module assertions is not even
>>>> documented in the ELisp manual.  IOW, this is a debugging aid which
>>>> will rarely if at all activated, and if it is, that's on purpose by
>>>> the programmer who is investigating some tricky problem.  Why is it a
>>>> problem to have too many assertions, which might help that programmer
>>>> find a bug?
>>>>
>>>> I added Daniel and Philipp to the discussion, in case they have
>>>> comments to this proposal.
> 
> Given the lack of comments, I conclude that there's no interest in
> installing this, and I'm therefore closing this bug.

I'm interested to get this installed - can I do anything to facilitate?