Subject: bug#76748: 31.0.50; `unload-feature' unbinds NEW-ALIAS and BASE-VARIABLE of variable alias

From: Jens Schmidt <[email-masked]>
Date: Fri, 14 Mar 2025 23:14:05 +0100

On 2025-03-12  23:53, Stefan Monnier wrote:

> You could also choose to keep that function "internal"
> and not document it in the manual/NEWS (like we did for
> `internal-make-var-non-special`), mostly justified in case its
> semantics is delicate/messy/ugly.

I decided to do exactly that - if anybody else except `unload-feature'
is ever going to need that function, she/he can do the documentation.
Will add some tests, anyway, when the rest of this has been decided
upon.

Adding the function `internal-delete-indirect-variable' and calling it
from loadhist.el was easy enough (too easy?), please see the attached
patch for a first draft.  I decided that the right way to unbind an
alias, as far as `unload-feature' is concerned, is to make it a regular
variable and unbind that.

More questions:

1. I have a bad feeling about butting in between 1st class citizens
   makunbound and fmakunbound in data.c, but couldn't find a better
   other place for that function.

2. Before or after switching the variable from an alias to a plain one
   in `internal-delete-indirect-variable', do I somehow need to clean or
   zero ...val->alias before overwriting it as ...val->value with
   "Fset (symbol, Qunbound);"?

3. defvaralias attaches the new alias to the load history as symbol.
   One could also attach it as (Fcons (Qalias, new_alias)) and provide a
   new cl method loadhist-unload-element for that case instead of handling
   aliases together with regular symbols.

Finally, `defvaralias' and `define-obsolete-variable-alias' have a
number of other side-effects, which I have not covered in function
`internal-delete-indirect-variable'.  Please let me know if I should
care about any of these:

4. defvaralias potentially calls

     notify_variable_watchers (new_alias, base_variable, Qdefvaralias, Qnil);

5. defvaralias declares both new alias and base symbol as special.

6. defvaralias sets

     XSYMBOL (new_alias)->u.s.trapped_write =
       XSYMBOL (base_variable)->u.s.trapped_write;

7. defvaralias puts the alias docstring into the variable-documentation
   property.

8. define-obsolete-variable-alias copies properties `saved-value',
   `saved-variable-comment'  from the obsolete to the non-obsolete
   symbol.

9. define-obsolete-variable-alias sets property `byte-obsolete-variable'
   on the obsolete symbol.

Thanks.
From d99bf052921f589ac90c850656fa2c44559e9218 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <[email-masked]>
Date: Fri, 14 Mar 2025 23:07:11 +0100
Subject: [PATCH] Correctly unload variable aliases.

* src/data.c (Finternal_delete_indirect_variable): Add new function.
* lisp/loadhist.el (loadhist-unload-element): Use it for variable
aliases.  (Bug#76748)
---
 lisp/loadhist.el | 10 +++++++---
 src/data.c       | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/lisp/loadhist.el b/lisp/loadhist.el
index 1a6f434561e..10b61f184d6 100644
--- a/lisp/loadhist.el
+++ b/lisp/loadhist.el
@@ -211,9 +211,13 @@ loadhist-unload-element
       (kill-local-variable x)))
   (if (and (boundp x) (timerp (symbol-value x)))
       (cancel-timer (symbol-value x)))
-  ;; Get rid of the default binding if we can.
-  (unless (local-variable-if-set-p x)
-    (makunbound x)))
+  (cond
+   ;; "Unbind" indirect variable.
+   ((not (eq (indirect-variable x) x))
+    (internal-delete-indirect-variable x))
+   ;; Get rid of the default binding if we can.
+   ((not (local-variable-if-set-p x))
+    (makunbound x))))
 
 (cl-defmethod loadhist-unload-element ((x (head define-type)))
   (let* ((name (cdr x)))
diff --git a/src/data.c b/src/data.c
index 5d1c065e785..fca1f73f464 100644
--- a/src/data.c
+++ b/src/data.c
@@ -773,6 +773,22 @@ DEFUN ("makunbound", Fmakunbound, Smakunbound, 1, 1, 0,
   return symbol;
 }
 
+DEFUN ("internal-delete-indirect-variable", Finternal_delete_indirect_variable, Sinternal_delete_indirect_variable,
+       1, 1, 0,
+       doc: /* Undeclare SYMBOL as variable alias, then unbind it.
+If SYMBOL is not a variable alias, unbind it like `makunbound' does.
+Return SYMBOL.  */)
+  (register Lisp_Object symbol)
+{
+  CHECK_SYMBOL (symbol);
+  if (SYMBOL_CONSTANT_P (symbol))
+    xsignal1 (Qsetting_constant, symbol);
+  else if (XSYMBOL (symbol)->u.s.redirect == SYMBOL_VARALIAS)
+    XSYMBOL (symbol)->u.s.redirect = SYMBOL_PLAINVAL;
+  Fset (symbol, Qunbound);
+  return symbol;
+}
+
 DEFUN ("fmakunbound", Ffmakunbound, Sfmakunbound, 1, 1, 0,
        doc: /* Make SYMBOL's function definition be nil.
 Return SYMBOL.
@@ -4263,6 +4279,7 @@ #define PUT_ERROR(sym, tail, msg)			\
   defsubr (&Sremove_pos_from_symbol);
   defsubr (&Sposition_symbol);
   defsubr (&Smakunbound);
+  defsubr (&Sinternal_delete_indirect_variable);
   defsubr (&Sfmakunbound);
   defsubr (&Sboundp);
   defsubr (&Sfboundp);
-- 
2.39.5