[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: |
Sat, 08 May 2021 09:55:38 +0300 |
> From: Spencer Baugh <sbaugh@catern.com>
> Cc: 48264@debbugs.gnu.org
> Date: Fri, 07 May 2021 17:43:51 -0400
>
> > 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.
>
> Hopefully. Although, I'm not sure this approach is fundamentally
> unreadable? The field names are already mangled with the trailing "_"
> to stop direct access; this is just further mangling them.
Yes, but it's much more than just the appended _.
Consider also the case of some developer instructing a user to provide
values of these fields in a GDB session: currently we need to tell the
user to use just "p foo->bar_". With this change, we'd need to make
the user type much more, and possibly also make sure Emacs is compiled
with -g3 to have the macros available to the debugger.
> >> 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?
>
> Possibly; by "defaulted" I intended to mean that the field is one which
> has a default. But I freely acknowledge it's not a great name.
So how about using _d_ of _def_instead? It's much shorter and
expresses the purpose no worse than _defaulted_.
> Keep in mind though, this name isn't exposed to the programmer
> anywhere - it might as well be _ABCDEFGHI_, nothing will change
> outside the definition of the BVAR_DEFAULTED_FIELD macro.
See above: I'd prefer to get rid of the macro for this purpose.
> > 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.
>
> A conditionally-compiled runtime check would be very easy to add - I'd
> just change BVAR to something like:
>
> (eassert (EQ (buffer_defaults->field ## _)); (buf)->field ## _)
>
> Which would make sure that it's not used on anything with a default.
> But of course that's substantially more annoying than a compile time
> check...
I'm not sure I understand why this is much more annoying, can you
elaborate? We have similar assertions, conditioned on
ENABLE_CHECKING, elsewhere in our macros, like XWINDOW etc, so why not
here?
- 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, 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/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
- bug#48264: [PATCH 2/2] Add compile-time check that BVAR is used correctly, Stefan Monnier, 2021/05/09
- bug#48264: [PATCH 2/2] Add compile-time check that BVAR is used correctly, Eli Zaretskii, 2021/05/09