GNU bug report logs - #78813
feature/igc: [PATCH] Make the array face_attr_sym readonly

Previous Next

Package: emacs;

Reported by: Helmut Eller <eller.helmut <at> gmail.com>

Date: Tue, 17 Jun 2025 08:50:02 UTC

Severity: normal

Tags: patch

Full log


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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78813 <at> debbugs.gnu.org, eller.helmut <at> gmail.com
Subject: Re: bug#78813: feature/igc: [PATCH] Make the array face_attr_sym
 readonly
Date: Fri, 20 Jun 2025 18:31:32 +0000
"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Fri, 20 Jun 2025 10:17:46 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: eller.helmut <at> gmail.com, 78813 <at> debbugs.gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> Date: Tue, 17 Jun 2025 15:28:57 +0000
>> >> From: Pip Cet <pipcet <at> protonmail.com>
>> >> Cc: 78813 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
>> >>
>> >> "Helmut Eller" <eller.helmut <at> gmail.com> writes:
>> >>
>> >> > Tags: patch
>> >> >
>> >> > This is a proposal to make the variable face_attr_sym const.  Mostly so
>> >> > that it's more obvious that it doesn't need to be traced by the GC.
>> >>
>> >> Makes sense to me, but should be on master.  Is this okay for master?
>> >
>> > What are the advantages of making this change on master?
>>
>> As on feature/igc, the change only makes it harder to break things by
>> changing the rest of the code; there's no functional change on either
>> branch, and while Helmut's patch is likely to reduce code size a little,
>> that's not the reason we should install it.
>>
>> I don't know whether we actually finally agreed that unprotected static
>> Lisp_Objects should be phased out, but IIRC that's what Gerd suggested
>> very strongly.  This change is part of that effort.
>
> Thanks, but I asked about this change on its own, not as part of some

That's the first paragraph: mainly, it changes code to be less risky in
its no-need-to-GC assumptions, so the very minor changes which would
cause those assumptions to be violated won't result in very tricky bugs.

> effort (which AFAIU is on the igc branch, not on master).  IOW,

Sorry I'm asking for clarification, but my impression was the igc branch
was to change very little in the !HAVE_MPS case, because general GC
cleanups such as this one make more sense on the main branch: fewer
merge conflicts, we'll see the benefits before merging the branch (or
even if for some reason we never do), a better git history, better test
coverage for pre-push testing.

(This isn't about improving code specific to the old GC; I understand you
don't want it touched and I'm working on the assumption that it'll
become unused or go away entirely at some point.)

If we decide not to reduce the number of static objects at this time and
focus on getting things to work, I can live with that and continue
working on the branch, but I can't speak for Helmut, of course.

Diverting all work that's even slightly related to GC to the feature/igc
branch and keeping master in its current state seems like a very risky
proposal to me.

So what shall we do?

> suppose we release Emacs 31 without the MPS support (something I hope
> will not need to happen, but just for the sake of this sub-thread):
> what would that change give us, considering the fact that this code
> was there since Emacs 21?

Perhaps Helmut can add something to what I wrote above.

Pip





This bug report was last modified today.

Previous Next


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