GNU bug report logs - #56739
29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros

Previous Next

To reply to this bug, email your comments to 56739 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Wing Hei Chan <whmunkchan <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize symbol macros
Date: Sun, 24 Jul 2022 20:12:10 +0800

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

Message #8 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Wing Hei Chan <whmunkchan <at> outlook.com>
Cc: 56739 <at> debbugs.gnu.org
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Tue, 02 Aug 2022 04:41:38 +0200

Wing Hei Chan <whmunkchan <at> 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.

Message #11 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Wing Hei Chan <whmunkchan <at> outlook.com>
Cc: 56739 <at> debbugs.gnu.org
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Wed, 03 Aug 2022 01:59:01 +0200

Wing Hei Chan <whmunkchan <at> 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.

Message #14 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Wing Hei Chan <whmunkchan <at> outlook.com>
Cc: 56739 <at> debbugs.gnu.org
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Wed, 03 Aug 2022 02:20:55 +0200

[Message part 1 (text/plain, inline)]

Hello,

I think we can fix this (both cases mentioned) similarly as in cl-letf:

[0001-WIP-Fix-symbol-macros-used-in-cl-psetf-Bug-56739.patch (text/x-diff, inline)]

From 29ea21a751ab6e71b2fb34c781131e31fc7b950d Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> 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

[Message part 3 (text/plain, inline)]

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.

Message #19 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Mon, 05 Sep 2022 21:14:35 +0200

Michael Heerdegen <michael_heerdegen <at> 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 <at> 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.

Message #22 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Tue, 06 Sep 2022 03:29:48 +0200

Lars Ingebrigtsen <larsi <at> 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.

Message #25 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Mon, 05 Sep 2022 22:22:28 -0400

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

Message #28 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Wing Hei Chan <whmunkchan <at> outlook.com>
Cc: 56739 <at> debbugs.gnu.org
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Mon, 05 Sep 2022 22:29:24 -0400

> 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

Message #31 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 56739 <at> debbugs.gnu.org,
 Wing Hei Chan <whmunkchan <at> outlook.com>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Thu, 7 Sep 2023 11:31:34 -0700

Stefan Monnier <monnier <at> 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.

Message #34 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Fri, 08 Sep 2023 05:07:23 +0200

Stefan Kangas <stefankangas <at> 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.

Message #37 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 09 Sep 2023 08:12:00 +0200

Michael Heerdegen <michael_heerdegen <at> 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.

Message #40 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 56739 <at> debbugs.gnu.org, Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Kangas <stefankangas <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 09 Sep 2023 09:08:36 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Michael Heerdegen <michael_heerdegen <at> 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...

Message #43 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Wing Hei Chan <whmunkchan <at> outlook.com>
Cc: "56739 <at> debbugs.gnu.org" <56739 <at> debbugs.gnu.org>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 09 Sep 2023 11:24:43 +0200

Wing Hei Chan <whmunkchan <at> 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.

Message #46 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Gerd Möllmann <gerd.moellmann <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 56739 <at> debbugs.gnu.org,
 Wing Hei Chan <whmunkchan <at> outlook.com>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 09 Sep 2023 11:42:04 -0400

> 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

Message #49 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 56739 <at> debbugs.gnu.org,
 Wing Hei Chan <whmunkchan <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 09 Sep 2023 18:33:59 +0200

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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 :-)

Message #55 received at 56739 <at> debbugs.gnu.org (full text, mbox):

From: Wing Hei Chan <whmunkchan <at> outlook.com>
To: Gerd Möllmann <gerd.moellmann <at> gmail.com>
Cc: "56739 <at> debbugs.gnu.org" <56739 <at> debbugs.gnu.org>
Subject: Re: bug#56739: 29.0.50; `cl-psetq' and `cl-psetf' fail to recognize
 symbol macros
Date: Sat, 9 Sep 2023 08:03:42 +0000

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.