GNU bug report logs - #75318
Lisp_Subr header and layout

Previous Next

Package: emacs;

Reported by: Pip Cet <pipcet <at> protonmail.com>

Date: Fri, 3 Jan 2025 14:31:02 UTC

Severity: normal

Full log


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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>,
 Andrea Corallo <acorallo <at> gnu.org>
Cc: monnier <at> iro.umontreal.ca, 75318 <at> debbugs.gnu.org
Subject: Re: bug#75318: Lisp_Subr header and layout
Date: Wed, 22 Jan 2025 15:31:00 +0200
> Cc: monnier <at> iro.umontreal.ca
> 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" <bug-gnu-emacs <at> gnu.org>
> 
> 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.




This bug report was last modified 144 days ago.

Previous Next


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