GNU bug report logs -
#78813
feature/igc: [PATCH] Make the array face_attr_sym readonly
Previous Next
Full log
View this message in rfc822 format
"Eli Zaretskii" <eliz <at> gnu.org> writes:
>> Date: Fri, 20 Jun 2025 18:31:32 +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:
>>
>> >> > 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.
>
> See my other message where I explained why I'd like not to make such
> changes, if that is the only advantage.
>> > 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.
>
> That's true in general, but if some change in the non-MPS case makes
> the overall code cleaner, there should be no reason not to make it on
> the branch.
I can't parse that statement. If a change affects only the non-MPS
case, which branch should we make it on? The master branch would make
sense to me, and the feature/igc branch doesn't make sense.
> As for merge conflicts, I'm not afraid of that, especially if we make
> changes in an area that doesn't see too many changes (initializations
> are generally of that nature).
Well, you did just push a change which causes a merge conflict here.
Helmut, as this discussion shows, I see no way to apply changes such as
this one right now; they won't be accepted in master, apparently, and
applying them to feature/igc will produce merge conflicts, as we've just
seen.
>> 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.
>
> Maybe I'm missing something, but what does this change have to do with
> GC? This particular static object is never modified and is not
> exposed to Lisp. So it shouldn't cause us any GC-related efforts.
This confuses me. The unusual thing here is precisely that we have a
static object which is not staticpro'd but which we know not to cause GC
problems. Helmut said so clearly when he posted the patch. "Never
modified and not exposed to Lisp" is irrelevant to the question of
whether GC needs to know about the object.
This single change isn't critical, of course, but feature/igc already
contains too many changes which should be merged to master before we
ever consider merging the branch as a whole. If the strategy is to
increase that number further, that makes a sensible merge of feature/igc
much less likely.
Pip
This bug report was last modified 25 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.