Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <address@hidden> writes:

>> From: Oleh Krehel <address@hidden>
>> Date: Thu, 23 Apr 2015 15:30:26 +0200
>> Cc: address@hidden
>> 
>> we should remove the macros that don't do anything.
>
> What this macro does is allow you to use field names like 'foo', when
> the field is really called 'foo_'.
>
> I think it's okay to remove INTERNAL_FIELD, but I think we should keep
> the trailing underscore appended in BVAR and KVAR.  That's how all
> this started: the fields were renamed to have a trailing underscore so
> that code that used foo->bar instead of BVAR (foo, bar) would be
> immediately flagged by the compiler.
>
>> As for accidental access, I'm sure these rare errors will be caught by
>> the code review / test suite.
>
> We don't want to rely on code reviews, since they are very informal
> and their coverage is too low to be efficient.
>
> Based on bitter past experience with similar errors that lay low for
> months, sometimes for years, I'd rather not give up those underscores
> in BVAR and KVAR, which means the struct fields should retain them.

I'm totally fine with this:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->Vlast_kbd_macro_ = val;
    }

just as I'm fine with this:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->Vlast_kbd_macro = val;
    }

Both are boilerplate that doesn't require additional thought. In the
first case, it's maybe more explicit that `Vlast_kbd_macro_' should not
be accessed outside the interface function `kset_last_kbd_macro'.

But this I don't like:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->INTERNAL_FIELD (Vlast_kbd_macro) = val;
    }

It's not obvious how simple or intricate INTERNAL_FIELD is or what it
does. At the first glance, looks like C++ member function call.

Oleh