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

From: Jens Schmidt <[email-masked]>
Date: Sat, 15 Mar 2025 22:50:34 +0100

On 2025-03-15  20:13, Stefan Monnier wrote:

> That would be the poor man's choice, but given you already wrote the
> code to do it better, I wouldn't make that choice.

Ok, kept the other choice, implemented your and Eli's feedback,
added tests, both for the new function and for unloading a feature
that defines a variable alias.

Please see the attached patch.

Thanks
From 5828cd35ba2d79b4f4ce6276463d9a5252dc179d 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/eval.c (Finternal_delete_indirect_variable): Add function.
* lisp/loadhist.el (loadhist-unload-element): Use it for variable
aliases.
* test/src/eval-tests.el (eval-tests--internal-delete-indirect-variable):
Test function `internal-delete-indirect-variable'.
* test/lisp/loadhist-tests.el (loadhist-test-unload-feature-alias):
* test/lisp/loadhist-resources/loadhist--alias.el: Test unloading of
features that define variable aliases.  (Bug#76748)
---
 lisp/loadhist.el                              | 10 +++++--
 src/eval.c                                    | 19 +++++++++++++
 .../loadhist-resources/loadhist--alias.el     | 28 +++++++++++++++++++
 test/lisp/loadhist-tests.el                   |  8 ++++++
 test/src/eval-tests.el                        | 12 ++++++++
 5 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 test/lisp/loadhist-resources/loadhist--alias.el

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/eval.c b/src/eval.c
index 5de723cf3bc..335fdae8ab9 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -720,6 +720,24 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
   return base_variable;
 }
 
+DEFUN ("internal-delete-indirect-variable", Finternal_delete_indirect_variable, Sinternal_delete_indirect_variable,
+       1, 1, 0,
+       doc: /* Internal use only.
+Undeclare SYMBOL as variable alias, then unbind it.
+Return SYMBOL.  */)
+  (register Lisp_Object symbol)
+{
+  CHECK_SYMBOL (symbol);
+  if (XSYMBOL (symbol)->u.s.redirect != SYMBOL_VARALIAS)
+    xsignal2 (Qerror,
+	      build_string ("Cannot undeclare a variable that is not an alias"),
+	      symbol);
+  XSYMBOL (symbol)->u.s.redirect = SYMBOL_PLAINVAL;
+  Fput (symbol, Qvariable_documentation, Qnil);
+  Fset (symbol, Qunbound);
+  return symbol;
+}
+
 static union specbinding *
 default_toplevel_binding (Lisp_Object symbol)
 {
@@ -4488,6 +4506,7 @@ syms_of_eval (void)
   defsubr (&Sdefvar_1);
   defsubr (&Sdefvaralias);
   DEFSYM (Qdefvaralias, "defvaralias");
+  defsubr (&Sinternal_delete_indirect_variable);
   defsubr (&Sdefconst);
   defsubr (&Sdefconst_1);
   defsubr (&Sinternal__define_uninitialized_variable);
diff --git a/test/lisp/loadhist-resources/loadhist--alias.el b/test/lisp/loadhist-resources/loadhist--alias.el
new file mode 100644
index 00000000000..96f1b77c48d
--- /dev/null
+++ b/test/lisp/loadhist-resources/loadhist--alias.el
@@ -0,0 +1,28 @@
+;;; loadhist--alias.el --- Dummy package for loadhist-tests  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025 Free Software Foundation, Inc.
+
+;; Author: Jens Schmidt <[email-masked]>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(defvaralias 'loadhist--alias-last-input-event 'last-input-event
+  "Alias on built-in variable.")
+
+(provide 'loadhist--alias)
+;;; loadhist--alias.el ends here
diff --git a/test/lisp/loadhist-tests.el b/test/lisp/loadhist-tests.el
index 0f1cedf9c7f..3bc8bbb0189 100644
--- a/test/lisp/loadhist-tests.el
+++ b/test/lisp/loadhist-tests.el
@@ -101,4 +101,12 @@ loadhist-tests-unload-feature-notnested
   (should (null (get 'loadhist--bar-dec 'function-history)))
   (should (null (get 'loadhist--foo-inc 'function-history))))
 
+(ert-deftest loadhist-test-unload-feature-alias ()
+  "Check that bug#76748 has been fixed."
+  (add-to-list 'load-path (expand-file-name
+                           "loadhist-resources/"
+                           loadhist--tests-dir))
+  (load "loadhist--alias" nil t)
+  (unload-feature 'loadhist--alias))
+
 ;;; loadhist-tests.el ends here
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index c5a46b62ee2..64a108f8744 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -282,6 +282,18 @@ eval-tests-defvaralias
   (should-error (defvaralias 'eval-tests--my-c 'eval-tests--my-d)
                 :type 'cyclic-variable-indirection))
 
+(ert-deftest eval-tests--internal-delete-indirect-variable ()
+  (defvar eval-tests--i-d-i-v-var 'foo)
+  (defvaralias 'eval-tests--i-d-i-v-var1 'eval-tests--i-d-i-v-var "Doc string.")
+  (internal-delete-indirect-variable 'eval-tests--i-d-i-v-var1)
+
+  (should (eq (indirect-variable 'eval-tests--i-d-i-v-var1)
+              'eval-tests--i-d-i-v-var1))
+  (should-not (boundp 'eval-tests--i-d-i-v-var1))
+  (should-not (get 'eval-tests--i-d-i-v-var1 'variable-documentation))
+
+  (should-error (internal-delete-indirect-variable 'eval-tests--i-d-i-v-var)))
+
 (defvar eval-tests/global-var 'global-value)
 (defvar-local eval-tests/buffer-local-var 'default-value)
 (ert-deftest eval-tests/default-value ()
-- 
2.39.5