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

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> catern.com>
Cc: 48264 <at> debbugs.gnu.org
Subject: bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros
Date: Fri, 07 May 2021 16:08:41 +0300

> From: Spencer Baugh <sbaugh <at> catern.com>
> Cc: 48264 <at> debbugs.gnu.org
> Date: Fri, 07 May 2021 08:59:36 -0400
> 
> >> -  b->abbrev_mode_ = val;
> >> +  b->BVAR_DEFAULTED_FIELD(abbrev_mode) = val;
> >
> > Yuck!  Can we avoid having a macro in the struct field names?  I'd
> > prefer that the BVAR_DEFAULTED_FIELD macro accepted the buffer as
> > argument instead (if we need a macro at all; see below).
> 
> Ah, yes, I probably should have given a little more explanation of this.
> I'm not tied to this approach, if we can think of a better way to make
> it statically guaranteed that BVAR is only used with fields that have a
> default.

If the sole purpose is to be able to detect coding mistakes, then
there are other possibilities to do that, if the compiler cannot help
in a way that leaves the sources readable.

> >> @@ -714,12 +719,12 @@ XBUFFER (Lisp_Object a)
> >>  INLINE void
> >>  bset_bidi_paragraph_direction (struct buffer *b, Lisp_Object val)
> >>  {
> >> -  b->bidi_paragraph_direction_ = val;
> >> +  b->BVAR_DEFAULTED_FIELD(bidi_paragraph_direction) = val;
> >>  }
> >>  INLINE void
> >>  bset_cache_long_scans (struct buffer *b, Lisp_Object val)
> >>  {
> >> -  b->cache_long_scans_ = val;
> >> +  b->BVAR_DEFAULTED_FIELD(cache_long_scans) = val;
> >>  }
> >>  INLINE void
> >>  bset_case_canon_table (struct buffer *b, Lisp_Object val)
> >> @@ -744,12 +749,12 @@ bset_display_count (struct buffer *b, Lisp_Object val)
> >>  INLINE void
> >>  bset_left_margin_cols (struct buffer *b, Lisp_Object val)
> >>  {
> >> -  b->left_margin_cols_ = val;
> >> +  b->BVAR_DEFAULTED_FIELD(left_margin_cols) = val;
> >>  }
> >
> > Hmm... I'm not sure I understand the effect of these.  Does it mean C
> > code can no longer set the buffer-local value of these variables, only
> > the default value?
> 
> No, this is purely just changing the name of the fields - it has no
> impact on functionality, C code can still set the buffer-local
> variables.

Then I guess the _defaulted_ part is a misnomer?

> >> +#define PER_BUFFER_VAR_DEFAULTED_OFFSET(VAR) \
> >> +  offsetof (struct buffer, BVAR_DEFAULTED_FIELD(VAR))
> >
> > Likewise here: I don't see how such macros make the code more
> > readable.  I think they make it less readable.
> 
> I agree but I couldn't find a better way to ensure that BVAR and
> BVAR_OR_DEFAULT are used on the correct fields.

Maybe someone could come up with a trick to have the diagnostics
without twisting the sources so much.  Failing that, maybe we should
simply have a test to detect the mistakes?  That wouldn't prevent bad
code from being compiled, but it should reveal it soon enough, since
tests are regularly run on hydra.

Previous Next

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