Subject: Re: bug#75318: Lisp_Subr header and layout

From: Eli Zaretskii <[email-masked]>
Date: Wed, 22 Jan 2025 15:17:54 +0200

> Cc: Po Lu <[email-masked]>, [email-masked]
> Date: Tue, 21 Jan 2025 23:11:13 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> This patch splits up the macros so they're reusable, initializes using
> LISPSYM_INITIALLY rather than lisp_h_Qnil, ensures that all Lisp_Object
> fields are initialized, uses explicit field-name initializers, and uses
> c-mode default indentation (this is incorrect in one place, but it's
> hardly noticeable).

While I realize that this is a battle lost long ago, I cannot resist
lamenting the once-simple way many lisp.h macros and inline functions
were defined, which is all but gone now, replaced by the never-ending
rabbit hole of macros that call macros that call macros that call...
Add to this the fact that each macro/inline function has several
different definitions, whose conditions are sometimes so complicated
that the only reasonable way to quickly understand what definition a
particular build uses is to fire up good-old GDB and evaluate the
macros there, and you get the picture.

For example, in the below:

>    static union Aligned_Lisp_Subr Swatch_gc_cons_percentage =
> -     {{{ PSEUDOVECTOR_FLAG | (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS) },
> -       { .a4 = watch_gc_cons_percentage },
> -       4, 4, "watch_gc_cons_percentage", {0}, lisp_h_Qnil}};

what is lisp_h_Qnil is relatively easy to understand, whereas the
replacement, viz.:

> +#define SUBR_INITIALIZER(lname, fnname, minargs, maxargs, intspec_value) \
> +  {{ .header = SUBR_HEADER,						\
> +    .function = { .a ## maxargs = fnname },				\
> +    .min_args = minargs,						\
> +    .max_args = maxargs,						\
> +    .symbol_name = lname,						\
> +    .intspec = { .string = intspec_value },				\
> +    .command_modes = LISPSYM_INITIALLY (Qnil),			\

requires one to look up LISPSYM_INITIALLY, which is

  #define LISPSYM_INITIALLY(name) \
    TAG_PTR_INITIALLY (Lisp_Symbol, (intptr_t) ((i##name) * sizeof *lispsym))

and then you need to go down the rabbit hole:

  TAG_PTR_INITIALLY
   => LISP_INITIALLY
       => LISP_WORD_TAG
           => VALBITS

Wonderful, right?

(No, I don't expect you to change anything in your patch, just realize
the plight.)

> +#ifdef HAVE_NATIVE_COMP
> +#define SUBR_NATIVECOMP_INITIALIZER			\
> +    .native_comp_u = LISPSYM_INITIALLY (Qnil),		\
> +    .lambda_list = LISPSYM_INITIALLY (Qnil),		\
> +    .type = LISPSYM_INITIALLY (Qnil),
> +#else
> +#define SUBR_NATIVECOMP_INITIALIZER
> +#endif
> +
> +#define SUBR_INITIALIZER(lname, fnname, minargs, maxargs, intspec_value) \
> +  {{ .header = SUBR_HEADER,						\
> +    .function = { .a ## maxargs = fnname },				\
> +    .min_args = minargs,						\
> +    .max_args = maxargs,						\
> +    .symbol_name = lname,						\
> +    .intspec = { .string = intspec_value },				\
> +    .command_modes = LISPSYM_INITIALLY (Qnil),				\
> +    SUBR_NATIVECOMP_INITIALIZER						\
> +    }}
> +

To make the depth of the proverbial rabbit hole at least to some
extent smaller here, could we please refrain from defining
SUBR_NATIVECOMP_INITIALIZER, and instead include the necessary stuff
directly in SUBR_INITIALIZER, with an #ifdef?  Or do you envision
enough other users of SUBR_NATIVECOMP_INITIALIZER to justify yet
another macro?

Thanks.