Permalink
Browse files

corrected a typo, and changed the implementation of extract declarati…

…ons from using cl:do to using cl:loop
  • Loading branch information...
alaa-alawi committed May 1, 2012
1 parent 3a031d7 commit 835fabdfbb7a6b982db2b741375e6bf9d409df78
Showing with 9 additions and 7 deletions.
  1. +9 −7 util.lisp
View
@@ -229,12 +229,14 @@ character set."
(escape-string string :test #'non-7bit-ascii-escape-char-p))
(defun extract-declarations (body)
- "Given a FORM, the declarations - if any - will be exctracted
+ "Given a FORM, the declarations - if any - will be extracted
from the head of the FORM, and will return two values the declarations,
and the remaining of FORM"
- (do ((sexp (first body) (first forms))
- (forms (rest body) (rest forms))
- (declarations nil))
- ((not (eq (first sexp) 'cl:declare))
- (values declarations (append (if (null sexp) sexp (list sexp)) forms)))
- (push sexp declarations)))
+ (loop for sexp = (first body) then (first forms)
+ for forms = (rest body) then (rest forms)
+ for declarations = nil
+ if (not (eq (first sexp) 'cl:declare))
+ do (return (values declarations
+ (append (if (null sexp) sexp (list sexp)) forms)))
+ else
+ do (push sexp declarations)))

5 comments on commit 835fabd

@hanshuebner

This comment has been minimized.

Show comment Hide comment
@hanshuebner

hanshuebner May 1, 2012

There is nothing wrong with DO except that it is hard to read, but of course, your directly translated loop is not that much easier to read. Sorry. I have proposed to use Alexandria because it has a function that does precisely what you want to do, and in the age of Quicklisp, there is no reason to not use other libraries.

Here is a loop that I'd like:

(defun extract-declarations (forms)
(loop with declarations
for forms on forms
for form = (first forms)
while (eql (first form) 'cl:declare)
do (push form declarations)
finally (return (values (nreverse declarations) forms))))

There is nothing wrong with DO except that it is hard to read, but of course, your directly translated loop is not that much easier to read. Sorry. I have proposed to use Alexandria because it has a function that does precisely what you want to do, and in the age of Quicklisp, there is no reason to not use other libraries.

Here is a loop that I'd like:

(defun extract-declarations (forms)
(loop with declarations
for forms on forms
for form = (first forms)
while (eql (first form) 'cl:declare)
do (push form declarations)
finally (return (values (nreverse declarations) forms))))

@alaa-alawi

This comment has been minimized.

Show comment Hide comment
@alaa-alawi

alaa-alawi May 1, 2012

Owner

This works with the note that the last form in the following samples (although many are only for testing purposes - hypothetical) return as a second value (nil), rather than nil. which may case problem upon expansion while in macro.

(extract-declarations '((declare (special x)) (list 1 2 3 5)))
(extract-declarations '((declare (special x)) (declare (fixnum x)) (list 1 2 3 5)))
(extract-declarations '((declare (special x)) (list 1 2 3 5) (declare (fixnum x))))
(extract-declarations '((list 1 2 3 5) (declare (fixnum x))))
(extract-declarations '((declare (special x)) (list 1 2 3 5)))
(extract-declarations '((list 1 2 3 5)))
(extract-declarations '(()))

I'm fine with your implementation, even using Alexandria (although I do not support it). my goal is to get published declarations to work in cl-who macros.

one more side question: in my second variation using LOOP, which part was not clear? was it the termination part? the accumulation part? the stepping part? or all of them?

Kindly point to the one that works best with the long term goals of cl-who library. and I'll push them my fork.

Owner

alaa-alawi replied May 1, 2012

This works with the note that the last form in the following samples (although many are only for testing purposes - hypothetical) return as a second value (nil), rather than nil. which may case problem upon expansion while in macro.

(extract-declarations '((declare (special x)) (list 1 2 3 5)))
(extract-declarations '((declare (special x)) (declare (fixnum x)) (list 1 2 3 5)))
(extract-declarations '((declare (special x)) (list 1 2 3 5) (declare (fixnum x))))
(extract-declarations '((list 1 2 3 5) (declare (fixnum x))))
(extract-declarations '((declare (special x)) (list 1 2 3 5)))
(extract-declarations '((list 1 2 3 5)))
(extract-declarations '(()))

I'm fine with your implementation, even using Alexandria (although I do not support it). my goal is to get published declarations to work in cl-who macros.

one more side question: in my second variation using LOOP, which part was not clear? was it the termination part? the accumulation part? the stepping part? or all of them?

Kindly point to the one that works best with the long term goals of cl-who library. and I'll push them my fork.

@hanshuebner

This comment has been minimized.

Show comment Hide comment
@hanshuebner

hanshuebner May 1, 2012

@alaa-alawi

This comment has been minimized.

Show comment Hide comment
@alaa-alawi

alaa-alawi May 1, 2012

Owner

I shall push your LOOP based implementation soon.

For the NIL thing. I meant that if any value of the the two values returned are nil, then this means that part (declarations or remaining forms) was not found.

Owner

alaa-alawi replied May 1, 2012

I shall push your LOOP based implementation soon.

For the NIL thing. I meant that if any value of the the two values returned are nil, then this means that part (declarations or remaining forms) was not found.

@hanshuebner

This comment has been minimized.

Show comment Hide comment
@hanshuebner

hanshuebner May 1, 2012

Please sign in to comment.