Subject: Re: bug#75521: scratch/igc: Delete unused macro DEFVAR_LISP_NOPROX

From: Eli Zaretskii <[email-masked]>
Date: Mon, 13 Jan 2025 21:35:04 +0200

> Date: Mon, 13 Jan 2025 19:18:59 +0000
> From: Pip Cet <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> "Eli Zaretskii" <[email-masked]> writes:
> 
> > So you are saying that the problem that we don't assign the new value
> > (received from vconcat) to the appropriate variable
> > (Vfont_weight_table etc.)?
> 
> My first patch assumes that behavior was intentional, and retains it; my
> second patch assumes it wasn't, and changes things so font_style_table
> is no longer needed, because we just use Vfont_weight_table directly in
> its place.

If we remove font_style_table, then all the code in font.c which uses
that table and accesses Vfont_weight_table etc. by indexing into
font_style_table, will stop working, no?  So I think I'd prefer to
just assign the new value of the table, as returned by vconcat, to the
corresponding variable (Vfont_weight_table etc.), and leave the rest
of the code intact.  That sounds like a much smaller change which only
touches a single place, where the problem is.  Are there any problems
with this?

> We should not attempt to keep two different memory locations
> synchronized by trying to catch all modifications.  That never works,
> and there's no reason for this complication.

I don't think I follow: which modifications are you talking about?  If
that's Vfont_weight_table etc., then these 3 are documented to be
unmodifiable directly.

> >> > GC can only free objects that are not reachable from any other object.
> >>
> >> No, that's not correct: the value of a forwarded symbol is not marked by
> >> alloc.c GC.
> >>
> >> 	      case SYMBOL_FORWARDED:
> >> 		/* If the value is forwarded to a buffer or keyboard field,
> >> 		   these are marked when we see the corresponding object.
> >> 		   And if it's forwarded to a C variable, either it's not
> >> 		   a Lisp_Object var, or it's staticpro'd already, or it's
> >> 		   reachable from font_style_table which is also
> >> 		   staticpro'd.  */
> >> 		break;
> >>
> >> That comment is incorrect, because font_style_table may contain a
> >> newly-consed vector, not the one that's still referred to as
> >> font-weight-table.
> >
> > Sorry, you lost me here.
> 
> As I've tried to explain a number of times, the problem is that AREF
> (font_style_table, 0) isn't necessarily equal to Vfont_weight_table.
> The comment claims it is, but that's incorrect.

Ah, so it's a separate issue with that comment.