unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: "Mattias Engdegård" <mattiase@acm.org>
Cc: 39373@debbugs.gnu.org
Subject: bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding
Date: Sat, 01 Feb 2020 15:15:08 -0500	[thread overview]
Message-ID: <jwv1rre9imh.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <3FB58E0B-E1DB-4709-98AB-92A45508486A@acm.org> ("Mattias Engdegård"'s message of "Sat, 1 Feb 2020 20:24:56 +0100")

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






  parent reply	other threads:[~2020-02-01 20:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31 17:08 bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding Mattias Engdegård
2020-01-31 19:27 ` Eli Zaretskii
2020-01-31 19:38   ` Mattias Engdegård
2020-01-31 20:25     ` Eli Zaretskii
2020-01-31 20:51       ` Mattias Engdegård
2020-02-01  7:48         ` Eli Zaretskii
2020-02-01 15:53           ` Drew Adams
2020-02-01 19:24           ` Mattias Engdegård
2020-02-01 19:28             ` Eli Zaretskii
2020-02-01 20:15             ` Stefan Monnier [this message]
2020-02-01 21:40               ` Mattias Engdegård
2020-02-02  3:31                 ` Eli Zaretskii
2020-02-02 11:58                   ` Mattias Engdegård
2020-02-02 13:55                     ` Stefan Monnier
2020-02-02 14:14                       ` Mattias Engdegård
2020-08-09 11:39                         ` Lars Ingebrigtsen
2020-08-09 13:28                           ` Mattias Engdegård
2020-08-09 19:42                             ` Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwv1rre9imh.fsf-monnier+emacs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=39373@debbugs.gnu.org \
    --cc=mattiase@acm.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).