GNU bug report logs -
#75318
Lisp_Subr header and layout
Previous Next
Full log
View this message in rfc822 format
> Date: Wed, 22 Jan 2025 15:30:24 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: Andrea Corallo <acorallo <at> gnu.org>, 75318 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
>
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>
> >> 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
>
> That statement applies to the status quo before the patch just as much
> as it does to the new status, though, right?
Could be; I didn't check. It's a general statement.
> > safe changes that don't break some build. Many/most people don't try
>
> This doesn't; it's impossible ta safely use the command_modes field for
> the reasons I explained.
Can you point me to the explanation, or explain again?
> And making it safe without using it at all seems both wasteful and all
> but impossible: we'd have to test the code somehow, making up usages of
> a field that hasn't been in real use for a long time.
I just wanted to avoid breaking a build by referencing the struct
member in a build that doesn't have it. There's no need to ensure the
member is always used safely. Doesn't initializing it to nil solve
this?
>
> > 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.)
>
> I agree, but I don't see how that applies here. Before the patch, if
> someone had started using nativecomp-only command_modes, we would have
> had very tricky GC/pdumper bugs which would have taken much longer to
> resolve than simply removing an unused field.
Well, both problems exist, they just push us in opposite directions.
> The new patch does still make it possible for someone using only
> nativecomp builds to accidentally use the field for non-nativecomp
> subrs. If someone tried to build that tree without nativecomp, they
> would get a compiler error, which is the best we can do to warn them of
> this situation, given the constraint that we use the same structure for
> nativecomp subrs and primitive ones.
OTOH, adding code that references the field without #ifdef, and
installing it without trying a non-nativecomp build, will break
everybody who builds --without-native-compilation.
> People might use it, and run into the various bugs that have
> accumulated. It's better to remove the unusable field (and there's all
> that talk about not wasting two words of staticvec, but wasting 1,000
> words is suddenly okay?).
If this leads to such a significant memory waste, then indeed what I
proposed is a non-starter, unfortunately.
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.