#375 loop (...) for vars on vars misbehaving

[Tomas Zellerin]

Following construct behaves different than expected in
clisp (2.40 and 2.41, Linux and Windows tried):

(let ((vars '(1 2 3 4)))
(loop for i from 0 to 10 for vars on vars do (print
vars)))
=> nil

The expansion (macroexpand-1 '(loop for i from 0 to
10 for vars on vars do (print vars))) shows the
reason.

(MACROLET ((LOOP-FINISH NIL (SYSTEM::LOOP-FINISH-
ERROR)))
(BLOCK NIL
(LET ((I 0))
(PROGN
(LET ((VARS NIL)) ; <-------- here
(LET NIL
(LET NIL
(MACROLET ((LOOP-FINISH NIL '(GO SYSTEM::END-
LOOP)))
(TAGBODY (SETQ VARS VARS) SYSTEM::BEGIN-LOOP
(WHEN (> I 10) (LOOP-FINISH)) (WHEN (ATOM
VARS) (LOOP-FINISH))
(PROGN (PROGN (PRINT VARS))) (PSETQ I (+ I
1)) (PSETQ VARS (CDR VARS))
(GO SYSTEM::BEGIN-LOOP) SYSTEM::END-LOOP
(MACROLET
((LOOP-FINISH NIL (SYSTEM::LOOP-FINISH-WARN)
'(GO SYSTEM::END-LOOP)))))))))))))

See discussion on
http://thread.gmane.org/gmane.lisp.clisp.general/11568
/focus=11568

[Jörg Höhle]

Logged In: YES
user_id=377168

Bug was not present in clisp-2.28
(macroexpand'
(loop for i from 0 to 10 for vars on vars do (print
vars)))

(MACROLET ((LOOP-FINISH NIL (SYSTEM::LOOP-FINISH-ERROR)))
(BLOCK NIL
(LET NIL
(LET ((I 0))
(PROGN
(LET ((#:G232 NIL))
(MACROLET ((LOOP-FINISH NIL '(GO SYSTEM::END-LOOP)))
(TAGBODY (PROGN (SETQ #:G232 VARS))
SYSTEM::BEGIN-LOOP
(PROGN (WHEN (> I 10) (LOOP-FINISH)))
(PROGN (WHEN (ENDP #:G232) (LOOP-FINISH))
(LET ((VARS #:G232)) (PROGN (PROGN (PRINT VARS)))))
(PROGN (PSETQ I (+ I 1)) (PSETQ #:G232 (CDR
#:G232)))
(GO SYSTEM::BEGIN-LOOP) SYSTEM::END-LOOP
(MACROLET
((LOOP-FINISH NIL (SYSTEM::LOOP-FINISH-WARN)
'(GO SYSTEM::END-LOOP)))))))))))) ;

I'm really surprised that such a bug can get in. I'd
expected ANSI tests to catch such shadowing issues. When
were these last ran?
Maybe that's another TODO item for the pre-release process?

It appears that 2.28's loop is radically different: the vars
variable is bound anew for every iteration, not bound

[Jörg Höhle]

Logged In: YES
user_id=377168

Not a bug.
CLHS 6.1.1.4 states "One implication of this interleaving is
that it is implementation-dependent whether the lexical
environment in which the initial value forms (variously
called the form1, form2, form3, step-fun, vector,
hash-table, and package) in any for-as-subclause, except
for-as-equals-then, are evaluated includes only the loop
variables preceding that form or includes more or all of the
loop variables; the form1 and form2 in a for-as-equals-then
form includes the lexical environment of all the loop
variables."

Therefore, no wonder no tests detects this change, since
it's not portable code.

Thus the only bug is that CLISP is encouraged to and should
document its "implementation dependent" choices (see CLHS
glossary).
Once this is documented, the testsuite may verify CLISP's
specific choice.

Given that CLISP's implementation changed over time, maybe
imnotes should instead document exactly that: do *not* rely
on it, its unportable anyway

[Sam Steingold]

Logged In: YES
user_id=5735

I am not sure I agree with Jorg.
if this code works in ALL other CL implementations
(including an older CLISP), it is de facto portable.
there is no good reason for CLISP not to accept it
(especially since it already does the expected thing
without the "for i from ..." part).

[Sam Steingold]

I fixed this at the cost of an extra bytecode instruction per variable per loop.
note that (loop for x = x then (1+ x)) still does not work (and sbcl does not support it either).
https://sourceforge.net/p/clisp/clisp/ci/52d209b7d29ddf1125d2d89f4185c20c2f09da12/

[Jörg Höhle]

6.1.1.4 Expanding Loop Forms http://clhs.lisp.se/Body/06_aad.htm
"the form1 and form2 in a for-as-equals-then form includes the lexical environment of all the loop variables."
Hence the second x in for x = x clearly cannot refer to an outside environment, in ANSI-CL.

This exception causes for-as-equals-then to be a strange citizen, IMHO. How can you maintain global left to right evaluation when the lexical environments differ?

[Bruno Haible]

> How can you maintain global left to right evaluation when the lexical environments differ?

Evaluation order and lexical environments/scope are orthogonal issues in general. In the compiler, when compiling a form, we can do that in any lexical environment we want. And in a macroexpander, you can manipulate evaluation order of form1, form2 by storing (lambda () form1) and (lambda () form2) and invoking these functions when you like.

But you are right that it is counter-intuitive and therefore a usability problem of the LOOP macro: Some forms are evaluated in the outer environments, some forms in the environment where the loop variables are visible - yet these forms feel like being of equal rank/level/position. That's how ANSI CL specifies LOOP...

[Jörg Höhle]

note that (loop for x = x then (1+ x)) still does not work

Maybe clisp should bail out in all such cases, instead of having half the initialisation forms evaluated in the outside environment and the other half in an environment encompassing all loop variables?

The bug is about this CLHS issue: Issue LOOP-INITFORM-ENVIRONMENT: VAGUE
http://clhs.lisp.se/Issues/iss222.htm
Please update doc/impissue.xml if clisp decides to follow the (reportedly much debated) recommendation.

I think it's actually a very good recommendation from a user POV, albeit hard to implement given all the variations (and package iterators etc.).

[Bruno Haible]

> Maybe clisp should bail out in all such cases

CLHS allows the implementation to handle this case one way or the other (LET or SETQ). But the implementation is not allowed to produce an error here, if x is defined in an outer scope.

[Jörg Höhle]

Regarding for-as-equals-then and section 6.1.1.4, I'm more and more convinced that there should be an amendment to ANSI-CL. It should distinguish for-as-equals-then from for-as-equals!

• for-as-equals Clearly, the environment must encompass all variables, as the code ("form1") can refer to values from a previous iteration (depending on the order of for-clauses).
• for-as-equals-then Here the change to ANSI-CL wording should be that only form2 is required to be evaluated in a lexical environment encompassing all variables. But form1 should be evaluated - like the initial value that it truly is - in the same way as in other for-clauses, i.e. before iteration.

As "it is implementation-dependent whether the lexical environment ... includes only the loop variables preceding that form ..." we may get out of that conundrum:

a) We provide reasonable semantics to (loop for x in/on/across x ...) that is: x gets its value from an outer binding, not NIL. We document that, conforming to ANSI-CL.
b) We document that we agree to disagree with ANSI-CL and that we prefer to implement Issue 222
http://clhs.lisp.se/Issues/iss222_w.htm such that (loop for list = list then (cdr list) receives reasonable semantics that one can rely upon (like DO/DO*/DOLIST).

Regarding Sam's patch, I think I'm going to revert it locally and see if I can fix the old code to do what I want (one key part are the german comments above note-initialization in loop.lsp). But I've not yet understood the complexity and justification for the many state variables within the defstruct).

[Sam Steingold]

I reverted my patch locally too ;-(
I am not sure I can agree with your ANSI-CL amendment though.

[Bruno Haible]

I agree that in most cases of for-as-equals-then with a then clause the user will want form1 to refer to the outer environment and form2 to an environment that includes the loop variables.

However, 6.1.1.4 is very clear about the lexical environment of form1.

Therefore, the proper course of action is to

1. Make a writeup on http://cliki.net/ANSI%20Clarifications%20and%20Errata
2. Seek agreement among the major implementations.
3. Then only modify the way clisp works.

[Jörg Höhle]

Reverting was TRT. The old clisp code already has all the pieces to evaluate initialisation forms in an outer environment. It just needs to assemble the pieces differently, in some (yet unknown exactly) circumstances. The difference can be seen in
(macroexpand-1'(loop for x across X for y across Y)) and
(macroexpand-1'(loop repeat (random 2) repeat (random 5)))
The first clause nicely uses LET, the second introduces SETQ and completely changes the scope.

[Sam Steingold]

the scope change is controlled by requires-stepbefore

[Sam Steingold]

see https://sourceforge.net/p/clisp/bugs/667/#2b06
and https://sourceforge.net/p/clisp/mailman/message/36162784/

[Bruno Haible]

Your testcase now generates a warning that tells you about the problem:

> (let ((vars '(1 2 3 4)))
(loop for i from 0 to 10 for vars on vars do (print vars)))
WARNING: Reference to VARS is implementation-dependent, per ANSI CL 6.1.1.4.
NIL