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

From: Pip Cet <[email-masked]>
Date: Sun, 12 Jan 2025 22:53:15 +0000

Pip Cet <[email-masked]> writes:

> "Stefan Kangas" <[email-masked]> writes:
>
>> Gerd Möllmann <[email-masked]> writes:
>>
>>> Stefan Kangas <[email-masked]> writes:
>>>
>>>> Severity: wishlist
>>>>
>>>> Can we delete the unused macro DEFVAR_LISP_NOPROX?
>>>
>>> +1
>>
>> Thanks, done, closing.
>
> Thanks!
>
> On master, this comment might need revising (staticpro now easserts that
> the same variable isn't added twice):
>
> /* Similar but define a variable whose value is the Lisp Object stored
>    at address.  Two versions: with and without gc-marking of the C
>    variable.  The nopro version is used when that variable will be
>    gc-marked for some other reason, since marking the same slot twice
>    can cause trouble with strings.  */
> void
> defvar_lisp_nopro (struct Lisp_Fwd const *o_fwd, char const *namestring)
> {
>   eassert (o_fwd->type == Lisp_Fwd_Obj);
>   Lisp_Object sym = intern_c_string (namestring);
>   XBARE_SYMBOL (sym)->u.s.declared_special = true;
>   XBARE_SYMBOL (sym)->u.s.redirect = SYMBOL_FORWARDED;
>   SET_SYMBOL_FWD (XBARE_SYMBOL (sym), o_fwd);
> }
>
> Also on master, the single user of DEFVAR_LISP_NOPRO is questionable:
> font.c doesn't staticpro Vfont_weight_table because it appears in
> font_style_table, but then font_style_table is sometimes modified so it
> points to a new vector, and I don't see how Vfont_weight_table is
> protected then.
>
> GDB experiment seems to indicate it's not protected at all, but it's in
> the dump so it isn't freed either.  When --enable-checking is in use,
> however, pdumper.c will abort the next time it sees an object that
> wasn't marked during a previous GC run.

Patch:

>From 4ba048bad6ff21ab95a5aeb2cbf33dc825890949 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Sun, 12 Jan 2025 22:45:55 +0000
Subject: [PATCH] Remove DEFVAR_LISP_NOPRO

* src/font.c (syms_of_font): Use 'DEFVAR_LISP', not 'DEFVAR_LISP_NOPRO'.
* src/lisp.h (DEFVAR_LISP_NOPRO): Macro removed.
* src/lread.c (defvar_lisp_nopro): Function removed.
(defvar_lisp): Inline defvar_lisp_nopro here.
---
 src/font.c  |  6 +++---
 src/lisp.h  |  7 -------
 src/lread.c | 13 ++-----------
 3 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/src/font.c b/src/font.c
index 86382267a4a..e6c41e41258 100644
--- a/src/font.c
+++ b/src/font.c
@@ -5954,7 +5954,7 @@ syms_of_font (void)
      table used by the font display code.  So we make them read-only,
      to avoid this confusing situation.  */
 
-  DEFVAR_LISP_NOPRO ("font-weight-table", Vfont_weight_table,
+  DEFVAR_LISP ("font-weight-table", Vfont_weight_table,
 	       doc: /*  Vector of valid font weight values.
 Each element has the form:
     [NUMERIC-VALUE SYMBOLIC-NAME ALIAS-NAME ...]
@@ -5963,14 +5963,14 @@ syms_of_font (void)
   Vfont_weight_table = BUILD_STYLE_TABLE (weight_table);
   make_symbol_constant (intern_c_string ("font-weight-table"));
 
-  DEFVAR_LISP_NOPRO ("font-slant-table", Vfont_slant_table,
+  DEFVAR_LISP ("font-slant-table", Vfont_slant_table,
 	       doc: /*  Vector of font slant symbols vs the corresponding numeric values.
 See `font-weight-table' for the format of the vector.
 This variable cannot be set; trying to do so will signal an error.  */);
   Vfont_slant_table = BUILD_STYLE_TABLE (slant_table);
   make_symbol_constant (intern_c_string ("font-slant-table"));
 
-  DEFVAR_LISP_NOPRO ("font-width-table", Vfont_width_table,
+  DEFVAR_LISP ("font-width-table", Vfont_width_table,
 	       doc: /*  Alist of font width symbols vs the corresponding numeric values.
 See `font-weight-table' for the format of the vector.
 This variable cannot be set; trying to do so will signal an error.  */);
diff --git a/src/lisp.h b/src/lisp.h
index e3142f3b8cc..7646a3dd5f1 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3532,7 +3532,6 @@ call0 (Lisp_Object fn)
 }
 
 extern void defvar_lisp (struct Lisp_Objfwd const *, char const *);
-extern void defvar_lisp_nopro (struct Lisp_Objfwd const *, char const *);
 extern void defvar_bool (struct Lisp_Boolfwd const *, char const *);
 extern void defvar_int (struct Lisp_Intfwd const *, char const *);
 extern void defvar_kboard (struct Lisp_Kboard_Objfwd const *, char const *);
@@ -3560,12 +3559,6 @@ #define DEFVAR_LISP(lname, vname, doc)		\
       = {Lisp_Fwd_Obj, &globals.f_##vname};	\
     defvar_lisp (&o_fwd, lname);		\
   } while (false)
-#define DEFVAR_LISP_NOPRO(lname, vname, doc)	\
-  do {						\
-    static struct Lisp_Objfwd const o_fwd	\
-      = {Lisp_Fwd_Obj, &globals.f_##vname};	\
-    defvar_lisp_nopro (&o_fwd, lname);		\
-  } while (false)
 #define DEFVAR_BOOL(lname, vname, doc)		\
   do {						\
     static struct Lisp_Boolfwd const b_fwd	\
diff --git a/src/lread.c b/src/lread.c
index ab900b3bee6..d6fa84bb826 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -5510,23 +5510,14 @@ defvar_bool (struct Lisp_Boolfwd const *b_fwd, char const *namestring)
 }
 
 /* Similar but define a variable whose value is the Lisp Object stored
-   at address.  Two versions: with and without gc-marking of the C
-   variable.  The nopro version is used when that variable will be
-   gc-marked for some other reason, since marking the same slot twice
-   can cause trouble with strings.  */
+   at address.  */
 void
-defvar_lisp_nopro (struct Lisp_Objfwd const *o_fwd, char const *namestring)
+defvar_lisp (struct Lisp_Objfwd const *o_fwd, char const *namestring)
 {
   Lisp_Object sym = intern_c_string (namestring);
   XBARE_SYMBOL (sym)->u.s.declared_special = true;
   XBARE_SYMBOL (sym)->u.s.redirect = SYMBOL_FORWARDED;
   SET_SYMBOL_FWD (XBARE_SYMBOL (sym), o_fwd);
-}
-
-void
-defvar_lisp (struct Lisp_Objfwd const *o_fwd, char const *namestring)
-{
-  defvar_lisp_nopro (o_fwd, namestring);
   staticpro (o_fwd->objvar);
 }
 
-- 
2.47.1