bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

'mode-local-print-bindings' in mode-local.el seems to be broken since the file was converted to lexical binding.
Straightforward patch attached -- acceptable for emacs-27? (It's a regression from Emacs 26.)


[-- Attachment #2: 0001-Fix-describe-mode-local-bindings-lexical-binding-dam.patch --]
[-- Type: application/octet-stream, Size: 1463 bytes --]

From ded807b01792e966a4ce4905cc3ed2483c52294a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Fri, 31 Jan 2020 17:37:17 +0100
Subject: [PATCH] Fix describe-mode-local-bindings lexical-binding damage

* lisp/cedet/mode-local.el (mode-local-print-bindings):
Repair code that relied on dynamic binding.
---
 lisp/cedet/mode-local.el | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lisp/cedet/mode-local.el b/lisp/cedet/mode-local.el
index a6e143cfcd..70ed62f4fb 100644
--- a/lisp/cedet/mode-local.el
+++ b/lisp/cedet/mode-local.el
@@ -819,14 +819,11 @@ mode-local-print-bindings
         )
     ;; Order symbols by type
     (mapatoms
-     #'(lambda (s)
-         (add-to-list (cond
-                       ((get s 'mode-variable-flag)
-                        (if (get s 'constant-flag) 'mc 'mv))
-                       ((get s 'override-flag)
-                        (if (get s 'constant-flag) 'fo 'ov))
-                       ('us))
-                      s))
+     (lambda (s) (push s (cond ((get s 'mode-variable-flag)
+                                (if (get s 'constant-flag) mc mv))
+                               ((get s 'override-flag)
+                                (if (get s 'constant-flag) fo ov))
+                               (t us))))
      table)
     ;; Print symbols by type
     (when us
-- 
2.21.1 (Apple Git-122.3)

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 31 Jan 2020 18:08:01 +0100
> 
> Straightforward patch attached -- acceptable for emacs-27?

If you find an easier way of fixing it, yes.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

31 jan. 2020 kl. 20.27 skrev Eli Zaretskii <eliz@gnu.org>:

> If you find an easier way of fixing it, yes.

Sorry if I'm misunderstanding you, but the patch just replaces 'add-to-list' with 'push' -- is that not easy enough?

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 31 Jan 2020 20:38:08 +0100
> Cc: 39373@debbugs.gnu.org
> 
> 31 jan. 2020 kl. 20.27 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > If you find an easier way of fixing it, yes.
> 
> Sorry if I'm misunderstanding you, but the patch just replaces 'add-to-list' with 'push' -- is that not easy enough?

That's not the "usual" way of fixing problems with lexical-binding,
and it isn't immediately clear to me why that fixes the problem.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

31 jan. 2020 kl. 21.25 skrev Eli Zaretskii <eliz@gnu.org>:

> That's not the "usual" way of fixing problems with lexical-binding,
> and it isn't immediately clear to me why that fixes the problem.

'add-to-list' doesn't work on lexical variables because it's a plain function taking the variable symbol as argument. 'push' works on lexical variables since it is a macro that expands to direct variable reference and setq. I should perhaps have included this in the commit message.

Happily, Emacs provides generalised variable support for 'if' and 'cons', so that they can be used as target for 'push'.

We could sink the pushes to the leaves of the condition tree, if you prefer:

(cond ((get s 'mode-variable-flag)
       (if (get s 'constant-flag) (push s mc) (push s mv)))
      ((get s 'override-flag)
       (if (get s 'constant-flag) (push s fo) (push s ov)))
      (t (push s us)))

Not as elegant or concise, but does not rely on if and cond as GVs in push.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 31 Jan 2020 21:51:48 +0100
> Cc: 39373@debbugs.gnu.org
> 
> 'add-to-list' doesn't work on lexical variables because it's a plain
> function taking the variable symbol as argument. 'push' works on
> lexical variables since it is a macro that expands to direct
> variable reference and setq.

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

And doesn't it mean we should audit all the gazillion uses of
add-to-list in our sources, and do that urgently?

> I should perhaps have included this in the commit message.

That would have definitely helped, thanks.

> We could sink the pushes to the leaves of the condition tree, if you prefer:

No, your original patch is fine, but please let's update the docs
ASAP.

Thanks.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Drew Adams]

> > 'add-to-list' doesn't work on lexical variables because it's a plain
> > function taking the variable symbol as argument. 'push' works on
> > lexical variables since it is a macro that expands to direct
> > variable reference and setq.
> 
> 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.)
> 
> And doesn't it mean we should audit all the gazillion uses of
> add-to-list in our sources, and do that urgently?

+1.  `add-to-list' uses `symbol-value'.  Everything that
uses `symbol-value' is in the same boat.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]

1 feb. 2020 kl. 08.48 skrev Eli Zaretskii <eliz@gnu.org>:

> 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.)
> 
> And doesn't it mean we should audit all the gazillion uses of
> add-to-list in our sources, and do that urgently?

Yes, I agree, but it's not quite as simple as I thought at first. A mechanised scan of the source for uses of add-to-list on lexical variables revealed a handful:

lisp/cedet/mode-local.el:823:23: add-to-list to lexical variable mc
lisp/net/tramp-cache.el:376:45: add-to-list to lexical variable properties
lisp/net/tramp-cache.el:430:25: add-to-list to lexical variable result
lisp/net/zeroconf.el:259:40: add-to-list to lexical variable result
lisp/net/zeroconf.el:267:40: add-to-list to lexical variable result
lisp/net/zeroconf.el:281:23: add-to-list to lexical variable result
lisp/org/org.el:18685:49: add-to-list to lexical variable load-uncore
lisp/autoinsert.el:174:47: add-to-list to lexical variable modes
lisp/whitespace.el:1687:33: add-to-list to lexical variable style
test/lisp/emacs-lisp/map-tests.el:230:30: add-to-list to lexical variable result

However, add-to-list has a compiler macro which tries to make it work even for lexical variables, and it mostly does -- specifically, when the LIST-VAR parameter is on the form (quote VARIABLE), which is the case in all the above cases except the one i mode-local.el.

The macro (which apparently works in non-compiled code as well) attempts to warn about lexical variables but somehow this warning doesn't always trigger. I haven't researched this further.

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. However, this last point is not strictly necessary for correctness.

Two patches attached: a doc update, and a replacement of add-to-list in the cases listed above.


[-- Attachment #2: 0001-Clarify-add-to-list-documentation-bug-39373.patch --]
[-- Type: application/octet-stream, Size: 2039 bytes --]

From e7535d23b0855307a93ea6e8fa7cf7fb5910ac56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 1 Feb 2020 20:11:11 +0100
Subject: [PATCH 1/2] Clarify add-to-list documentation (bug#39373)

While add-to-list often works with lexical variables, this is a hack
that isn't always effective; better tell the user not to try.

* doc/lispref/lists.texi (List Variables): Add a note about lexical
variables to the add-to-list description.  Fix the equivalent code.
* lisp/subr.el (add-to-list): Amend doc string.
---
 doc/lispref/lists.texi | 8 ++++++--
 lisp/subr.el           | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/lispref/lists.texi b/doc/lispref/lists.texi
index 5ef21d0671..ce0d9a3c92 100644
--- a/doc/lispref/lists.texi
+++ b/doc/lispref/lists.texi
@@ -777,6 +777,9 @@ List Variables
 The argument @var{symbol} is not implicitly quoted; @code{add-to-list}
 is an ordinary function, like @code{set} and unlike @code{setq}.  Quote
 the argument yourself if that is what you want.
+
+Do not use this function when @var{symbol} refers to a lexical
+variable.
 @end defun
 
 Here's a scenario showing how to use @code{add-to-list}:
@@ -799,8 +802,9 @@ List Variables
 @var{value})} is this:
 
 @example
-(or (member @var{value} @var{var})
-    (setq @var{var} (cons @var{value} @var{var})))
+(if (member @var{value} @var{var})
+    @var{var}
+  (setq @var{var} (cons @var{value} @var{var})))
 @end example
 
 @defun add-to-ordered-list symbol element &optional order
diff --git a/lisp/subr.el b/lisp/subr.el
index a4fdc6bdfe..05fb82321e 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1845,6 +1845,7 @@ add-to-list
 If ELEMENT is added, it is added at the beginning of the list,
 unless the optional argument APPEND is non-nil, in which case
 ELEMENT is added at the end.
+LIST-VAR should not refer to a lexical variable.
 
 The return value is the new value of LIST-VAR.
 
-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #3: 0002-Replace-add-to-list-to-lexical-variable-with-push-bu.patch --]
[-- Type: application/octet-stream, Size: 7104 bytes --]

From 4501670056beed4c8e709674b3b80dc2a1160c0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 1 Feb 2020 18:07:32 +0100
Subject: [PATCH 2/2] Replace add-to-list to lexical variable with push
 (bug#39373)

Since 'add-to-list', being a plain function, cannot access lexical
variables, such use must be rewritten for correctness.

* lisp/autoinsert.el (auto-insert-alist):
* lisp/cedet/mode-local.el (mode-local-print-bindings):
* lisp/net/tramp-cache.el (tramp-flush-connection-properties)
(tramp-list-connections):
* lisp/net/zeroconf.el (zeroconf-list-service-names)
(zeroconf-list-service-types, zeroconf-list-services):
* lisp/org/org.el (org-reload):
* lisp/whitespace.el (whitespace-report-region):
* test/lisp/emacs-lisp/map-tests.el (test-map-do):
Replace add-to-list with push.
---
 lisp/autoinsert.el                |  2 +-
 lisp/cedet/mode-local.el          | 14 ++++++--------
 lisp/net/tramp-cache.el           |  4 ++--
 lisp/net/zeroconf.el              | 12 ++++++------
 lisp/org/org.el                   |  5 +++--
 lisp/whitespace.el                |  2 +-
 test/lisp/emacs-lisp/map-tests.el |  2 +-
 7 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/lisp/autoinsert.el b/lisp/autoinsert.el
index 9bc3aad278..25961d4108 100644
--- a/lisp/autoinsert.el
+++ b/lisp/autoinsert.el
@@ -171,7 +171,7 @@ auto-insert-alist
                  (mapatoms (lambda (mode)
                              (let ((name (symbol-name mode)))
                                (when (string-match "-mode$" name)
-                                 (add-to-list 'modes name)))))
+                                 (push name modes)))))
                  (sort modes 'string<)))
      (completing-read "Local variables for mode: " v1 nil t)
      " . (("
diff --git a/lisp/cedet/mode-local.el b/lisp/cedet/mode-local.el
index a6e143cfcd..a1aea30c20 100644
--- a/lisp/cedet/mode-local.el
+++ b/lisp/cedet/mode-local.el
@@ -819,14 +819,12 @@ mode-local-print-bindings
         )
     ;; Order symbols by type
     (mapatoms
-     #'(lambda (s)
-         (add-to-list (cond
-                       ((get s 'mode-variable-flag)
-                        (if (get s 'constant-flag) 'mc 'mv))
-                       ((get s 'override-flag)
-                        (if (get s 'constant-flag) 'fo 'ov))
-                       ('us))
-                      s))
+     (lambda (s) (push s (cond
+                          ((get s 'mode-variable-flag)
+                           (if (get s 'constant-flag) mc mv))
+                          ((get s 'override-flag)
+                           (if (get s 'constant-flag) fo ov))
+                          (t us))))
      table)
     ;; Print symbols by type
     (when us
diff --git a/lisp/net/tramp-cache.el b/lisp/net/tramp-cache.el
index b81a1a23d5..62e25fa1f0 100644
--- a/lisp/net/tramp-cache.el
+++ b/lisp/net/tramp-cache.el
@@ -373,7 +373,7 @@ tramp-flush-connection-properties
    (let ((hash (gethash key tramp-cache-data))
 	 properties)
      (when (hash-table-p hash)
-       (maphash (lambda (x _y) (add-to-list 'properties x 'append)) hash))
+       (maphash (lambda (x _y) (push x properties)) hash))
      properties))
   (setq tramp-cache-data-changed t)
   (remhash key tramp-cache-data))
@@ -427,7 +427,7 @@ tramp-list-connections
 	 (when (and (tramp-file-name-p key)
 		    (null (tramp-file-name-localname key))
 		    (tramp-connection-property-p key "process-buffer"))
-	   (add-to-list 'result key)))
+	   (push key result)))
        tramp-cache-data)
       result))
 
diff --git a/lisp/net/zeroconf.el b/lisp/net/zeroconf.el
index b8becd712f..cb3c0f2a7e 100644
--- a/lisp/net/zeroconf.el
+++ b/lisp/net/zeroconf.el
@@ -256,17 +256,17 @@ zeroconf-list-service-names
   "Return all discovered Avahi service names as list."
   (let (result)
     (maphash
-     (lambda (_key value) (add-to-list 'result (zeroconf-service-name value)))
+     (lambda (_key value) (push (zeroconf-service-name value) result))
      zeroconf-services-hash)
-    result))
+    (delete-dups result)))
 
 (defun zeroconf-list-service-types ()
   "Return all discovered Avahi service types as list."
   (let (result)
     (maphash
-     (lambda (_key value) (add-to-list 'result (zeroconf-service-type value)))
+     (lambda (_key value) (push (zeroconf-service-type value) result))
      zeroconf-services-hash)
-    result))
+    (delete-dups result)))
 
 (defun zeroconf-list-services (type)
   "Return all discovered Avahi services for a given service type TYPE.
@@ -278,9 +278,9 @@ zeroconf-list-services
     (maphash
      (lambda (_key value)
        (when (equal type (zeroconf-service-type value))
-	 (add-to-list 'result value)))
+	 (push value result)))
      zeroconf-services-hash)
-    result))
+    (delete-dups result)))
 
 (defvar zeroconf-service-added-hooks-hash (make-hash-table :test 'equal)
   "Hash table of hooks for newly added services.
diff --git a/lisp/org/org.el b/lisp/org/org.el
index 5c8b02b9d1..568f5b9b87 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -18682,13 +18682,14 @@ org-reload
 			      (and (string= org-dir contrib-dir)
 				   (org-load-noerror-mustsuffix (concat contrib-dir f)))
 			      (and (org-load-noerror-mustsuffix (concat (org-find-library-dir f) f))
-				   (add-to-list 'load-uncore f 'append)
+				   (push f load-uncore)
 				   't)
 			      f))
 			lfeat)))
     (when load-uncore
       (message "The following feature%s found in load-path, please check if that's correct:\n%s"
-	       (if (> (length load-uncore) 1) "s were" " was") load-uncore))
+	       (if (> (length load-uncore) 1) "s were" " was")
+               (reverse load-uncore)))
     (if load-misses
 	(message "Some error occurred while reloading Org feature%s\n%s\nPlease check *Messages*!\n%s"
 		 (if (> (length load-misses) 1) "s" "") load-misses (org-version nil 'full))
diff --git a/lisp/whitespace.el b/lisp/whitespace.el
index 111b175263..db7c023324 100644
--- a/lisp/whitespace.el
+++ b/lisp/whitespace.el
@@ -1684,7 +1684,7 @@ whitespace-report-region
             (mapcar
              #'(lambda (option)
                  (when force
-                   (add-to-list 'style (car option)))
+                   (push (car option) style))
                  (goto-char rstart)
                  (let ((regexp
                         (cond
diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el
index 06fd55faad..c52bb83fa3 100644
--- a/test/lisp/emacs-lisp/map-tests.el
+++ b/test/lisp/emacs-lisp/map-tests.el
@@ -227,7 +227,7 @@ test-map-do
   (with-maps-do map
     (let ((result nil))
       (map-do (lambda (k v)
-                (add-to-list 'result (list (int-to-string k) v)))
+                (push (list (int-to-string k) v) result))
               map)
       (should (equal result '(("2" 5) ("1" 4) ("0" 3)))))))
 
-- 
2.21.1 (Apple Git-122.3)

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 1 Feb 2020 20:24:56 +0100
> Cc: 39373@debbugs.gnu.org
> 
> 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.

I tend to agree.  Stefan, any comments?

> Two patches attached: a doc update, and a replacement of add-to-list in the cases listed above.

LGTM, on both counts, but let's hear from Stefan (and others, if
someone has an opinion).

Thanks.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Stefan Monnier]

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

[Mattias Engdegård]

1 feb. 2020 kl. 21.15 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

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

Understood, thank you. (Obviously they aren't expanded when the function is called indirectly, but I didn't find anyone doing that with add-to-list.)

> The message is supposed not to trigger when it's applied to dynamically
> scoped var, but it's probably not 100% reliable.

It appears that the warning triggers (as a hard error, actually) when add-to-list is called directly from a function, but not if it only occurs inside a lambda. All the cases found were inside lambdas (or they would have been fixed long ago).

> BTW, this problem doesn't affect only `add-to-list`.  Other culprits
> include `add-hook`, `run-hooks`, `set`, and `symbol-value`.

Right. I see that some of them are detected by the compiler (in byte-compile-form), but add-to-list is commented out.

The set of functions is a bit open-ended; there is also add-to-ordered-list, add-to-history, etc. Not sure how much text needs to be added for all these. There is a general note about lexical variables and symbol values in the manual, in the section about lexical binding.

> LGTM,

Thanks, pushed to emacs-27.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Sat, 1 Feb 2020 22:40:16 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 39373@debbugs.gnu.org
> 
> > BTW, this problem doesn't affect only `add-to-list`.  Other culprits
> > include `add-hook`, `run-hooks`, `set`, and `symbol-value`.
> 
> Right. I see that some of them are detected by the compiler (in byte-compile-form), but add-to-list is commented out.
> 
> The set of functions is a bit open-ended; there is also add-to-ordered-list, add-to-history, etc. Not sure how much text needs to be added for all these. There is a general note about lexical variables and symbol values in the manual, in the section about lexical binding.

I'd prefer to have a short note in the description of each one of
them.

Thanks.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

2 feb. 2020 kl. 04.31 skrev Eli Zaretskii <eliz@gnu.org>:

> I'd prefer to have a short note in the description of each one of
> them.

Right -- I added a note to 'add-to-ordered-list' and 'add-to-history', since their arguments were described as variables.

For 'set' and 'symbol-value', the argument is described as a symbol, which I'd say makes a slight difference here, since lexical variables are mentioned in the description of the relationship between variables and symbols, and also explicitly in the documentation of 'set' in the manual.

The hook functions mostly talk about symbols and hooks, not variables, and I think the risk of them being used on lexvars is slight.

Good enough?

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Stefan Monnier]

> The hook functions mostly talk about symbols and hooks, not variables, and
> I think the risk of them being used on lexvars is slight.

I also expected so, but I've already changed a handful of
`(run-hooks 'foo)` to `(mapcar #'funcall foo)`.  The problem is a bit
different, tho: it's *also* wrong to use it on dynamically-scoped let-bound
vars (tho it mostly works), so the doc should not focus on lexical
vars but on the difference between vars and hooks.


        Stefan

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

2 feb. 2020 kl. 14.55 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> I also expected so, but I've already changed a handful of
> `(run-hooks 'foo)` to `(mapcar #'funcall foo)`.  The problem is a bit
> different, tho: it's *also* wrong to use it on dynamically-scoped let-bound
> vars (tho it mostly works), so the doc should not focus on lexical
> vars but on the difference between vars and hooks.

Very well, in that case you are probably better placed to provide the nuanced modifications necessary, if any.

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> 2 feb. 2020 kl. 14.55 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
>> I also expected so, but I've already changed a handful of
>> `(run-hooks 'foo)` to `(mapcar #'funcall foo)`.  The problem is a bit
>> different, tho: it's *also* wrong to use it on dynamically-scoped let-bound
>> vars (tho it mostly works), so the doc should not focus on lexical
>> vars but on the difference between vars and hooks.
>
> Very well, in that case you are probably better placed to provide the
> nuanced modifications necessary, if any.

If I'm reading this thread right, the specific problem in this bug
report was fixed (and the documentation of add-to-list updated), but the
more general problem of going through all the instances of add-to-list
(and friends) in lexical files hasn't been done?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Mattias Engdegård]

9 aug. 2020 kl. 13.39 skrev Lars Ingebrigtsen <larsi@gnus.org>:

> If I'm reading this thread right, the specific problem in this bug
> report was fixed (and the documentation of add-to-list updated), but the
> more general problem of going through all the instances of add-to-list
> (and friends) in lexical files hasn't been done?

No, I believe add-to-list calls have been updated and I would be happy to mark the bug as done.
Stefan alluded to some improvement possibilities in the hook function documentation (re the difference between variables and hooks).

bug#39373: 27.0.50; [PATCH] mode-local-print-bindings broken with lexical-binding

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> 9 aug. 2020 kl. 13.39 skrev Lars Ingebrigtsen <larsi@gnus.org>:
>
>> If I'm reading this thread right, the specific problem in this bug
>> report was fixed (and the documentation of add-to-list updated), but the
>> more general problem of going through all the instances of add-to-list
>> (and friends) in lexical files hasn't been done?
>
> No, I believe add-to-list calls have been updated and I would be happy
> to mark the bug as done.
> Stefan alluded to some improvement possibilities in the hook function
> documentation (re the difference between variables and hooks).

OK; then I'll just close this bug report, and if a new bug report about
those improvements can be opened (if that helps to track any work to be
done in this area).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no