GNU bug report logs - #75318
Lisp_Subr header and layout

Previous Next

Full log

Message #29 received at 75318 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: luangruo <at> yahoo.com, monnier <at> iro.umontreal.ca, 75318 <at> debbugs.gnu.org
Subject: Re: bug#75318: Lisp_Subr header and layout
Date: Wed, 22 Jan 2025 17:27:57 +0200

> Date: Wed, 22 Jan 2025 14:25:22 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75318 <at> debbugs.gnu.org, luangruo <at> yahoo.com, monnier <at> iro.umontreal.ca
> 
> > 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.

It isn't bad, it's that replacing lisp_h_Qnil with it makes the code
harder to understand (without going crazy with M-.).

As for LISPSYM_INITIALLY, its name is the main source of confusion:
the INITIALLY part all but prevents any attempt to get a clear idea
what the macro does, without actually looking up all the way to the
basics.  Unless on remembers that by heart, of course (I don't).

> > 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?)

No.  I meant to use #ifdef inside the macro.  But if that is
impossible in a C macro, I'd even support making this an inline
function.

Anyway, this is a minor comment, so if it's hard to do better, let's
not go overboard.  I'll survive.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.