Package: emacs;
Reported by: Pip Cet <pipcet <at> protonmail.com>
Date: Fri, 3 Jan 2025 14:31:02 UTC
Severity: normal
View this message in rfc822 format
From: Pip Cet <pipcet <at> protonmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: luangruo <at> yahoo.com, monnier <at> iro.umontreal.ca, 75318 <at> debbugs.gnu.org Subject: bug#75318: Lisp_Subr header and layout Date: Wed, 22 Jan 2025 14:25:22 +0000
"Eli Zaretskii" <eliz <at> gnu.org> writes:
>> Cc: Po Lu <luangruo <at> yahoo.com>, monnier <at> iro.umontreal.ca
>> 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" <bug-gnu-emacs <at> gnu.org>
>>
>> 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.
I do. I agree completely. I don't know how to fix it.
XIL is necessary for CHECK_LISP_OBJECT_TYPE (and there seems to be a
consensus we can't unconditionally enable that one yet), but I don't see
why we can't make an exception for Qnil and make the simple definition
of lisp_h_Qnil apply to this specific symbol in all circumstances:
#define Qnil XIL(0)
If we used that instead, it would make LISPSYM_INITIALLY slightly harder
to find because fewer places would use it.
> 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
If you're saying that LISPSYM_INITIALLY is bad, I agree completely.
If you're saying a single LISPSYM_INITIALLY in DEFUN, which is
well-documented to the point that users do not have to read the
definition, is worse than three of them in C files, I don't.
> #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?
I had forgotten how hard it was to get used to this.
I think we should definitely avoid making the situation worse, even if
that means we can't do fun stuff.
> (No, I don't expect you to change anything in your patch, just realize
> the plight.)
Thank you, in any case.
>> +#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?
Do you mean
#ifdef HAVE_NATIVE_COMP
#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), \
.native_comp_u = LISPSYM_INITIALLY (Qnil), \
.lambda_list = LISPSYM_INITIALLY (Qnil), \
.type = LISPSYM_INITIALLY (Qnil),
}}
#else
#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 }, \
}}
#endif
(and, I assume, the same thing for SUBR_HEADER?)
Avoiding the repetition appears to require a second macro no matter what
I do.
While *I* realize *that* battle was lost long ago, I still think
nativecomp subrs should have had their own PVEC type.
It's too late for that now; let's not revisit a discussion that I
remember as very fair, and which definitely concluded that we'd share
the PVEC type.
I'm thinking (very vaguely; no patch expected in 2025) about proposing
to reduce all Lisp_Objects in the subr object to a simple plist, though,
or a vector if we have to. I think 32 bytes is a very good size for a
subr to have, and that's header, plist (Lisp_Object so first),
minargs+maxargs+4-byte gap, function pointer union.
It's easier to GC-trace an ordinary PVEC with lisplen=1 and restlen=2
without worrying about whether it is a subr, I tihnk.
Pip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.