GNU bug report logs - #39428
28.0.50; cl--push-clause-loop-body: eval macro arg just once

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39428 in the body.
You can then email your comments to 39428 AT debbugs.gnu.org in the normal way.

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

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; cl--push-clause-loop-body: eval macro arg just once
Date: Tue, 04 Feb 2020 22:05:30 +0100

X-Debbugs-Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>,dick.r.chiang <at> gmail.com

I think this little tweak has sense.  WDYT?


--8<-----------------------------cut here---------------start------------->8---
commit eb88afb89a8cfb760d89569e74c8ece1d1c446cb
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Tue Feb 4 21:52:06 2020 +0100

    Eval macro arg just once
    
    * lisp/emacs-lisp/cl-macs.el (cl--push-clause-loop-body):
    Use `macroexp-let2'.

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 9d0fd15bc3..4c2f58907d 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -1037,9 +1037,10 @@ cl-loop
 
 (defmacro cl--push-clause-loop-body (clause)
   "Apply CLAUSE to both `cl--loop-conditions' and `cl--loop-body'."
-  `(progn
-     (push ,clause cl--loop-conditions)
-     (push ,clause cl--loop-body)))
+  (macroexp-let2 nil sym clause
+    `(progn
+       (push ,sym cl--loop-conditions)
+       (push ,sym cl--loop-body))))
 
 ;; Below is a complete spec for cl-loop, in several parts that correspond
 ;; to the syntax given in CLtL2.  The specs do more than specify where

--8<-----------------------------cut here---------------end--------------->8---


In GNU Emacs 28.0.50 (build 21, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2020-02-04 built on calancha-pc.dy.bbexcite.jp
Repository revision: e287da5a8154d83a97107b64915ccc17e3a086b8
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

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

From: dick.r.chiang <at> gmail.com
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 39428 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Tue, 04 Feb 2020 16:15:25 -0500

LGTM, thanks.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 39428 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Tue, 04 Feb 2020 19:26:28 -0500

> I think this little tweak has sense.  WDYT?

Yes, it looks good (tho I must say I don't really understand what
cl--loop-conditions and cl--loop-body are for and even less why we'd
want to push the same clause onto both lists).


        Stefan

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 39428 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Wed, 05 Feb 2020 19:03:40 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> I must say I don't really understand what
> cl--loop-conditions and cl--loop-body are for and even less why we'd
> want to push the same clause onto both lists

Those things came from bug#29799.  I have not followed the latest
changes in that report; the purpose seems to cover some
edge cases.  It has no impact on our test suite; furthermore, they
have added new tests for those edge cases matching what clisp does.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 39428-done <at> debbugs.gnu.org,
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Wed, 05 Feb 2020 19:08:42 +0100

dick.r.chiang <at> gmail.com writes:

> LGTM, thanks.

Pushed at master branch as commit 'Eval macro arg just once'
(ebff24c0b8fa54854fe8445f2eba1d99fb76ecf2)

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: eliz <at> gnu.org
Cc: 39428 <at> debbugs.gnu.org
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Wed, 05 Feb 2020 19:08:42 +0100 (5 minutes, 25 seconds ago)

Tino Calancha <tino.calancha <at> gmail.com> writes:

> LGTM, thanks.
>Pushed at master branch as commit 'Eval macro arg just once'
>(ebff24c0b8fa54854fe8445f2eba1d99fb76ecf2)
Eli, I have noticed that, even if I opened this report for emacs master,
the code is also at emacs-27.
Do we need to cherry pick this fix to emacs-27 now?
Or should we wait after the release?
IMO, it's not urgent, I mean, I have not a case where it's
breaking something; it's simply more correct, because you are
protected from unwanted side-effects.
Message-ID: <874kw4nb7m.fsf <at> calancha-pc.dy.bbexcite.jp>

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 39428 <at> debbugs.gnu.org
Subject: Re: bug#39428: 28.0.50; cl--push-clause-loop-body: eval macro arg
 just once
Date: Wed, 05 Feb 2020 20:35:47 +0200

> From: Tino Calancha <tino.calancha <at> gmail.com>
> Cc: 39428 <at> debbugs.gnu.org
> Date: Wed, 05 Feb 2020 19:08:42 +0100 (5 minutes, 25 seconds ago)
> 
> >Pushed at master branch as commit 'Eval macro arg just once'
> >(ebff24c0b8fa54854fe8445f2eba1d99fb76ecf2)
> Eli, I have noticed that, even if I opened this report for emacs master,
> the code is also at emacs-27.
> Do we need to cherry pick this fix to emacs-27 now?

I don't see a need, this doesn't seem to be serious enough to have in
Emacs 27.

> IMO, it's not urgent, I mean, I have not a case where it's
> breaking something; it's simply more correct, because you are
> protected from unwanted side-effects.

Then it's definitely for master.

Thanks.

Previous Next

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