[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexica
From: |
Stefan Monnier |
Subject: |
bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding |
Date: |
Sat, 01 Feb 2020 15:15:08 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
>> If that's the case, then why don't we say that loud and clear in the ELisp
>> manual?
>> (The doc string of add-to-list has some vague
>> recommendation, not sure if it really talks about this aspect, but
>> that's definitely not enough for such a serious issue.)
Good idea.
>> And doesn't it mean we should audit all the gazillion uses of
>> add-to-list in our sources, and do that urgently?
In dynamically-scoped files, it's OK. But yes, we should audit them and
change those that need changing for lexical scoping.
I added a compiler macro as a crutch to handle the most command
problems, but it's just an ugly hack which can make things worse by
hiding the problem.
> The compiler-macro (which apparently works in non-compiled code as well)
Compiler macros work when the code passes through `macroexpand-all`, so
it works when the code is compiled as well as when it's `load`ed (thanks
to "eager" macroexpansion), but not when it's passed directly to `eval`.
> attempts to warn about lexical variables but somehow this warning
> doesn't always trigger. I haven't researched this further.
The message is supposed not to trigger when it's applied to dynamically
scoped var, but it's probably not 100% reliable.
> It all seems rather fragile to me, and I'd rather add a note about not using
> add-to-list for lexical variables to its doc string and the manual, and fix
> all the calls listed above.
Yes, this compiler-macro isn't supposed to replace educating the
programmers (in the manual) and fixing the actual problems.
BTW, this problem doesn't affect only `add-to-list`. Other culprits
include `add-hook`, `run-hooks`, `set`, and `symbol-value`.
> Two patches attached: a doc update, and a replacement of add-to-list in the
> cases listed above.
LGTM,
Stefan
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Eli Zaretskii, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Drew Adams, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Mattias Engdegård, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Eli Zaretskii, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding,
Stefan Monnier <=
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Mattias Engdegård, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Eli Zaretskii, 2020/02/01
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Mattias Engdegård, 2020/02/02
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Stefan Monnier, 2020/02/02
- bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding, Mattias Engdegård, 2020/02/02