Subject: bug#75318: Lisp_Subr header and layout

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

> Cc: [email-masked]
> Date: Tue, 21 Jan 2025 23:46:45 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> diff --git a/src/lisp.h b/src/lisp.h
> index 9c31630cc1a..46e940dad09 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -2226,12 +2226,12 @@ CHAR_TABLE_SET (Lisp_Object ct, int idx, Lisp_Object val)
>        const char *string;
>        Lisp_Object native;
>      } intspec;
> -    Lisp_Object command_modes;
>      /* Positive values: offset into etc/DOC.  Negative values: one's
>         complement of index into the native comp unit's vector of
>         documentation strings.  */
>      EMACS_INT doc;
>  #ifdef HAVE_NATIVE_COMP
> +    Lisp_Object command_modes;
>      Lisp_Object native_comp_u;
>      char *native_c_name;
>      Lisp_Object lambda_list;

IME, having different definitions of data structures under different
build conditions is a problematic solution: it makes it harder to make
safe changes that don't break some build.  Many/most people don't try
to build all of the configurations Emacs supports when they make
changes, so it is easy to write code which references a struct member
that doesn't exist in some build configuration, and thus break it, and
miss the fact that build is broken by not trying it.  (It is
unrealistic to expect everyone who has write access to the repository
to test-build all the possible configurations Emacs supports as a
prerequisite for installing a change.)

So: do we really need to remove command_modes from the struct in a
build without native compilation?  What bad things will happen if we
leave it there?  If we would like to be a bit more efficient by
avoiding a few function calls, like here:

> --- a/src/pdumper.c
> +++ b/src/pdumper.c
> @@ -2992,16 +2992,15 @@ dump_subr (struct dump_context *ctx, const struct Lisp_Subr *subr)
>                               COLD_OP_NATIVE_SUBR,
>  			     make_lisp_ptr ((void *) subr, Lisp_Vectorlike));
>        dump_field_lv (ctx, &out, subr, &subr->intspec.native, WEIGHT_NORMAL);
> -      dump_field_lv (ctx, &out, subr, &subr->command_modes, WEIGHT_NORMAL);
>      }
>    else
>      {
>        dump_field_emacs_ptr (ctx, &out, subr, &subr->symbol_name);
>        dump_field_emacs_ptr (ctx, &out, subr, &subr->intspec.string);
> -      dump_field_emacs_ptr (ctx, &out, subr, &subr->command_modes);
>      }

then it should be okay to #ifdef away those calls when
!HAVE_NATIVE_COMP, as that doesn't run the risks mentioned above.

I also would like to hear from Andrea about this patch.

Thanks.