[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros
From: |
Eli Zaretskii |
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@catern.com>
> Cc: 48264@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.
- bug#48264: [PATCH v3 07/15] Add BVAR_OR_DEFAULT macro as a stub, (continued)
bug#48264: [PATCH v3 10/15] Get rid of buffer_permanent_local_flags array, Spencer Baugh, 2021/05/06
bug#48264: [PATCH v3 11/15] Delete SET_PER_BUFFER_VALUE_P and buffer local_flags field, Spencer Baugh, 2021/05/06
bug#48264: [PATCH v3 13/15] Assert that PER_BUFFER_IDX for Lisp variables is not 0, Spencer Baugh, 2021/05/06
bug#48264: [PATCH v3 14/15] Remove PER_BUFFER_IDX and buffer_local_flags, Spencer Baugh, 2021/05/06
bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/06
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Eli Zaretskii, 2021/05/07
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/07
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros,
Eli Zaretskii <=
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/07
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Eli Zaretskii, 2021/05/08
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/08
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Eli Zaretskii, 2021/05/08
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/08
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/08
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Eli Zaretskii, 2021/05/09
- bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros, Spencer Baugh, 2021/05/09
- bug#48264: [PATCH 1/2] Take buffer field name in DEFVAR_PER_BUFFER, Spencer Baugh, 2021/05/09
- bug#48264: [PATCH 2/2] Add compile-time check that BVAR is used correctly, Spencer Baugh, 2021/05/09