LOOP WITH can't destructure short lists/NIL
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| SBCL |
Low
|
Unassigned |
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-
at least 2 expected, but 0 found
[Condition of type SB-KERNEL:
Expected results: ((NIL NIL))
tested on sbcl "1.0.44.36", x8664 linux
Roman Marynchak (roman-marynchak) wrote : | #2 |
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-DESTRUCTUR
* (macroexpand '(loop with (a b) = (list)))
(BLOCK NIL
(LET ((#:LOOP-
(SB-
SB-LOOP:
(sb!int:
(lambda-list arg-list &rest body)
(let ((*ignores* nil))
(declare (special *ignores*))
(let ((d-var-lambda-list (subst-
`
(declare (ignore ,@*ignores*))
,@body))))
IMHO, LOOP-DESTRUCTUR
Also, note that CCL has this bug too (and CLISP does not).
Changed in sbcl: | |
assignee: | nobody → Roman Marynchak (roman-marynchak) |
status: | Confirmed → In Progress |
Roman Marynchak (roman-marynchak) wrote : | #3 |
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.
tags: | added: review |
Changed in sbcl: | |
assignee: | Roman Marynchak (roman-marynchak) → nobody |
status: | In Progress → Confirmed |
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-
(let ((bindings '())
(rebindings '())
(variable (gensym "VAR")))
(labels ((walk (pattern value &aux (var (gensym "TEMP")))
(walk pattern variable)
`(let* ((,variable ,value)
(let ,(nreverse rebindings)
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:/
>
> ** 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:/
>
> 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-
> at least 2 expected, but 0 found
> [Condition of type SB-KERNEL:
>
>
> Expected results: ((NIL NIL))
>
> tested on sbcl "1.0.44.36", x8664 linux
>
>
Roman Marynchak (roman-marynchak) wrote : | #5 |
Paul, thank you for the review! I will try to implement a better solution.
Changed in sbcl: | |
assignee: | nobody → Roman Marynchak (roman-marynchak) |
status: | Confirmed → In Progress |
tags: | removed: review |
Changed in sbcl: | |
importance: | Undecided → Low |
Changed in sbcl: | |
assignee: | Roman Marynchak (roman-marynchak) → nobody |
status: | In Progress → Triaged |
Stas Boukarev (stassats) wrote : | #6 |
In c42076a971dab88
Changed in sbcl: | |
status: | Triaged → Fix Committed |
Changed in sbcl: | |
status: | Fix Committed → Fix Released |
Seems to be the real bug in our LOOP, marking it as confirmed. I hope to look into this one in the near future.