Subject: bug#78813: feature/igc: [PATCH] Make the array face_attr_sym readonly

From: Pip Cet <[email-masked]>
Date: Fri, 20 Jun 2025 10:17:46 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Tue, 17 Jun 2025 15:28:57 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], Eli Zaretskii <[email-masked]>
>>
>> "Helmut Eller" <[email-masked]> 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.

I'd prefer LISPSYM_INITIALLY to go away at some point soon, so we can at
least benchmark whether putting lispsym into MPS-managed memory makes
things better or worse.  However, the current code uses
LISPSYM_INITIALLY implicitly, so using it explicitly would make such
changes easier, not harder.

Helmut, I notice both your code and the existing code omit the
LFACE_FAMILY_INDEX case.  Do you happen to know if there's a reason for
that?  If there is, we should add a comment to make sure no one attempts
to reuse this vector in a context where the QCfamily symbol is needed.

An alternative would be to make face_attr_sym a function containing a
switch statement.  Either approach would reduce the different kinds of
unusual static Lisp_Object-containing structures by one: we either use
symbols in code, which is fine, or we do the same thing as the font
drivers (const Lisp_Objects initialized with LISPSYM_INITIALLY).

We could also simply add a comment explaining the unusual situation
(and, possibly, the LFACE_FAMILY_INDEX case).

Pip