* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
@ 2022-07-24 12:12 Wing Hei Chan
2022-08-02 2:41 ` Michael Heerdegen
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Wing Hei Chan @ 2022-07-24 12:12 UTC (permalink / raw)
To: 56739
The following form produces (2 2) due to the failure of detecting
dependencies involving symbol macros.
(cl-symbol-macrolet ((c a))
(let ((a 1) (b 2))
(cl-psetq a b
b c)
(list a b)))
It should have behaved the same as the following form without symbol
macros, that is, producing (2 1).
(let ((a 1) (b 2))
(cl-psetq a b
b a)
(list a b))
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-07-24 12:12 bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros Wing Hei Chan
@ 2022-08-02 2:41 ` Michael Heerdegen
2022-08-02 23:59 ` Michael Heerdegen
2022-09-06 2:29 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2 siblings, 0 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-02 2:41 UTC (permalink / raw)
To: Wing Hei Chan; +Cc: 56739
Wing Hei Chan <whmunkchan@outlook.com> writes:
> The following form produces (2 2) due to the failure of detecting
> dependencies involving symbol macros.
>
> (cl-symbol-macrolet ((c a))
> (let ((a 1) (b 2))
> (cl-psetq a b
> b c)
> (list a b)))
Dunno how a good fix would look like - but it is easy to follow why this
error happens: `cl-psetf' analyses the expressions (each second
argument) for whether they are "simple" (independent of the variables):
(if (or (not (symbolp (car p))) (cl--expr-depends-p (nth 1 p) vars))
(setq simple nil))
but the symbol macro is not yet expanded when this is done, and the
expression `c` passes the test - which is wrong.
Maybe that test should just check for symbol macros in addition?
Michael.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-07-24 12:12 bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros Wing Hei Chan
2022-08-02 2:41 ` Michael Heerdegen
@ 2022-08-02 23:59 ` Michael Heerdegen
2022-08-03 0:20 ` Michael Heerdegen
2022-09-06 2:29 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-02 23:59 UTC (permalink / raw)
To: Wing Hei Chan; +Cc: 56739
Wing Hei Chan <whmunkchan@outlook.com> writes:
> The following form produces (2 2) due to the failure of detecting
> dependencies involving symbol macros.
>
> (cl-symbol-macrolet ((c a))
> (let ((a 1) (b 2))
> (cl-psetq a b
> b c)
> (list a b)))
There is a second case that also fails: a symbol macro at a PLACE
position:
(cl-symbol-macrolet ((c a))
(let ((a 1))
(cl-psetf c 2
b a)
(list a b)))
==> (2 2) ;should be (2 1)
Michael.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-08-02 23:59 ` Michael Heerdegen
@ 2022-08-03 0:20 ` Michael Heerdegen
2022-09-05 19:14 ` Lars Ingebrigtsen
2022-09-06 2:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 2 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-03 0:20 UTC (permalink / raw)
To: Wing Hei Chan; +Cc: 56739
[-- Attachment #1: Type: text/plain, Size: 81 bytes --]
Hello,
I think we can fix this (both cases mentioned) similarly as in cl-letf:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Fix-symbol-macros-used-in-cl-psetf-Bug-56739.patch --]
[-- Type: text/x-diff, Size: 1447 bytes --]
From 29ea21a751ab6e71b2fb34c781131e31fc7b950d Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 3 Aug 2022 02:06:16 +0200
Subject: [PATCH] WIP: Fix symbol macros used in cl-psetf (Bug#56739)
---
lisp/emacs-lisp/cl-macs.el | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 78d19db479..f3051752ba 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2653,12 +2653,17 @@ cl-psetf
\(fn PLACE VAL PLACE VAL ...)"
(declare (debug setf))
- (let ((p args) (simple t) (vars nil))
+ (let ((p args) (simple t) (vars nil)
+ (smacros (alist-get :cl-symbol-macros macroexpand-all-environment)))
(while p
- (if (or (not (symbolp (car p))) (cl--expr-depends-p (nth 1 p) vars))
- (setq simple nil))
- (if (memq (car p) vars)
- (error "Destination duplicated in psetf: %s" (car p)))
+ (when (or (not (symbolp (car p)))
+ (assq (car p) smacros)
+ (and (symbolp (nth 1 p))
+ (assq (nth 1 p) smacros))
+ (cl--expr-depends-p (nth 1 p) vars))
+ (setq simple nil))
+ (when (memq (car p) vars)
+ (error "Destination duplicated in psetf: %s" (car p)))
(push (pop p) vars)
(or p (error "Odd number of arguments to cl-psetf"))
(pop p))
--
2.30.2
[-- Attachment #3: Type: text/plain, Size: 353 bytes --]
But I'm not a fan of symbol macros any more: the concept sounds nice
first, but actually you only save one pair of parens when coding while
they introduce a special case that one always has too keep in mind for
macro expansions: any symbol might not just be a symbol.
I guess this is not the only place where they are not handled correctly.
Michael.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-08-03 0:20 ` Michael Heerdegen
@ 2022-09-05 19:14 ` Lars Ingebrigtsen
2022-09-06 1:29 ` Michael Heerdegen
2022-09-06 2:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-05 19:14 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: 56739, Wing Hei Chan, Stefan Monnier
Michael Heerdegen <michael_heerdegen@web.de> writes:
> I think we can fix this (both cases mentioned) similarly as in cl-letf:
>
> From 29ea21a751ab6e71b2fb34c781131e31fc7b950d Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Wed, 3 Aug 2022 02:06:16 +0200
> Subject: [PATCH] WIP: Fix symbol macros used in cl-psetf (Bug#56739)
This was a month ago, but as far as I can tell, this patch was never
pushed.
Perhaps Stefan has some comments here; added to the CCs.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-09-05 19:14 ` Lars Ingebrigtsen
@ 2022-09-06 1:29 ` Michael Heerdegen
0 siblings, 0 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-09-06 1:29 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 56739, Wing Hei Chan, Stefan Monnier
Lars Ingebrigtsen <larsi@gnus.org> writes:
> This was a month ago, but as far as I can tell, this patch was never
> pushed.
I didn't want to push without at least one person saying "looks good".
Michael.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-08-03 0:20 ` Michael Heerdegen
2022-09-05 19:14 ` Lars Ingebrigtsen
@ 2022-09-06 2:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-09-07 18:31 ` Stefan Kangas
1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-06 2:22 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: 56739, Wing Hei Chan
> - (let ((p args) (simple t) (vars nil))
> + (let ((p args) (simple t) (vars nil)
> + (smacros (alist-get :cl-symbol-macros macroexpand-all-environment)))
> (while p
> - (if (or (not (symbolp (car p))) (cl--expr-depends-p (nth 1 p) vars))
> - (setq simple nil))
> - (if (memq (car p) vars)
> - (error "Destination duplicated in psetf: %s" (car p)))
> + (when (or (not (symbolp (car p)))
> + (assq (car p) smacros)
This looks like a safe way to make it work when the place is
a symbol macro.
> + (and (symbolp (nth 1 p))
> + (assq (nth 1 p) smacros))
> + (cl--expr-depends-p (nth 1 p) vars))
But this doesn't look strong enough; if (nth 1 p) is (+ c c) where `c`
is a symbol macro the same problem will appear. So the test needs to be
pushed into `cl--expr-depends-p` (probably into `cl--safe-expr-p` where
it will fix other related problems elsewhere).
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-07-24 12:12 bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros Wing Hei Chan
2022-08-02 2:41 ` Michael Heerdegen
2022-08-02 23:59 ` Michael Heerdegen
@ 2022-09-06 2:29 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2 siblings, 0 replies; 16+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-06 2:29 UTC (permalink / raw)
To: Wing Hei Chan; +Cc: 56739
> The following form produces (2 2) due to the failure of detecting
> dependencies involving symbol macros.
>
> (cl-symbol-macrolet ((c a))
> (let ((a 1) (b 2))
> (cl-psetq a b
> b c)
> (list a b)))
BTW, another way to get similar problems is with `defvaralias`:
(defvaralias 'my-c 'my-a)
(defvar my-a 1)
(let ((b 2))
(cl-psetq my-a b
b my-c)
(list my-a b))
We should probably introduce a `(macroexp|cl)--simple-var-p` test to
catch the various cases where symbols aren't simple variables.
Another thing that might be worth digging into is how important is that
`cl-psetf` optimization for lexical variables (remember: this was
written back in the days of dynamic scoping and Mattias has made some
improvements to the compiler which may render this optimization
redundant inside the macro itself.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2022-09-06 2:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-09-07 18:31 ` Stefan Kangas
2023-09-08 3:07 ` Michael Heerdegen
0 siblings, 1 reply; 16+ messages in thread
From: Stefan Kangas @ 2023-09-07 18:31 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Michael Heerdegen, 56739, Wing Hei Chan
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> - (let ((p args) (simple t) (vars nil))
>> + (let ((p args) (simple t) (vars nil)
>> + (smacros (alist-get :cl-symbol-macros macroexpand-all-environment)))
>> (while p
>> - (if (or (not (symbolp (car p))) (cl--expr-depends-p (nth 1 p) vars))
>> - (setq simple nil))
>> - (if (memq (car p) vars)
>> - (error "Destination duplicated in psetf: %s" (car p)))
>> + (when (or (not (symbolp (car p)))
>> + (assq (car p) smacros)
>
> This looks like a safe way to make it work when the place is
> a symbol macro.
>
>> + (and (symbolp (nth 1 p))
>> + (assq (nth 1 p) smacros))
>> + (cl--expr-depends-p (nth 1 p) vars))
>
> But this doesn't look strong enough; if (nth 1 p) is (+ c c) where `c`
> is a symbol macro the same problem will appear. So the test needs to be
> pushed into `cl--expr-depends-p` (probably into `cl--safe-expr-p` where
> it will fix other related problems elsewhere).
(That was one year ago.)
Michael, did you have a chance to look into this? It would be nice to
get this bug fixed.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-07 18:31 ` Stefan Kangas
@ 2023-09-08 3:07 ` Michael Heerdegen
2023-09-09 6:12 ` Michael Heerdegen
0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2023-09-08 3:07 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 56739, Wing Hei Chan, Stefan Monnier
Stefan Kangas <stefankangas@gmail.com> writes:
> Michael, did you have a chance to look into this? It would be nice to
> get this bug fixed.
I would rather leave this one to Stefan to be honest because I'm only
partially understanding what is needed.
Michael.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-08 3:07 ` Michael Heerdegen
@ 2023-09-09 6:12 ` Michael Heerdegen
2023-09-09 7:08 ` Gerd Möllmann
0 siblings, 1 reply; 16+ messages in thread
From: Michael Heerdegen @ 2023-09-09 6:12 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Gerd Möllmann, 56739, Wing Hei Chan, Stefan Monnier
Michael Heerdegen <michael_heerdegen@web.de> writes:
> > Michael, did you have a chance to look into this? It would be nice to
> > get this bug fixed.
>
> I would rather leave this one to Stefan to be honest because I'm only
> partially understanding what is needed.
Or maybe Gerd can help?
Don't want to bother you Gerd, but maybe you are even interested in this
one.
Regards,
Michael.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-09 6:12 ` Michael Heerdegen
@ 2023-09-09 7:08 ` Gerd Möllmann
[not found] ` <f49326d6-85e2-4fed-83ae-815a82cefe30@outlook.com>
2023-09-09 15:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 2 replies; 16+ messages in thread
From: Gerd Möllmann @ 2023-09-09 7:08 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: 56739, Wing Hei Chan, Stefan Kangas, Stefan Monnier
Michael Heerdegen <michael_heerdegen@web.de> writes:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> > Michael, did you have a chance to look into this? It would be nice to
>> > get this bug fixed.
>>
>> I would rather leave this one to Stefan to be honest because I'm only
>> partially understanding what is needed.
>
> Or maybe Gerd can help?
Not sure I can help.
In CL, all the p?set[qf] handle symbol macros. I haven't tested this,
but from doc strings I'd say none of these supports symbol macros in
Emacs.
There is also an intricacy here:
Setq, with CL behaviour, would behave like setf for symbol macros. And
setq is in C. I don't know if maintainers would accept such a change in
C (performance, dependencies, ...).
So...
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
[not found] ` <f49326d6-85e2-4fed-83ae-815a82cefe30@outlook.com>
@ 2023-09-09 8:03 ` Wing Hei Chan
2023-09-09 9:24 ` Gerd Möllmann
0 siblings, 1 reply; 16+ messages in thread
From: Wing Hei Chan @ 2023-09-09 8:03 UTC (permalink / raw)
To: Gerd Möllmann; +Cc: 56739@debbugs.gnu.org
Resending this message as I didn't send it to the thread.
> Afaik, *left-hand* side symbol macros are all handled correctly:
>
> - `setq' is handled by `cl-symbol-macrolet' itself;
> - `setf' expands symbol macros as needed;
> - `cl-setq' and `cl-setf' expand to `setf'.
>
> The original bug is about *right-hand* side symbol macros, namely that
> `cl-setq' and `cl-setf' can introduce an unsound optimization in such
> cases. In fact, I'm not sure this optimization is needed anymore (see
> Stefan's comment). The simplest way to fix the bug is to remove the
> optimization, but I'll leave the decision to more knowledgeable people.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-09 8:03 ` Wing Hei Chan
@ 2023-09-09 9:24 ` Gerd Möllmann
0 siblings, 0 replies; 16+ messages in thread
From: Gerd Möllmann @ 2023-09-09 9:24 UTC (permalink / raw)
To: Wing Hei Chan; +Cc: 56739@debbugs.gnu.org
Wing Hei Chan <whmunkchan@outlook.com> writes:
> Resending this message as I didn't send it to the thread.
>
> > Afaik, *left-hand* side symbol macros are all handled correctly:
> >
> > - `setq' is handled by `cl-symbol-macrolet' itself;
> > - `setf' expands symbol macros as needed;
> > - `cl-setq' and `cl-setf' expand to `setf'.
> >
> > The original bug is about *right-hand* side symbol macros, namely that
> > `cl-setq' and `cl-setf' can introduce an unsound optimization in such
> > cases. In fact, I'm not sure this optimization is needed anymore (see
> > Stefan's comment). The simplest way to fix the bug is to remove the
> > optimization, but I'll leave the decision to more knowledgeable people.
Then I've possibly misunderstood what this is about, sorry.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-09 7:08 ` Gerd Möllmann
[not found] ` <f49326d6-85e2-4fed-83ae-815a82cefe30@outlook.com>
@ 2023-09-09 15:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-09-09 16:33 ` Gerd Möllmann
1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-09 15:42 UTC (permalink / raw)
To: Gerd Möllmann; +Cc: Michael Heerdegen, 56739, Wing Hei Chan, Stefan Kangas
> Setq, with CL behaviour, would behave like setf for symbol macros. And
> setq is in C. I don't know if maintainers would accept such a change in
> C (performance, dependencies, ...).
Quoting cl-macs.el:
[...]
(defun cl--sm-macroexpand-1 (orig-fun exp &optional env)
[...]
;; Convert setq to setf if required by symbol-macro expansion.
[...]
-- Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
2023-09-09 15:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-09-09 16:33 ` Gerd Möllmann
0 siblings, 0 replies; 16+ messages in thread
From: Gerd Möllmann @ 2023-09-09 16:33 UTC (permalink / raw)
To: 56739; +Cc: michael_heerdegen, whmunkchan, monnier, stefankangas
Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:
>> Setq, with CL behaviour, would behave like setf for symbol macros. And
>> setq is in C. I don't know if maintainers would accept such a change in
>> C (performance, dependencies, ...).
>
> Quoting cl-macs.el:
>
> [...]
> (defun cl--sm-macroexpand-1 (orig-fun exp &optional env)
> [...]
> ;; Convert setq to setf if required by symbol-macro expansion.
> [...]
Clever :-)
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-09-09 16:33 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-24 12:12 bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros Wing Hei Chan
2022-08-02 2:41 ` Michael Heerdegen
2022-08-02 23:59 ` Michael Heerdegen
2022-08-03 0:20 ` Michael Heerdegen
2022-09-05 19:14 ` Lars Ingebrigtsen
2022-09-06 1:29 ` Michael Heerdegen
2022-09-06 2:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-09-07 18:31 ` Stefan Kangas
2023-09-08 3:07 ` Michael Heerdegen
2023-09-09 6:12 ` Michael Heerdegen
2023-09-09 7:08 ` Gerd Möllmann
[not found] ` <f49326d6-85e2-4fed-83ae-815a82cefe30@outlook.com>
2023-09-09 8:03 ` Wing Hei Chan
2023-09-09 9:24 ` Gerd Möllmann
2023-09-09 15:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-09-09 16:33 ` Gerd Möllmann
2022-09-06 2:29 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.