LOOP WITH can't destructure short lists/NIL

[3b]

 (loop with (a b) = (list) repeat 1 collect (list a b)) ;; using (list) here since plain NIL apparently gets optimized away

gives the following error:
error while parsing arguments to DESTRUCTURING-BIND:
  invalid number of elements in
    ()
  to satisfy lambda list
    (A B &REST #:LOOP-IGNORED-VAR-0):
  at least 2 expected, but 0 found
   [Condition of type SB-KERNEL::ARG-COUNT-ERROR]

Expected results: ((NIL NIL))

tested on sbcl "1.0.44.36", x8664 linux

[Roman Marynchak (roman-marynchak)]

Seems to be the real bug in our LOOP, marking it as confirmed. I hope to look into this one in the near future.

[Roman Marynchak (roman-marynchak)]

I have found the corresponding CLHS section - 6.1.1.7. It says: "Destructuring allows binding of a set of variables to a corresponding set of values anywhere that a value can normally be bound to a single variable. During loop expansion, each variable in the variable list is matched with the values in the values list. If there are more variables in the variable list than there are values in the values list, the remaining variables are given a value of nil. If there are more values than variables listed, the extra values are discarded".

Our LOOP expansion uses DESTRUCTURING-BIND directly through LOOP-DESTRUCTURING-BIND, so the problem occurs (because DESTRUCTURING-BIND does not allow the pattern mismatch). Consider this macroexpansion:

* (macroexpand '(loop with (a b) = (list)))

(BLOCK NIL
  (LET ((#:LOOP-DESTRUCTURE-608 (LIST)))
    (SB-LOOP::LOOP-DESTRUCTURING-BIND (A B) #:LOOP-DESTRUCTURE-608
                                      (SB-LOOP::LOOP-BODY NIL NIL NIL NIL
                                                          NIL))))

SB-LOOP::LOOP-DESTRUCTURING-BIND code is below:

(sb!int:defmacro-mundanely loop-destructuring-bind
    (lambda-list arg-list &rest body)
  (let ((*ignores* nil))
    (declare (special *ignores*))
    (let ((d-var-lambda-list (subst-gensyms-for-nil lambda-list)))
      `(destructuring-bind ,d-var-lambda-list
           ,arg-list
         (declare (ignore ,@*ignores*))
         ,@body))))

IMHO, LOOP-DESTRUCTURING-BIND macro is a place where the fix should be implemented.

Also, note that CCL has this bug too (and CLISP does not).

[Roman Marynchak (roman-marynchak)]

Here is the solution which fixes the bug and passes all regression tests too.

I cannot call it brilliant, but in case we want to use DESTRUCTURING-BIND, there is no other way - we need to adjust the actual values list to match the pattern. So the list is extended with NILs to have the same length as the pattern has.

In case a pattern is a dotted list, CLHS does not give the clear advice about the pattern mismatch, so we do not try to handle the code like (LOOP WITH (A . B) = (LIST) ...) at this time.

I didn't prepare the actual commit-based patch, because I feel that there will be some critical review of the solution, and it will be improved :) Comments/advices are welcome, as usual.

[Paul Khuong (pvk)]

I don't think that's the best way to go about this. I would either try and reuse loop-really-desetq, or something like

(defun emit-loop-destructuring-bind (pattern value body)
  (let ((bindings '())
        (rebindings '())
        (variable (gensym "VAR")))
    (labels ((walk (pattern value &aux (var (gensym "TEMP")))
               (when pattern
                 (push `(,var ,value) bindings)
                 (cond ((consp pattern)
                        (walk (car pattern) `(car ,var))
                        (walk (cdr pattern) `(cdr ,var)))
                       (t
                        (push `(,pattern ,var) rebindings))))
               nil))
      (walk pattern variable)
      `(let* ((,variable ,value)
              ,@(nreverse bindings))
         (let ,(nreverse rebindings)
           ,@body)))))

On 2011-01-02, at 2:07 PM, Roman Marynchak wrote:

> Here is the solution which fixes the bug and passes all regression tests
> too.
>
> I cannot call it brilliant, but in case we want to use DESTRUCTURING-
> BIND, there is no other way - we need to adjust the actual values list
> to match the pattern. So the list is extended with NILs to have the same
> length as the pattern has.
>
> In case a pattern is a dotted list, CLHS does not give the clear advice
> about the pattern mismatch, so we do not try to handle the code like
> (LOOP WITH (A . B) = (LIST) ...) at this time.
>
> I didn't prepare the actual commit-based patch, because I feel that
> there will be some critical review of the solution, and it will be
> improved :) Comments/advices are welcome, as usual.
>
>
> ** Patch added: "695286.patch"
> https://bugs.launchpad.net/sbcl/+bug/695286/+attachment/1781806/+files/695286.patch
>
> ** Tags added: review
>
> ** Changed in: sbcl
> Assignee: Roman Marynchak (roman-marynchak) => (unassigned)
>
> ** Changed in: sbcl
> Status: In Progress => Confirmed
>
> --
> You received this bug notification because you are a member of SBCL
> hackers, which is subscribed to SBCL.
> https://bugs.launchpad.net/bugs/695286
>
> Title:
> LOOP WITH can't destructure short lists/NIL
>
> Status in Steel Bank Common Lisp:
> Confirmed
>
> Bug description:
> (loop with (a b) = (list) repeat 1 collect (list a b)) ;; using (list) here since plain NIL apparently gets optimized away
>
> gives the following error:
> error while parsing arguments to DESTRUCTURING-BIND:
> invalid number of elements in
> ()
> to satisfy lambda list
> (A B &REST #:LOOP-IGNORED-VAR-0):
> at least 2 expected, but 0 found
> [Condition of type SB-KERNEL::ARG-COUNT-ERROR]
>
>
> Expected results: ((NIL NIL))
>
> tested on sbcl "1.0.44.36", x8664 linux
>
>

[Roman Marynchak (roman-marynchak)]

Paul, thank you for the review! I will try to implement a better solution.