GNU bug report logs - #69953
[PATCH] Remove duplicated asserts and checks

Previous Next

Full log

View this message in rfc822 format

From: Daniel Colascione <dancol <at> dancol.org>
To: Sergey Vinokurov <serg.foo <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 69953 <at> debbugs.gnu.org, p.stephani2 <at> gmail.com
Subject: bug#69953: [PATCH] Remove duplicated asserts and checks
Date: Sat, 11 May 2024 06:11:01 -0700

Sergey Vinokurov <serg.foo <at> gmail.com> writes:

> On 09/05/2024 08:24, Eli Zaretskii wrote:
>> tags 69953 wontfix
>> close 69953
>> thanks
>> 
>>> Cc: serg.foo <at> gmail.com, 69953 <at> debbugs.gnu.org
>>> Date: Sat, 27 Apr 2024 11:27:41 +0300
>>> From: Eli Zaretskii <eliz <at> gnu.org>
>>>
>>> Ping! Ping!  Philipp and Daniel, any comments?
>>>
>>>> Cc: serg.foo <at> gmail.com, 69953 <at> debbugs.gnu.org
>>>> Date: Sat, 13 Apr 2024 10:42:50 +0300
>>>> From: Eli Zaretskii <eliz <at> gnu.org>
>>>>
>>>> Ping! Philipp and Daniel, do you have any comments on this?
>>>>
>>>>> Cc: Philipp Stephani <phst <at> google.com>, 69953 <at> debbugs.gnu.org,
>>>>>   Daniel Colascione <dancol <at> dancol.org>
>>>>> Date: Sat, 23 Mar 2024 09:15:04 +0200
>>>>> From: Eli Zaretskii <eliz <at> gnu.org>
>>>>>
>>>>>> Date: Sat, 23 Mar 2024 03:27:34 +0000
>>>>>> From: Sergey Vinokurov <serg.foo <at> gmail.com>
>>>>>>
>>>>>> 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?


Programming is not an exercise in Huffman compression :-).  Sometimes,
things that look repetitive are not duplicative: the same text can express
different things in different contexts.  The assertions in this patch
express the intent of the authors of macros like
MODULE_FUNCTION_BEGIN_NO_CATCH.  Their presence in the code signifies
in writing the contract to which users of MODULE_FUNCTION_BEGIN_NO_CATCH
and other facilities must adhere.  Deleting them would therefore make it
incrementally harder to understand the code, as it costs the brain more
to infer a contract than to read one.

I definitely appreciate the desire to clean up the codebase.  I think
the energy that's gone into this thread could be more productively
applied to other code cleanups.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.