all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* 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.