bug#62419: 28.2; Elisp let-bound buffer-local variable and kill-local-variable

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

> (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))))