GNU bug report logs - #75626
31.0.50; Dired misses or double-processes files when auto-revert-mode is enabled

Previous Next

Full log

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

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
 of text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 75626 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Michael Albinus <michael.albinus <at> gmx.de>,
 Tassilo Horn <tsdh <at> gnu.org>
Subject: Re: bug#75626: 31.0.50; Dired misses or double-processes files when
 auto-revert-mode is enabled
Date: Wed, 05 Feb 2025 16:22:52 +0000

Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs <at> gnu.org> writes:

> Michael Albinus <michael.albinus <at> gmx.de> writes:
>
>> Cut'n'waste error. Thanks for the heads-up, I've fixed it.
>
> Thanks.
>
>
>> > | + (let ((buf (and (not (memq (current-buffer)
>> > | inhibit-auto-revert-buffers))
>> > | +                       (current-buffer))))
>> > | +         (unwind-protect
>> > | +             (progn
>> > | + (when buf (add-to-list 'inhibit-auto-revert-buffers buf))
>> > | +               ,@body)
>> >
>> > I think at least in this part where body is inserted the variable BUF
>> > should be uninterned for the obvious reason.  Or at least have a more
>> > obscure name.
>>
>> Yes, I've seen such constructs somewhere else in Emacs' code. I don't
>> understand why a simple (lexical) let-binding of buf isn't
>> sufficient. The problem would exist for any macro. Could you pls show me
>> an example how it could go wrong?
>
> Have you ever read (info "(elisp) Surprising Local Vars") - and the whole
> (info "(elisp) Problems with Macros") chapter?  (If not, you should do...)
>
> Short version: Lisp macro expansion is dumb ("unhygienic"): if your
> macro uses a variable that accidentally has the same name as a variable
> in the "inserted" code (like a BODY), when expanding the macro these
> unrelated variables will become one and the same variable in the code
> ("variable" or "name clash") causing a variety of funny and unexpected
> surprises that are weird and nasty to debug.
>
> In case of `inhibit-auto-revert', the macro could interfere with a
> variable named `buf' that may occur in BODY.  A variable named `buf' may
> already be introduced in the surrounding code, e.g.
>
> #+begin_src emacs-lisp
> (let ((buf (get-buffer "*scratch*")))
>   (inhibit-auto-revert
>     (setq-local this-buffers-friend buf)
>     (do-this-and-that...)))
> #+end_src
>
>
> The expansion would look like:
>
> #+begin_src emacs-lisp
> (let ((buf (get-buffer "*scratch*")))
>  ...
>  (let ((buf (and (not (memq (current-buffer) inhibit-auto-revert-buffers))
>                  (current-buffer))))
>    (unwind-protect
>        (progn
>          (if buf
>              (add-to-list 'inhibit-auto-revert-buffers buf))
>          (setq this-buffers-friend buf) ;; Ouch!! Refers to the wrong `buf'!
>          (do-this-and-that...))
>      ...)))
> #+end_src
>
>> (If you believe it is needed, you can just patch autorevert.el yourself,
>> with a comment in the code.)
>
> If I didn't misunderstand and this was new to you - am I allowed to
> suggest this as an exercise?
>
> The idea for fixing is that the macro instead introduces a new
> uninterned symbol instead of an interned one, and uses that in the
> expansion (see manual): a new uninterned symbol can't clash with any
> existing variable.  `macroexpand' and `macroexpand-all' are your friend:
> use these to see what you get.  Later you can do that in your mind only.
> If you don't do that _all_the_time_, you'll get crazy with this stuff.

AFAIU you can't do that all the time though, like if you want to
introduce a local var in your macro that may be used in body e.g. `it`
in cl-loop.  Correct me if I am wrong.

-- 
Thierry

Previous Next

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