bug-gnu-emacs
[Top][All Lists]
Advanced

[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






reply via email to

[Prev in Thread] Current Thread [Next in Thread]