[Openmcl-devel] LOOP parallel for/as termination eval order issue.

[Kaz Kylheku]

On Mon, 18 Oct 2010 00:27:43 +0400, Stas Boukarev <stassats at gmail.com>
wrote:
> Kaz Kylheku <kaz at kylheku.com> writes:
> 
>> Hi all,
>>
>> The following for returns nil in CCL and Allegro, but (1 2 3) in Clisp.
>>
>> (loop for x in nil and y = '(1 2 3) then (cdr y) finally (return y))
>>
>> My expectation is that the CLISP behavior is right, because
>> X and Y ought to be initialized in parallel to NIL and (1 2 3)
>> respectively. Only then should the loop termination test kick in
>> due to X having run out of items. The FINALLY clause must not
>> see a nil value of Y.
>> When major implementations get stuff like this wrong, no wonder
>> Lispers eschew LOOP (or include their own LOOP macro in the
>> code base).
> I find this test-case to be contrived.

Hi Stas,

I find that I have a problem with the suggestion
that contrived code should not expect correct
behavior from high level language features. This
hasn't been valid since they days programmers
by and large abandoned macro assemblers.

"Contrived" has to do not with what the code is, but how
it originated, which is largely irrelevant.

The origin in this case is that I took the nonworking
loop from an actual program and
rendered it into a minimal test case which reproduces
the behavior. This is industry-standard reporting practice.
(Sometimes bug submitters are even chided for not
posting minimal test cases).

If you're interested, here is the actual code
(with workaround already applied):

    (when eliminate-recent
       (loop for old-response in response-history and
             reduced-matches = match-text then new-matches
             for new-matches = (if (stringp old-response)
                                 (remove old-response
                                         reduced-matches
                                         :test match-function)
                                 reduced-matches)
             until (null new-matches)
             finally (setf match-text (or new-matches
                                          reduced-matches
                                          match-text))))

match-text contains a list of possible correct answers.
response-history is a list of answers given for the same
question (in reverse chronological order: answers are
pushed onto it).

The idea is to eliminate all the most recently
given answers, but leave at least one, so that when the
question comes up multiple times during testing, the student
will cycle through all of the answers.

Consider what happens if the response-history is empty.
It appears that CCL's loop doesn't peform the other
initializations. The finally clause executes, whereby
reduced-matches and new-matches are found to be nil.
The result is that match-text is empty, causing there
to be no correct answers, and thus any answer to be
marked wrong.

The workaround for the broken loop is the extra branch
of the OR in the finally clause. The original code
has (or new-matches reduced-matches) because it
does not expect a situation where both of these are
null. Why this OR? Because new-matches can become empty.
This means that the response history contains all possible
answers. We do not want to eliminate all answers, but
rather all the answers except the one that was least
recently given. So if we happen to eliminate everything,
we backtrack to reduced-matches, which is is the prior
iteration's value of new-matches, which should then
contain exactly the least recently used item.

The program has another LOOP that is breaking under CCL,
in related code, also having to do with processing
eliminated answers.  I cannot see how that one is breaking
and don't have an obvious workaround. The weekend drawing
to a close, I won't have an opportunity to wrap my head around
it for probably several days.

Cheers.