bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1657 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 2. Juli 2017 um
18:53 Uhr:

> Michael Heerdegen <michael_heerdegen@web.de> schrieb am So., 18. Juni
> 2017 um 06:17 Uhr:
>
>> Philipp Stephani <p.stephani2@gmail.com> writes:
>>
>> > It's possible to fix this (see attached patch), but at the expense of
>> > breaking other valid use cases such as (cl-incf (buffer-local-value
>> > ...)). Not sure whether the bug can be fixed at all without breaking
>> > other stuff.
>>
>> I have no solution, but some thoughts.
>>
>> The more I think about it, the more I come to the conclusion that
>> `buffer-local-value' does not have a well defined according place.
>>
>> The function `buffer-local-value' is not injective: it maps different
>> states to the same value because it can't express whether the VARIABLE's
>> binding is buffer-local or not.  But we need this information because we
>> need to undo creating a buffer local binding in the setter when closing
>> the `letf'.
>>
>> And the setter, accepting only a value for the binding, isn't
>> surjective, because the argument doesn't hold any information of
>> buffer-localness.  Moreover, we want the setter to always create a
>> buffer-local binding in one situation (setf), but this isn't true for
>> the setter we need to use for `cl-letf'.
>>
>> We could widen the semantics of `cl-letf' to do what we want in this
>> case, but I'm not sure if it's worth the trouble.  Not if there are more
>> cases like this.
>>
>>
> Thanks for this great analysis. Given this, it seems that the place
> definition for `buffer-local-value' should be removed from gv.el.
>

Here's a patch that does just that.

[-- Attachment #1.2: Type: text/html, Size: 2456 bytes --]

[-- Attachment #2: 0001-Remove-buffer-local-value-as-generalized-variable-Bug-.txt --]
[-- Type: text/plain, Size: 2480 bytes --]

From 4ad3058476bdc7f5fa7e60d91c240a63e1f5f73d Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Fri, 16 Jun 2017 22:55:52 +0200
Subject: [PATCH] Remove `buffer-local-value' as generalized variable
 (Bug#26624)

* lisp/emacs-lisp/gv.el (buffer-local-value): Remove.
* lisp/files.el (file-name-non-special): Remove a stale comment.
---
 etc/NEWS              | 7 +++++++
 lisp/emacs-lisp/gv.el | 4 ----
 lisp/files.el         | 2 --
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index aacdf79b57..5b87939cf7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -51,6 +51,13 @@ sets the XTerm window title.  The default is to set the window title.
 ** The FILENAME argument to 'file-name-base' is now mandatory and no
 longer defaults to 'buffer-file-name'.
 
+** 'buffer-local-value' is no longer a generalized variable.  This is
+because the buffer-local value of a variable always consists of two
+pieces: the actual value of the variable, and whether it is
+buffer-local.  But 'buffer-local-value' returns only one piece of
+information, which is not enough for a generalized variable.  In
+particular, 'cl-letf' can't be made to work with 'buffer-local-value'.
+
 \f
 * Lisp Changes in Emacs 27.1
 
diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 892d6e9716..f504ca43b0 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -367,10 +367,6 @@ setf
 (gv-define-setter window-point (v &optional w) `(set-window-point ,w ,v))
 (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v))
 
-(gv-define-setter buffer-local-value (val var buf)
-  (macroexp-let2 nil v val
-    `(with-current-buffer ,buf (set (make-local-variable ,var) ,v))))
-
 (gv-define-expander alist-get
   (lambda (do key alist &optional default remove testfn)
     (macroexp-let2 macroexp-copyable-p k key
diff --git a/lisp/files.el b/lisp/files.el
index fe7cb1a8a9..4bb611a3ba 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7005,8 +7005,6 @@ file-name-non-special
            (when (and visit buffer-file-name)
              (setq buffer-file-name (concat "/:" buffer-file-name))))))
       (`unquote-then-quote
-       ;; We can't use `cl-letf' with `(buffer-local-value)' here
-       ;; because it wouldn't work during bootstrapping.
        (let ((buffer (current-buffer)))
          ;; `unquote-then-quote' is only used for the
          ;; `verify-visited-file-modtime' action, which takes a buffer
-- 
2.14.1