GNU bug report logs - #62419
28.2; Elisp let-bound buffer-local variable and kill-local-variable

Previous Next

Package: emacs;

Reported by: Matthew Malcomson <hardenedapple <at> gmail.com>

Date: Fri, 24 Mar 2023 13:42:02 UTC

Severity: normal

Found in version 28.2

Done: Stefan Kangas <stefankangas <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62419 in the body.
You can then email your comments to 62419 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Fri, 24 Mar 2023 13:42:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Matthew Malcomson <hardenedapple <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 24 Mar 2023 13:42:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matthew Malcomson <hardenedapple <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.2; Elisp let-bound buffer-local variable and kill-local-variable
Date: Fri, 24 Mar 2023 13:37:57 +0000
[Message part 1 (text/plain, inline)]
Hello,

I’m inlining some elisp which has behaviour I find unintuitive.
To view the bug I would run each top-level form with C-x C-e in turn in an elisp buffer.
This behaviour may be expected — the manual mentions something related — but I believe this is an unintended edge-case.
N.b. the use of `auto-fill-function’ is just for a variable which turns buffer-local when set, not as anything related to this particular symbol.
FWIW I believe this behaviour to be the root cause of https://github.com/joaotavora/yasnippet/issues/919 (which was closed due to not being able to reproduce it).

—————
;; Ensure that `auto-fill-function' has a buffer-local version and a global
;; version.
(setq auto-fill-function 'local-symbol)
(describe-variable 'auto-fill-function)
;; `auto-fill-function' is let-bound in the buffer scope
(let ((auto-fill-function 'temp-symbol))
  ;; Now there is no buffer-local variable for `auto-fill-function', but the
  ;; `let' unwrapping info is still there.
  (kill-local-variable 'auto-fill-function)
  ;; Since the check in the emacs source is
  ;; a) Is there a buffer-local variable.
  ;; b) Is there a let-binding shadowing the current variable.
  ;; Then this `setq' sets the *global* variable.
  (setq auto-fill-function 'other-symbol))
;; Exiting the `let' has special handling to avoid resetting a local variable
;; when the local variable was `let' bound, which means that overall the `setq'
;; set the global variable and the `let' has been lost.
(describe-variable 'auto-fill-function)
—————


I think the final state after having done the above should be:
1) Global value has not changed.
2) Local value has not changed.

While the observed state after the above is:
1) Global value has been set to `other-symbol'.
2) Local value has been removed.

- The `setq` inside the `let` sets the *global* value rather than creating a buffer-local variable.
- The `let` on the buffer-local version of the variable is lost.

The manual for `make-variable-buffer-local` — in `(elisp) Creating Buffer-Local` — does mention that if a variable is in a `let` binding then a `setq` does not create a buffer-local version.
That said, I’m guessing the intention of this behaviour is so a `let` binding on a global variable is modified rather than bypassed by a `setq`.
I’d suggest that is not relevant to the above code since the use of `kill-local-variable` means the `let` is effectively lost already (e.g. it does not get “reset” on an unwind).

I’m attaching the source code hack that I’ve started using to work around this.
I’m not proposing this as a change, just including it for extra context about the cause.
I *believe* that the form of the C code around here looks like the special case of a forwarded variable not having a buffer-local value but having a buffer-local `let` binding could easily have been an oversight rather than intention.
(N.b. for this hack I guessed the meanings of the `let` enum values).

[emacs-patch.diff (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sat, 25 Mar 2023 11:55:02 GMT) Full text and rfc822 format available.

Message #8 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matthew Malcomson <hardenedapple <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2;
 Elisp let-bound buffer-local variable and kill-local-variable
Date: Sat, 25 Mar 2023 14:49:54 +0300
> From: Matthew Malcomson <hardenedapple <at> gmail.com>
> Date: Fri, 24 Mar 2023 13:37:57 +0000
> 
> I’m inlining some elisp which has behaviour I find unintuitive.
> To view the bug I would run each top-level form with C-x C-e in turn in an elisp buffer.
> This behaviour may be expected — the manual mentions something related — but I believe this is an unintended edge-case.
> N.b. the use of `auto-fill-function’ is just for a variable which turns buffer-local when set, not as anything related to this particular symbol.
> FWIW I believe this behaviour to be the root cause of https://github.com/joaotavora/yasnippet/issues/919 (which was closed due to not being able to reproduce it).
> 
> —————
> ;; Ensure that `auto-fill-function' has a buffer-local version and a global
> ;; version.
> (setq auto-fill-function 'local-symbol)
> (describe-variable 'auto-fill-function)
> ;; `auto-fill-function' is let-bound in the buffer scope
> (let ((auto-fill-function 'temp-symbol))
>   ;; Now there is no buffer-local variable for `auto-fill-function', but the
>   ;; `let' unwrapping info is still there.
>   (kill-local-variable 'auto-fill-function)
>   ;; Since the check in the emacs source is
>   ;; a) Is there a buffer-local variable.
>   ;; b) Is there a let-binding shadowing the current variable.
>   ;; Then this `setq' sets the *global* variable.
>   (setq auto-fill-function 'other-symbol))
> ;; Exiting the `let' has special handling to avoid resetting a local variable
> ;; when the local variable was `let' bound, which means that overall the `setq'
> ;; set the global variable and the `let' has been lost.
> (describe-variable 'auto-fill-function)
> —————
> 
> 
> I think the final state after having done the above should be:
> 1) Global value has not changed.
> 2) Local value has not changed.
> 
> While the observed state after the above is:
> 1) Global value has been set to `other-symbol'.
> 2) Local value has been removed.
> 
> - The `setq` inside the `let` sets the *global* value rather than creating a buffer-local variable.
> - The `let` on the buffer-local version of the variable is lost.

What is the purpose of doing this? what are you trying to accomplish,
and why?

> The manual for `make-variable-buffer-local` — in `(elisp) Creating Buffer-Local` — does mention that if a variable is in a `let` binding then a `setq` does not create a buffer-local version.
> That said, I’m guessing the intention of this behaviour is so a `let` binding on a global variable is modified rather than bypassed by a `setq`.
> I’d suggest that is not relevant to the above code since the use of `kill-local-variable` means the `let` is effectively lost already (e.g. it does not get “reset” on an unwind).

Did you also read about default-toplevel-value and
set-default-toplevel-value (in the "Default Value" node of the ELisp
manual)?

> I’m not proposing this as a change, just including it for extra context about the cause.
> I *believe* that the form of the C code around here looks like the special case of a forwarded variable not having a buffer-local value but having a buffer-local `let` binding could easily have been an oversight rather than intention.

Stefan, any comments?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sat, 25 Mar 2023 16:21:02 GMT) Full text and rfc822 format available.

Message #11 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Matthew Malcomson <hardenedapple <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62419 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sat, 25 Mar 2023 16:19:57 +0000
> On 25 Mar 2023, at 11:49, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Matthew Malcomson <hardenedapple <at> gmail.com>
>> Date: Fri, 24 Mar 2023 13:37:57 +0000
>> 
>> 
>> 
>> I think the final state after having done the above should be:
>> 1) Global value has not changed.
>> 2) Local value has not changed.
>> 
>> While the observed state after the above is:
>> 1) Global value has been set to `other-symbol'.
>> 2) Local value has been removed.
>> 
>> - The `setq` inside the `let` sets the *global* value rather than creating a buffer-local variable.
>> - The `let` on the buffer-local version of the variable is lost.
> 
> What is the purpose of doing this? what are you trying to accomplish,
> and why?
> 

I came across this when looking into that yasnippet bug I linked in my previous email.
This sequence is not performed on purpose.
The let binding and kill-local-variable happen outside that plugins control, and are unexpected.

The specific context is:
When yasnippet minor mode is turned on it records the “original” `auto-fill-function` and replaces it with its own wrapper using `setq`.
It uses the “original” to `funcall` from the wrapper and to resetting when yasnippet minor mode turned off.
Yasnippet sets `auto-fill-function` to its wrapper using ’setq’ to ensure the value is buffer-local.

AIUI the minor mode is usually turned on outside of any let binding, but the described situation happens due to an edge-case:
- `newline` let-binds `auto-fill-mode`
- Visited file has changed outside of emacs, so user is queried whether to `revert-buffer` while in `newline`
- `revert-buffer` calls `normal-mode`, which runs `kill-all-local-variables`
- Later, `yasnippet-mode` is turned on and the `setq` is evaluated.

N.b. The change I’m suggesting only means that whatever was outside the `let` binding is kept.
That implies that there would still likely be some problem edge-case in the yasnippet code.
I’m just raising the bug on the premise that the existing behaviour seems likely to cause harder to debug problems that my suggestion.
I.e. once at the top-level, seeing a `setq` of a buffer-local variable has changed the top-level global value is very surprising to me.
But not seeing the effect of a `setq` because it was performed inside some unexpected `let` binding is less surprising.
So I believe that debugging unexpected behaviour due to the second would be easier than behaviour due to the first.


>> The manual for `make-variable-buffer-local` — in `(elisp) Creating Buffer-Local` — does mention that if a variable is in a `let` binding then a `setq` does not create a buffer-local version.
>> That said, I’m guessing the intention of this behaviour is so a `let` binding on a global variable is modified rather than bypassed by a `setq`.
>> I’d suggest that is not relevant to the above code since the use of `kill-local-variable` means the `let` is effectively lost already (e.g. it does not get “reset” on an unwind).
> 
> Did you also read about default-toplevel-value and
> set-default-toplevel-value (in the "Default Value" node of the ELisp
> manual)?

Honestly I haven’t looked hard into how to fix the bug I was hitting in elisp.
A solution seems like it would have to answer the more general question of what yasnippet should do if it’s minor-mode is turned on while in a `let` binding of `auto-fill-mode`, and I’m not a yasnippet developer.
I'm reporting this on the premise that the elisp behaviour seemed strange enough to me to ask whether it should be different.

> 
>> I’m not proposing this as a change, just including it for extra context about the cause.
>> I *believe* that the form of the C code around here looks like the special case of a forwarded variable not having a buffer-local value but having a buffer-local `let` binding could easily have been an oversight rather than intention.
> 
> Stefan, any comments?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 26 Mar 2023 14:02:02 GMT) Full text and rfc822 format available.

Message #14 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 26 Mar 2023 10:01:48 -0400
> (setq auto-fill-function 'local-symbol)
> (describe-variable 'auto-fill-function)
> ;; `auto-fill-function' is let-bound in the buffer scope
> (let ((auto-fill-function 'temp-symbol))
>   ;; Now there is no buffer-local variable for `auto-fill-function', but the
>   ;; `let' unwrapping info is still there.
>   (kill-local-variable 'auto-fill-function)
>   ;; Since the check in the emacs source is
>   ;; a) Is there a buffer-local variable.
>   ;; b) Is there a let-binding shadowing the current variable.
>   ;; Then this `setq' sets the *global* variable.
>   (setq auto-fill-function 'other-symbol))
> ;; Exiting the `let' has special handling to avoid resetting a local variable
> ;; when the local variable was `let' bound, which means that overall the `setq'
> ;; set the global variable and the `let' has been lost.

AFAIK the behavior is "as intended": the `let` only affects *one*
binding, either the global one or the buffer-local one.

> AIUI the minor mode is usually turned on outside of any let binding, but the
> described situation happens due to an edge-case:
> - `newline` let-binds `auto-fill-mode`
> - Visited file has changed outside of Emacs, so user is queried whether to
> `revert-buffer` while in `newline`
> - `revert-buffer` calls `normal-mode`, which runs `kill-all-local-variables`
> - Later, `yasnippet-mode` is turned on and the `setq` is evaluated.

The "`newline` let-binds `auto-fill-mode`" seems like the source of the
problem :-(

As for yasnippet.el, I suspect that I patch like the one below would
avoid the problem.


        Stefan


diff --git a/yasnippet.el b/yasnippet.el
index 78ef38ac39..98124c7a61 100644
--- a/yasnippet.el
+++ b/yasnippet.el
@@ -860,10 +828,9 @@ which decides on the snippet to expand.")
   "Hook run when `yas-minor-mode' is turned on.")
 
 (defun yas--auto-fill-wrapper ()
-  (when (and auto-fill-function
-             (not (eq auto-fill-function #'yas--auto-fill)))
-    (setq yas--original-auto-fill-function auto-fill-function)
-    (setq auto-fill-function #'yas--auto-fill)))
+  (when auto-fill-function ;Turning the mode ON.
+    (cl-assert (local-variable-p 'auto-fill-function))
+    (add-function :around (local 'auto-fill-function) #'yas--auto-fill)))
 
 ;;;###autoload
 (define-minor-mode yas-minor-mode
@@ -906,8 +873,8 @@ Key bindings:
          ;; auto-fill handler.
          (remove-hook 'post-command-hook #'yas--post-command-handler t)
          (remove-hook 'auto-fill-mode-hook #'yas--auto-fill-wrapper)
-         (when (local-variable-p 'yas--original-auto-fill-function)
-           (setq auto-fill-function yas--original-auto-fill-function))
+         (when (local-variable-p 'auto-fill-function)
+           (remove-function (local 'auto-fill-function) #'yas--auto-fill))
          (setq emulation-mode-map-alists
                (remove 'yas--direct-keymaps emulation-mode-map-alists)))))
 
@@ -3880,7 +3847,7 @@ field start.  This hook does nothing if an undo is in progress."
                    snippet (yas--snippet-field-mirrors snippet)))
       (setq yas--todo-snippet-indent nil))))
 
-(defun yas--auto-fill ()
+(defun yas--auto-fill (orig-fun &rest args)
   ;; Preserve snippet markers during auto-fill.
   (let* ((orig-point (point))
          (end (progn (forward-paragraph) (point)))
@@ -3897,44 +3864,7 @@ field start.  This hook does nothing if an undo is in progress."
             reoverlays))
     (goto-char orig-point)
     (let ((yas--inhibit-overlay-hooks t))
-      (if yas--original-auto-fill-function
-          (funcall yas--original-auto-fill-function)
-        ;; Shouldn't happen, gather more info about it (see #873/919).
-        (let ((yas--fill-fun-values `((t ,(default-value 'yas--original-auto-fill-function))))
-              (fill-fun-values `((t ,(default-value 'auto-fill-function))))
-              ;; Listing 2 buffers with the same value is enough
-              (print-length 3))
-          (save-current-buffer
-            (dolist (buf (let ((bufs (buffer-list)))
-                           ;; List the current buffer first.
-                           (setq bufs (cons (current-buffer)
-                                            (remq (current-buffer) bufs)))))
-              (set-buffer buf)
-              (let* ((yf-cell (assq yas--original-auto-fill-function
-                                    yas--fill-fun-values))
-                     (af-cell (assq auto-fill-function fill-fun-values)))
-                (when (local-variable-p 'yas--original-auto-fill-function)
-                  (if yf-cell (setcdr yf-cell (cons buf (cdr yf-cell)))
-                    (push (list yas--original-auto-fill-function buf) yas--fill-fun-values)))
-                (when (local-variable-p 'auto-fill-function)
-                  (if af-cell (setcdr af-cell (cons buf (cdr af-cell)))
-                    (push (list auto-fill-function buf) fill-fun-values))))))
-          (lwarn '(yasnippet auto-fill bug) :error
-                 "`yas--original-auto-fill-function' unexpectedly nil in %S!  Disabling auto-fill.
-  %S
-  `auto-fill-function': %S\n%s"
-                 (current-buffer) yas--fill-fun-values fill-fun-values
-                 (if (fboundp 'backtrace--print-frame)
-                     (with-output-to-string
-                       (mapc (lambda (frame)
-                               (apply #'backtrace--print-frame frame))
-                             yas--watch-auto-fill-backtrace))
-                   ""))
-          ;; Try to avoid repeated triggering of this bug.
-          (auto-fill-mode -1)
-          ;; Don't pop up more than once in a session (still log though).
-          (defvar warning-suppress-types) ; `warnings' is autoloaded by `lwarn'.
-          (add-to-list 'warning-suppress-types '(yasnippet auto-fill bug)))))
+      (apply orig-fun args))
     (save-excursion
       (setq end (progn (forward-paragraph) (point)))
       (setq beg (progn (backward-paragraph) (point))))





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 26 Mar 2023 14:35:02 GMT) Full text and rfc822 format available.

Message #17 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 26 Mar 2023 10:34:05 -0400
> The "`newline` let-binds `auto-fill-mode`" seems like the source of the
> problem :-(

We could fix it with a patch like the one below (intended for `master`).


        Stefan


diff --git a/lisp/simple.el b/lisp/simple.el
index 3e50e888dad..6f0215bfb1d 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -659,7 +659,7 @@ newline
          (beforepos (point))
          (last-command-event ?\n)
          ;; Don't auto-fill if we have a prefix argument.
-         (auto-fill-function (if arg nil auto-fill-function))
+         (inhibit-auto-fill (or inhibit-auto-fill arg))
          (arg (prefix-numeric-value arg))
          (procsym (make-symbol "newline-postproc")) ;(bug#46326)
          (postproc
@@ -9269,11 +9269,15 @@ default-indent-new-line
        ;; If we're not inside a comment, just try to indent.
        (t (indent-according-to-mode))))))
 
+(defvar inhibit-auto-fill nil
+  "Non-nil means to do as if `auto-fill-mode' was disabled.")
+
 (defun internal-auto-fill ()
   "The function called by `self-insert-command' to perform auto-filling."
-  (when (or (not comment-start)
-            (not comment-auto-fill-only-comments)
-            (nth 4 (syntax-ppss)))
+  (when (and (not inhibit-auto-fill)
+             (or (not comment-start)
+                 (not comment-auto-fill-only-comments)
+                 (nth 4 (syntax-ppss))))
     (funcall auto-fill-function)))
 
 (defvar normal-auto-fill-function #'do-auto-fill





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 26 Mar 2023 15:02:02 GMT) Full text and rfc822 format available.

Message #20 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Matthew Malcomson <hardenedapple <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 26 Mar 2023 16:01:22 +0100

> On 26 Mar 2023, at 15:01, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> 
>> (setq auto-fill-function 'local-symbol)
>> (describe-variable 'auto-fill-function)
>> ;; `auto-fill-function' is let-bound in the buffer scope
>> (let ((auto-fill-function 'temp-symbol))
>>  ;; Now there is no buffer-local variable for `auto-fill-function', but the
>>  ;; `let' unwrapping info is still there.
>>  (kill-local-variable 'auto-fill-function)
>>  ;; Since the check in the emacs source is
>>  ;; a) Is there a buffer-local variable.
>>  ;; b) Is there a let-binding shadowing the current variable.
>>  ;; Then this `setq' sets the *global* variable.
>>  (setq auto-fill-function 'other-symbol))
>> ;; Exiting the `let' has special handling to avoid resetting a local variable
>> ;; when the local variable was `let' bound, which means that overall the `setq'
>> ;; set the global variable and the `let' has been lost.
> 
> AFAIK the behavior is "as intended": the `let` only affects *one*
> binding, either the global one or the buffer-local one.
> 

Not going to push much on this since your suggested change to `newline` would fix everything to me.
But the part I think is strange is `setq` not creating a buffer-local binding in this environment.
(i.e. not to do with what `let` is affecting).

The “special behaviour” that a `setq` may act on a global binding of an automatic buffer-local variable when inside a let binding seems to me like the intention is to avoid “bypassing” a let on a global binding.

I.e. currently the behaviour of `setq` on automatic	buffer-local variables is:
	- Outside `let`, always affect buffer-local (creating if necessary)
	- In `let` of global binding, affect global binding.
	- In `let` of buffer-local binding, affect buffer-local
	- In `let` of buffer-local binding but where buffer-local value has been killed, affect global value.

I believe that last condition is strange and the behaviour of `setq` would be more understandable without it.
Especially since when viewed from the top-level (i.e. evaluate some lisp which contains a `setq` of an automatic buffer-local variable and may or may not have a `let`) the options are:
	- If `setq` is outside any `let`, it will affect a buffer-local binding.
	- If `setq` is inside a `let` then it won’t be visible once exited the `let` (i.e. it affects the binding that the `let` acted on)
	- *Unless* the `let` was of a buffer-local binding and that buffer-local binding was killed, in which case the `setq` will affect the global binding.

Either way, thanks for looking into this!
Matthew





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 26 Mar 2023 15:17:02 GMT) Full text and rfc822 format available.

Message #23 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 26 Mar 2023 11:16:44 -0400
>>> (setq auto-fill-function 'local-symbol)
>>> (describe-variable 'auto-fill-function)
>>> ;; `auto-fill-function' is let-bound in the buffer scope
>>> (let ((auto-fill-function 'temp-symbol))
>>>  ;; Now there is no buffer-local variable for `auto-fill-function', but the
>>>  ;; `let' unwrapping info is still there.
>>>  (kill-local-variable 'auto-fill-function)
>>>  ;; Since the check in the emacs source is
>>>  ;; a) Is there a buffer-local variable.
>>>  ;; b) Is there a let-binding shadowing the current variable.
>>>  ;; Then this `setq' sets the *global* variable.
>>>  (setq auto-fill-function 'other-symbol))
>>> ;; Exiting the `let' has special handling to avoid resetting a local variable
>>> ;; when the local variable was `let' bound, which means that overall the `setq'
>>> ;; set the global variable and the `let' has been lost.
>> 
>> AFAIK the behavior is "as intended": the `let` only affects *one*
>> binding, either the global one or the buffer-local one.
>> 
>
> Not going to push much on this since your suggested change to
> `newline` would fix everything to me.  But the part I think is strange
> is `setq` not creating a buffer-local binding in this environment.

Hmm... maybe you're right that the (setq auto-fill-function 'other-symbol)
shouldn't set the global variable but the local one.
It might be a bug in how we check whether there's a let-binding that
should make us refrain from obeying the "automatically set buffer-locally".

Good point.  I'll have to take a closer look.

> I.e. currently the behaviour of `setq` on automatic	buffer-local variables is:
> 	- Outside `let`, always affect buffer-local (creating if necessary)
> 	- In `let` of global binding, affect global binding.
> 	- In `let` of buffer-local binding, affect buffer-local
> 	- In `let` of buffer-local binding but where buffer-local value has
> been killed, affect global value.
>
> I believe that last condition is strange and the behaviour of `setq` would
> be more understandable without it.

Agreed.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 26 Mar 2023 15:31:02 GMT) Full text and rfc822 format available.

Message #26 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 26 Mar 2023 11:30:43 -0400
> I.e. currently the behaviour of `setq` on automatic	buffer-local variables is:
> 	- Outside `let`, always affect buffer-local (creating if necessary)
> 	- In `let` of global binding, affect global binding.
> 	- In `let` of buffer-local binding, affect buffer-local
> 	- In `let` of buffer-local binding but where buffer-local value has
> been killed, affect global value.
>
> I believe that last condition is strange and the behaviour of `setq` would
> be more understandable without it.

I think the patch below would fix this corner case.


        Stefan


diff --git a/src/eval.c b/src/eval.c
index ef7ca33f834..82ada40b309 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3364,7 +3364,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
@@ -3379,6 +3379,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;
       }





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Wed, 29 Mar 2023 10:57:02 GMT) Full text and rfc822 format available.

Message #29 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Matthew Malcomson <hardenedapple <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Wed, 29 Mar 2023 11:56:32 +0100
Thanks!

Just for the record I’ve been running using essentially this patch (but with !=) since you suggested it and have not had any problems.

FWIW I also think the change to avoid `let` binding `auto-fill-function` in `normal-mode` is good.

Regards,
Matthew

> On 26 Mar 2023, at 16:30, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> 
>> I.e. currently the behaviour of `setq` on automatic	buffer-local variables is:
>> 	- Outside `let`, always affect buffer-local (creating if necessary)
>> 	- In `let` of global binding, affect global binding.
>> 	- In `let` of buffer-local binding, affect buffer-local
>> 	- In `let` of buffer-local binding but where buffer-local value has
>> been killed, affect global value.
>> 
>> I believe that last condition is strange and the behaviour of `setq` would
>> be more understandable without it.
> 
> I think the patch below would fix this corner case.
> 
> 
>        Stefan
> 
> 
> diff --git a/src/eval.c b/src/eval.c
> index ef7ca33f834..82ada40b309 100644
> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -3364,7 +3364,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
> @@ -3379,6 +3379,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;
>       }
> 





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62419; Package emacs. (Sun, 02 Apr 2023 21:56:02 GMT) Full text and rfc822 format available.

Message #32 received at 62419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 62419 <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2; Elisp let-bound buffer-local variable and
 kill-local-variable
Date: Sun, 02 Apr 2023 17:55:36 -0400
> Just for the record I’ve been running using essentially this patch (but
> with !=) since you suggested it and have not had any problems.

Thanks for confirming it fixes the problem for you.
I pushed it to `master` (doesn't seem "obviously safe" enough for `emacs-29`).

> FWIW I also think the change to avoid `let` binding
> `auto-fill-function` in `normal-mode` is good.

I also pushed it to `master`.


        Stefan





Reply sent to Stefan Kangas <stefankangas <at> gmail.com>:
You have taken responsibility. (Tue, 12 Sep 2023 00:01:04 GMT) Full text and rfc822 format available.

Notification sent to Matthew Malcomson <hardenedapple <at> gmail.com>:
bug acknowledged by developer. (Tue, 12 Sep 2023 00:01:04 GMT) Full text and rfc822 format available.

Message #37 received at 62419-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 62419-done <at> debbugs.gnu.org
Subject: Re: bug#62419: 28.2;
 Elisp let-bound buffer-local variable and kill-local-variable
Date: Mon, 11 Sep 2023 17:00:48 -0700
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Just for the record I’ve been running using essentially this patch (but
>> with !=) since you suggested it and have not had any problems.
>
> Thanks for confirming it fixes the problem for you.
> I pushed it to `master` (doesn't seem "obviously safe" enough for `emacs-29`).
>
>> FWIW I also think the change to avoid `let` binding
>> `auto-fill-function` in `normal-mode` is good.
>
> I also pushed it to `master`.

It seems like this issue was fixed, but it was left open in the bug
tracker.  I'm therefore closing it now.

Please remember to close bug reports when they are fixed.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 10 Oct 2023 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 170 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.