GNU bug report logs - #48264
28.0.50; Changing the default for DEFVAR_PER_BUFFER variables takes O(#buffers) time

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> catern.com>

Date: Thu, 6 May 2021 20:25:01 UTC

Severity: normal

Found in version 28.0.50

Full log


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

From: Spencer Baugh <sbaugh <at> catern.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 48264 <at> debbugs.gnu.org
Subject: Re: bug#48264: [PATCH v3 03/15] Add and use BUFFER_DEFAULT_VALUE_P
Date: Fri, 07 May 2021 09:38:20 -0400
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Spencer Baugh <sbaugh <at> catern.com>
>> Cc: 48264 <at> debbugs.gnu.org
>> Date: Fri, 07 May 2021 08:49:59 -0400
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >> From: Spencer Baugh <sbaugh <at> catern.com>
>> >> Date: Thu,  6 May 2021 17:33:34 -0400
>> >> Cc: Spencer Baugh <sbaugh <at> catern.com>
>> >> 
>> >> This makes the code more clear and allows us to more easily change how
>> >> this property is determined.
>> >
>> > Does it?  Can you explain why you think so?  It looks like we are
>> > replacing clear code with an equally clear different code.
>> 
>> Well, "if (idx > 0)" as a conditional requires a fair bit of digging in
>> the implementation of DEFVAR_PER_BUFFER variables to understand.  On the
>> other hand, "if (BUFFER_DEFAULT_VALUE_P (offset))" is immediately clear:
>> We're checking if this variable has a default value.
>
> It's the other way around here: the test "if (idx > 0)" is clear,
> whereas "if (BUFFER_DEFAULT_VALUE_P (offset))" makes me go look up the
> definition of the macro, because the name is not expressive enough,
> and the argument "offset" doesn't help, either.

Sure; what about the name "BUFFER_VAR_HAS_DEFAULT_VALUE_P"?  More
verbose, but I think that makes the functionality sufficiently clear
that one won't need to look at the definition of the macro.

"idx > 0" is only clear if one has memorized how all the different
pieces of DEFVAR_PER_BUFFER metadata (such as the index) work.  But I
don't think a casual reader would have done that.




This bug report was last modified 2 years and 289 days ago.

Previous Next


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