master 5223762e02a: src/eval.c: Fix bug#62419

[Stefan Monnier]

branch: master
commit 5223762e02ac84eee984cd1f7a17865766cdad9a
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    src/eval.c: Fix bug#62419
    
    Yup, almost 40 years after ELisp first combined them, buffer-local
    and let bindings still don't work quite right :-(
    
    The "automatically buffer-local if set" semantics should follow the
    principle that it becomes buffer-local iff the var's current binding
    refers to the top-level/global/non-let binding.
    
    * src/eval.c (let_shadows_buffer_binding_p): Disregard non-global
    let-bindings.
    
    * test/src/eval-tests.el (eval-test--bug62419): New test.
---
 src/eval.c             |  3 ++-
 test/src/eval-tests.el | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/eval.c b/src/eval.c
index eb40c953f96..1a4d3ad0307 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3400,7 +3400,7 @@ DEFUN ("fetch-bytecode", Ffetch_bytecode, Sfetch_bytecode,
   return object;
 }
 
-/* Return true if SYMBOL currently has a let-binding
+/* Return true if SYMBOL's default currently has a let-binding
    which was made in the buffer that is now current.  */
 
 bool
@@ -3415,6 +3415,7 @@ let_shadows_buffer_binding_p (struct Lisp_Symbol *symbol)
        struct Lisp_Symbol *let_bound_symbol = XSYMBOL (specpdl_symbol (p));
        eassert (let_bound_symbol->u.s.redirect != SYMBOL_VARALIAS);
        if (symbol == let_bound_symbol
+           && p->kind != SPECPDL_LET_LOCAL /* bug#62419 */
            && EQ (specpdl_where (p), buf))
          return 1;
       }
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index 1e7edca3bac..e0a27439ba2 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -247,4 +247,23 @@ expressions works for identifiers starting with period."
             (should (equal (string-trim (buffer-string))
                            expected-messages))))))))
 
+(defvar-local eval-test--local-var 'global)
+
+(ert-deftest eval-test--bug62419 ()
+  (with-temp-buffer
+    (setq eval-test--local-var 'first-local)
+    (let ((eval-test--local-var t))
+      (kill-local-variable 'eval-test--local-var)
+      (setq eval-test--local-var 'second-local)
+      (should (eq eval-test--local-var 'second-local)))
+    ;; FIXME: It's not completely clear if exiting the above `let'
+    ;; should restore the buffer-local binding to `first-local'
+    ;; (i.e. reset the value of the second buffer-local binding to the
+    ;; first's initial value) or should do nothing (on the principle that
+    ;; the first buffer-local binding doesn't exists any more so there's
+    ;; nothing to restore).  I think both semantics make sense.
+    ;;(should (eq eval-test--local-var 'first-local))
+    )
+  (should (eq eval-test--local-var 'global)))
+
 ;;; eval-tests.el ends here