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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 21 Jun 2025 09:56:41 +0300

> From: Helmut Eller <[email-masked]>
> Cc: Pip Cet <[email-masked]>,  [email-masked]
> Date: Fri, 20 Jun 2025 20:29:40 +0200
> 
> On Fri, Jun 20 2025, Eli Zaretskii wrote:
> 
> > Thanks, but I asked about this change on its own, not as part of some
> > effort (which AFAIU is on the igc branch, not on master).  IOW,
> > 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?
> 
> It would add some minor protection against accidental modifications of
> the face_attr_sym array.

How can it be accidentally modified?  It's a static array not exposed
to Lisp.  Its purpose is to avoid code that mentions the attribute
keywords and their LFACE_* indices literally, as in

  if (attr_filter == LFACE_FOUNDRY_INDEX && EQ (keyword, QCfoundry)
      || attr_filter == LFACE_SWIDTH_INDEX && EQ (keyword, QCwidth)
      || attr_filter == LFACE_HEIGHT_INDEX && EQ (keyword, QCheight)
      || ...

etc.

More generally, the usual style in Emacs is to initialize stuff in
init_foo and syms_of_foo functions, which are called during the
various stages and variants of the initialization and startup.
Initialization using C initializers is used for simple values like
scalars.  The LISP_INITIALLY stuff should be used only very sparingly
and if strictly necessary, because it is very confusing at first
sight.  So I'd very much prefer that we kept the current conventions
for initializing complex variables, especially variables with Lisp
values.