From 62157d0e9490db57282f35d0ee9a84cc0b8628d2 Mon Sep 17 00:00:00 2001
From: "Robert A. Burks" <rburksdev@gmail.com>
Date: Sat, 9 Mar 2024 13:38:52 -0500
Subject: [PATCH 05/31] Fix+ert improper setting and notification in
 defvaralias (bug#00002)

'base_variable' was being set even when 'new_alias' was unbound.  Also,
it was improperly calling 'set_internal' with indication of let binding.

* src/eval.c (Fdefvaralias):
Fix variable watchers being improperly notified of an unbound variable
being let bound when adding a newly created variable alias. (BUG#00002)
* test/src/data-tests.el (data-tests-varalias-watchers-bug00002):
Added regression testing for defvaralias. (BUG#00002)
---
 src/eval.c             | 33 ++++++++++++++----------
 test/src/data-tests.el | 57 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index f48d7b0682f..1d250b995e9 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -612,19 +612,26 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
   /* https://lists.gnu.org/r/emacs-devel/2008-04/msg00834.html
      If n_a is bound, but b_v is not, set the value of b_v to n_a,
      so that old-code that affects n_a before the aliasing is setup
-     still works.  */
-  if (NILP (Fboundp (base_variable)))
-    set_internal (base_variable, find_symbol_value (new_alias),
-                  Qnil, SET_INTERNAL_BIND);
-  else if (!NILP (Fboundp (new_alias))
-           && !EQ (find_symbol_value (new_alias),
-                   find_symbol_value (base_variable)))
-    call2 (intern ("display-warning"),
-           list3 (Qdefvaralias, intern ("losing-value"), new_alias),
-           CALLN (Fformat_message,
-                  build_string
-                  ("Overwriting value of `%s' by aliasing to `%s'"),
-                  new_alias, base_variable));
+     still works.
+     n_a and b_v may be alias chains. Fboundp, find_symbol_value, and
+     set_internal follow redirection.
+     In the instance of variable obsolescence:
+     new_alias = old variable being defined as obsolete
+     base_variable = the new variable replacing it  */
+  if (!NILP (Fboundp (new_alias))) /* BUG#00002 */
+    {
+      if (NILP (Fboundp (base_variable)))
+	set_internal (base_variable, find_symbol_value (new_alias),
+		      Qnil, SET_INTERNAL_SET);
+      else if (!EQ (find_symbol_value (new_alias),
+		    find_symbol_value (base_variable)))
+	call2 (intern ("display-warning"),
+	       list3 (Qdefvaralias, intern ("losing-value"), new_alias),
+	       CALLN (Fformat_message,
+		      build_string
+		      ("Overwriting value of `%s' by aliasing to `%s'"),
+		      new_alias, base_variable));
+    }
 
   {
     union specbinding *p;
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 1d34dd3c4a9..7b31730c4c2 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -643,6 +643,63 @@ data-tests-watchers-forward-bug00001
     (should (null  r1))
     (remove-variable-watcher 'right-margin-width f1)))
 
+;; BUG#00002
+(ert-deftest data-tests-varalias-watchers-bug00002 ()
+  (let ((r1 nil))
+    (cl-flet ((bug-advice (&rest _) (push "we were warned" r1)))
+      (advice-add  #'display-warning :override #'bug-advice)
+      (unwind-protect
+          (let ((a1 (gensym))  ;; unbound cased
+                (a2 (gensym))  ;; unbound cased
+                (a3 (gensym))
+                (a4 (gensym))
+                (a5 (gensym))
+                (v1 (gensym))  ;; unbound cased
+                (v2 (gensym))
+                (v3 (gensym))  ;; unbound cased
+                (v4 (gensym))
+                (v5 (gensym))
+                (f1 (lambda (&rest args) (push args r1))))
+            (set a3 5)
+            (set a4 5)
+            (set a5 10)
+            (set v2 5)
+            (set v4 5)
+            (set v5 5)
+            (add-variable-watcher v1 f1)
+            (add-variable-watcher v2 f1)
+            (add-variable-watcher v3 f1)
+            (add-variable-watcher v4 f1)
+            (add-variable-watcher v5 f1)
+            (add-variable-watcher a5 f1) ;; becoming an alias
+            ;; 1) n_a unbound and b_v unbound
+            (defvaralias a1 v1)
+            (should (null r1))
+            (should-not (boundp v1))
+            (should-not (boundp a1))
+            ;; 2) n_a unbound and b_v bound
+            (defvaralias a2 v2)
+            (should (null r1))
+            (should `(eval (= 5 ,v2 ,a2)))
+            ;; 3) n_a bound and b_v unbound
+            (defvaralias a3 v3)
+            ;; -- b_v watcher being notified it is being set
+            (should (equal (pop r1) `(,v3 5 set nil)))
+            (should (null r1))
+            (should `(eval (= 5 ,v3 ,a3)))
+            ;; 4) both bound and equal
+            (defvaralias a4 v4)
+            (should (null r1))
+            (should `(eval (= 5 ,v4 ,a4)))
+            ;; 5) both bound and unequal (n_a has a watcher also)
+            (defvaralias a5 v5)
+            ;; -- n_a watcher being notified it is becoming an alias
+            (should (equal (pop r1) `(,a5 ,v5 defvaralias nil)))
+            ;; -- user warned n_a is loosing its value
+            (should (equal (pop r1) "we were warned"))
+            (should `(eval (= 5 ,v5 ,a5))))
+        (advice-remove #'display-warning #'bug-advice)))))
+
 (ert-deftest data-tests-kill-all-local-variables () ;bug#30846
   (with-temp-buffer
     (setq-local data-tests-foo1 1)
-- 
2.34.1

