bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros

[Spencer Baugh]

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...