[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: |
Spencer Baugh |
Subject: |
bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros |
Date: |
Fri, 07 May 2021 17:43:51 -0400 |
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@catern.com>
>> Cc: 48264@debbugs.gnu.org
>> Date: Fri, 07 May 2021 08:59:36 -0400
>> 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.
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.
>> > 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?
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. 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.
>> >> +#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.
Yes, I hope someone comes up with a better idea than me.
> 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...
- 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 <=
- 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
- bug#48264: [PATCH 2/2] Add compile-time check that BVAR is used correctly, Stefan Monnier, 2021/05/09