GNU bug report logs -
#69953
[PATCH] Remove duplicated asserts and checks
Previous Next
To reply to this bug, email your comments to 69953 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#69953
; Package
emacs
.
(Sat, 23 Mar 2024 03:29:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Sergey Vinokurov <serg.foo <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sat, 23 Mar 2024 03:29:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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
[0001-Remove-duplicated-asserts-and-checks.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#69953
; Package
emacs
.
(Sat, 23 Mar 2024 07:16:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 69953 <at> debbugs.gnu.org (full text, mbox):
> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#69953
; Package
emacs
.
(Sat, 23 Mar 2024 12:41:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 69953 <at> debbugs.gnu.org (full text, mbox):
The issue is that the same thing is being checked just after it was
checked. The checks I propose to remove are not going to catch any new
errors under any possible scenario.
Even with module_assertions disabled, the assert functions are still
called - a very minor annoyance.
On 23/03/2024 07:15, Eli Zaretskii wrote:
>> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#69953
; Package
emacs
.
(Sat, 13 Apr 2024 07:44:06 GMT)
Full text and
rfc822 format available.
Message #14 received at 69953 <at> debbugs.gnu.org (full text, mbox):
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.
>
>
>
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#69953
; Package
emacs
.
(Sat, 27 Apr 2024 08:29:04 GMT)
Full text and
rfc822 format available.
Message #17 received at 69953 <at> debbugs.gnu.org (full text, mbox):
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.
> >
> >
> >
> >
>
>
>
>
This bug report was last modified 7 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.