bug#6583: 23.2; cl loop macro with `and' clause

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Kevin Ryde
Evaluating

    (require 'cl)
    (loop for elem in '(1 2 3)
          for k = elem and j = 99
          do
          (print k))

shows

    1
    1
    2

where I thought it might be

    1
    2
    3

I'm don't know much about the cl loop macro but thought the `for k' step
would be evaluated after the `for elem' step, "sequential" per the cl
info manual near the end of "For Clauses"

    If you include several `for' clauses in a row, they are treated
    sequentially

The 1,2,3 is what you get from pasting the same form into clisp, if that
suggests what an actual common lisp does or should do.  And in Emacs
it's had if you omit the "and j",

    (loop for elem in '(1 2 3)
          for k = elem
          do
          (print k))
    =>
    1 2 3

Nosing around the macro expansion I wondered if the "step" of k/j gets
mispositioned if there's an `and', but it's hard to be sure.


I struck this when making a loop over an alist where I thought to take
apart the key and value with an `and' as they didn't need to be
sequential,

    (loop for elem in my-alist
          for k = (car elem) and v = (cdr elem)
          do
          ...

Alas the effect of the "1 1 2" was to double the first element and omit
the last.


In GNU Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0)
 of 2010-05-16 on raven, modified by Debian
configured using `configure  '--build' 'i486-linux-gnu' '--build' 'i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.2/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.2/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_AU
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default enable-multibyte-characters: t



Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Thierry Volpiatto-2
Kevin Ryde <[hidden email]> writes:

> Evaluating
>
>     (require 'cl)
>     (loop for elem in '(1 2 3)
>           for k = elem and j = 99
>           do
>           (print k))

IMHO this is not correct, 'and' clauses should be used after conditionals
(e.g if, when etc..)

,----
| ELISP>  (loop for elem in '(1 2 3)
|            for k = elem
|            if (oddp k)
|              collect k and do (print k))
`----

> shows
>
>     1
>     1
>     2

Instead of returning this, emacs should throw an error as SBCL does for
same code.

,----
| CL-USER> (loop for i in '(1 2 3)
|               for k = i and j = 99
|               (print k))
| ; Evaluation aborted.
`----

> where I thought it might be
>
>     1
>     2
>     3
>
> I'm don't know much about the cl loop macro but thought the `for k' step
> would be evaluated after the `for elem' step, "sequential" per the cl
> info manual near the end of "For Clauses"
>
>     If you include several `for' clauses in a row, they are treated
>     sequentially
>
> The 1,2,3 is what you get from pasting the same form into clisp, if that
> suggests what an actual common lisp does or should do.  And in Emacs
> it's had if you omit the "and j",
>
>     (loop for elem in '(1 2 3)
>           for k = elem
>           do
>           (print k))
>     =>
>     1 2 3
>
> Nosing around the macro expansion I wondered if the "step" of k/j gets
> mispositioned if there's an `and', but it's hard to be sure.
>
>
> I struck this when making a loop over an alist where I thought to take
> apart the key and value with an `and' as they didn't need to be
> sequential,
>
>     (loop for elem in my-alist
>           for k = (car elem) and v = (cdr elem)
>           do
>           ...
>
> Alas the effect of the "1 1 2" was to double the first element and omit
> the last.
>
>
> In GNU Emacs 23.2.1 (i486-pc-linux-gnu, GTK+ Version 2.20.0)
>  of 2010-05-16 on raven, modified by Debian
> configured using `configure  '--build' 'i486-linux-gnu' '--build' 'i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.2/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.2/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''
>
> Important settings:
>   value of $LC_ALL: nil
>   value of $LC_COLLATE: nil
>   value of $LC_CTYPE: nil
>   value of $LC_MESSAGES: nil
>   value of $LC_MONETARY: nil
>   value of $LC_NUMERIC: nil
>   value of $LC_TIME: nil
>   value of $LANG: en_AU
>   value of $XMODIFIERS: nil
>   locale-coding-system: iso-latin-1-unix
>   default enable-multibyte-characters: t
>
>
>
>

--
Thierry Volpiatto
Gpg key: http://pgp.mit.edu/




Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Lawrence Mitchell
In reply to this post by Kevin Ryde
Kevin Ryde wrote:
[...]

> I struck this when making a loop over an alist where I thought to take
> apart the key and value with an `and' as they didn't need to be
> sequential,

>     (loop for elem in my-alist
>           for k = (car elem) and v = (cdr elem)
>           do
>           ...

Note the idiomatic way of writing this loop:

(loop for (k . v) in my-alist
      do ...)

Although this does not address the question of the bug.

--
Lawrence Mitchell <[hidden email]>




Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Noam Postavsky-2
In reply to this post by Kevin Ryde
tag 6583 + confirmed
found 6583 24.5
found 6583 25.0.94
quit

Thierry Volpiatto <thierry.volpiatto <at> gmail.com> wrote:
> IMHO this is not correct, 'and' clauses should be used after conditionals
> (e.g if, when etc..)

No, it should also work for 'for' clauses, as documented in CL Hyperspec [1]

    for-as-clause::= {for | as} for-as-subclause {and for-as-subclause}*

and Emacs' CL manual [2]

    If you include several ‘for’ clauses in a row, they are treated
    sequentially (as if by ‘let*’ and ‘setq’).  You can instead use the
    word ‘and’ to link the clauses, in which case they are processed in
    parallel (as if by ‘let’ and ‘cl-psetq’).

The SBCL example is just missing the 'do', that's why it failed.


[1]: http://www.lispworks.com/documentation/lw51/CLHS/Body/m_loop.htm
[2]: http://www.gnu.org/software/emacs/manual/html_node/cl/For-Clauses.html



Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Alex Gramiak
In reply to this post by Kevin Ryde
tags 6583 patch
quit

Kevin Ryde <[hidden email]> writes:

> Evaluating
>
>     (require 'cl)
>     (loop for elem in '(1 2 3)
>           for k = elem and j = 99
>           do
>           (print k))
>
> shows
>
>     1
>     1
>     2
>
> where I thought it might be
>
>     1
>     2
>     3
>
> I'm don't know much about the cl loop macro but thought the `for k' step
> would be evaluated after the `for elem' step, "sequential" per the cl
> info manual near the end of "For Clauses"
>
>     If you include several `for' clauses in a row, they are treated
>     sequentially
>
> The 1,2,3 is what you get from pasting the same form into clisp, if that
> suggests what an actual common lisp does or should do.  And in Emacs
> it's had if you omit the "and j",
>
>     (loop for elem in '(1 2 3)
>           for k = elem
>           do
>           (print k))
>     =>
>     1 2 3
>
> Nosing around the macro expansion I wondered if the "step" of k/j gets
> mispositioned if there's an `and', but it's hard to be sure.
>
>
> I struck this when making a loop over an alist where I thought to take
> apart the key and value with an `and' as they didn't need to be
> sequential,
>
>     (loop for elem in my-alist
>           for k = (car elem) and v = (cdr elem)
>           do
>           ...
>
> Alas the effect of the "1 1 2" was to double the first element and omit
> the last.
This problem appears to have been present even in the initial revision
(fcd737693e8), specifically in the (eq word '=) clause.

I see no reason for the (or ands (eq (car args) 'and)) consequent, as
what it does when it detects an 'and is:

* In the first iteration, set var (k) to the first form (elem) at the
  beginning of the iteration

* In subsequent iterations, set var (k) to itself at the beginning of
  the iteration (noop)

* At the end of every iteration, set var (k) to the second form
  (defaulting to the first form, elem).

The last point is the main problem: you can't set the variable to the
first form at the end of the iteration as it might depend on other loop
variables (in this case elem) that are updated at the beginning of the
iteration.

I've attached a patch below that appears to solve this issue. With the
patch, var is set to the first or second form at the beginning of the
iteration (just like it is when no 'and is present).


0001-Fix-and-in-cl-loop-s-for-as-equals-then-subclause.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Noam Postavsky-2
Alex <[hidden email]> writes:

>
> This problem appears to have been present even in the initial revision
> (fcd737693e8), specifically in the (eq word '=) clause.
>
> I see no reason for the (or ands (eq (car args) 'and)) consequent, as
> what it does when it detects an 'and is:
>
> * In the first iteration, set var (k) to the first form (elem) at the
>   beginning of the iteration
>
> * In subsequent iterations, set var (k) to itself at the beginning of
>   the iteration (noop)
>
> * At the end of every iteration, set var (k) to the second form
>   (defaulting to the first form, elem).
>
> The last point is the main problem: you can't set the variable to the
> first form at the end of the iteration as it might depend on other loop
> variables (in this case elem) that are updated at the beginning of the
> iteration.
>
> I've attached a patch below that appears to solve this issue. With the
> patch, var is set to the first or second form at the beginning of the
> iteration (just like it is when no 'and is present).

I can't claim to fully understand the loop macro implementation, but
your patch breaks this example from the manual `(cl) For Clauses':

     (cl-loop for x below 5 for y = nil then x collect (list x y))
             => ((0 nil) (1 1) (2 2) (3 3) (4 4))
     (cl-loop for x below 5 and y = nil then x collect (list x y))
             => ((0 nil) (1 0) (2 1) (3 2) (4 3))

With your patch the second loop gives ((0 nil) (1 1) (2 2) (3 3) (4 4))
like the first.



Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Alex Gramiak
[hidden email] writes:

> Alex <[hidden email]> writes:

> I can't claim to fully understand the loop macro implementation, but
> your patch breaks this example from the manual `(cl) For Clauses':
>
>      (cl-loop for x below 5 for y = nil then x collect (list x y))
>              => ((0 nil) (1 1) (2 2) (3 3) (4 4))
>      (cl-loop for x below 5 and y = nil then x collect (list x y))
>              => ((0 nil) (1 0) (2 1) (3 2) (4 3))
>
> With your patch the second loop gives ((0 nil) (1 1) (2 2) (3 3) (4 4))
> like the first.

You're right, sorry. This breaks loops with variables that are updated
in loop-for-steps rather than loop-for-sets. When I started testing
other cases I was accidentally using the pre-patch branch to do so.

I can't think of an easy solution to cover both problems right now. If
no one better suited can figure this out, I'll come back to this after
completing an ert suite for cl-loop (of which I'm part-way through).



Reply | Threaded
Open this post in threaded view
|

bug#6583: 23.2; cl loop macro with `and' clause

Lars Ingebrigtsen
Alex <[hidden email]> writes:

> You're right, sorry. This breaks loops with variables that are updated
> in loop-for-steps rather than loop-for-sets. When I started testing
> other cases I was accidentally using the pre-patch branch to do so.
>
> I can't think of an easy solution to cover both problems right now. If
> no one better suited can figure this out, I'll come back to this after
> completing an ert suite for cl-loop (of which I'm part-way through).

Alex, this was three years ago.  Did you make any progress here?  :-)

    (loop for elem in '(1 2 3)
          for k = elem and j = 99
          do
          (print k))

still displays 1 1 2, which has to be wrong...

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