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

Previous Next

Full log

Message #39 received at 69953 <at> debbugs.gnu.org (full text, mbox):

From: Sergey Vinokurov <serg.foo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: p.stephani2 <at> gmail.com, dancol <at> dancol.org, 69953 <at> debbugs.gnu.org
Subject: Re: bug#69953: [PATCH] Remove duplicated asserts and checks
Date: Sat, 11 May 2024 13:57:31 +0100

On 11/05/2024 13:18, Eli Zaretskii wrote:
>> Date: Sat, 11 May 2024 13:12:33 +0100
>> Cc: p.stephani2 <at> gmail.com, dancol <at> dancol.org, 69953 <at> debbugs.gnu.org
>> From: Sergey Vinokurov <serg.foo <at> gmail.com>
>>
>>> Given the lack of responses, I doubt that.
>>
>> Is there no way to reevaluate the patch from the start? It's fairly
>> small, perhaps I did poor job explaining why it's good idea?
> 
> AFAIU, the code isn't wrong, and doesn't cause any problems.  You just
> think it's redundant.  That's a judgment call, and without Daniel and
> Philipp, who worked on most of this case, assessing the proposal, I
> don't want to change code that worked well for us for several Emacs
> releases, just because it might be redundant.
> 
> Why are you so eager to install these changes?

As I benefit from Emacs daily I want to help make it the best version of 
itself. Many things in Emacs I rely on were made by someone, I thought 
this change would ever so slightly benefit others.

It's small indeed and maybe even inconsequential. But as the saying 
goes, the devil is in the details - the small things. Another reason to 
bother is to see whether I can manage to get small things merged - 
bigger contributions would likely involve all the hard parts of merging 
small contributions and then some. If I'm not able to finish small 
things then bigger ones are unlikely to succeed.

As for judgement call I don't know, I mostly unwrapped the various macro 
calls and saw that what one ends up with is assert after assert of the 
same property (e.g. that current thread is OK). For example, if in plain 
C within Emacs, in some other module one found

Lisp_Object
foo(Lisp_Object arg)
  {
    ...
    eassert(NILP(arg));
    eassert(NILP(arg));
    ...
  }

then it would surely be desirable to keep only one call to eassert as 
the second one serves no purpose and provides no extra checks. I argue 
that my patch does exactly that, the only complication is that some 
assert calls come from macro definitions which need to be unwrapped to 
see what's going on.

To paraphrase the example, I removed the second call to assert while 
keeping the first one so same checks happen in the same places as before.

Previous Next

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