Re: [Gcl-devel] Lisp programming style advice

[Camm Maguire]

Greetings!  Thanks, as always, Paul.

Here is what I'm committing now.  I've implemented everything save the
flet suggestion.  I cannot seem to get the ,name recognized as a valid
defined function in the (return ) calls when using flet.

=============================================================================
(defmacro with-package-iterator ((name plist &rest symbol-types) . body)
  (let ((p (gensym)) (i (gensym)) (l (gensym)) (q (gensym)) (dum (gensym))
        (x (gensym))(y (gensym)) (access (gensym)) declaration)
    (multiple-value-setq (declaration body) (si::find-declarations body))
    (if (null symbol-types)
        (specific-error :too-few-arguments "Symbol type specifiers must be 
supplied"))
    `(let ((,p (cons t (if (atom ,plist) (list ,plist) ,plist))) (,q nil) (,l 
nil)
           (,i -1) (,x 0) (,y 0))
       (macrolet ((,name () 
                     '(block ,name
                            (when (null (setq ,l (cdr ,l)))
                              (when (eql (incf ,i) (+ ,x ,y))
                                (when (null (setq ,q (cdr ,q))) 
                                  (when (null (setq ,p (cdr ,p)))
                                    (return-from ,name nil))
                                  (rplaca ,p (coerce-to-package (car ,p)))
                                  (setq ,q (list 
                                            (si::coerce-to-package (car ,p))))
                                  (when (member :inherited (list 
,@symbol-types))
                                    (rplacd ,q (package-use-list (car ,q)))))
                                (multiple-value-setq (,y ,x) (si::package-size 
(car ,q)))
                                (when (or (not (member :internal (list 
,@symbol-types)))
                                          (not (eq (car ,p) (car ,q))))
                                  (setq ,x 0))
                                (when (and (not (member :external (list 
,@symbol-types)))
                                           (eq (car ,p) (car ,q)))
                                  (setq ,y 0))
                                (when (zerop (+ ,x ,y)) 
                                  (setq ,i -1)
                                  (return-from ,name (,name)))
                                (setq ,i 0))
                              (setq ,l (if (< ,i ,x)
                                           (si::package-internal (car ,q) ,i)
                                         (si::package-external (car ,q) (- ,i 
,x)))))
                            (when (null ,l)
                              (return-from ,name (,name)))
                            (multiple-value-setq (,dum ,access) 
                                                 (find-symbol 
                                                  (symbol-name (car ,l)) (car 
,p)))
                            (when (and (not (eq ,access :inherited)) 
                                       (not (eq (car ,p) (car ,q))))
                              (return-from ,name (,name)))
                            (values 't (car ,l) ,access (car ,p)))))
                 (declare (fixnum ,x ,y))
                 ,@declaration
                 ,@body))))
=============================================================================


"Paul F. Dietz" <address@hidden> writes:

> Camm Maguire wrote:
> > Greetings!  In the course of addressing some of the ansi issues
> > revealed by Paul's tests, I'm finding myself writing ever more
> > sophisticated (for me) lisp code for GCL with a still partial
> > understanding of the language. I'd like to take a moment to solicit
> > the opinions of the list on the with-package-iterator macro I
> > committed recently.
> 
> (1) Don't write (if <form1> <form2>); write (when <form1> <form2>).
>    This is easier to understand when <form2> is very large.
>    Similarly, prefer (unless <form1> <form2>) to (if (not <form1>) <form2>).
>    The reader should not be forced to scan down the page to search
>    for the optional 'else' clause.
> 
> 
> (2) Instead of
> 
> (progn
>     (setq ,q (cdr ,q))
>     (if (null ,q)
>       ... ))
> 
> write
> 
> (if (null (setq ,q (cdr ,q)))
>     ...)
> 
> This reduces the indentation overload.
> 
> 
> (3) You probably don't want a big body in a macrolet -- that can lead
>    to lots of code expansion.  Instead, use a flet.
> 
> 
> (4) Instead of (setq ,i (1+ ,i)), use (incf ,i).
> 
> 
> And a bug:
> 
> (5) You are assigning a list to ,x at one point, but you declared ,x
>    to be a fixnum.  Use multiple-value-setq instead.
> 
> (setq ,x (multiple-value-list
>         (si::package-size (car ,q))))
> (setq ,y (first ,x))
> (setq ,x (second ,x))
> ==>
> (multiple-value-setq (,y ,x) (si::package-size (car ,q)))
> 
> (You don't want multiple-value-list to be consing up that short
> temporary list anyway.)
> 
>       Paul
> 
> 
> 

-- 
Camm Maguire                                            address@hidden
==========================================================================
"The earth is but one country, and mankind its citizens."  --  Baha'u'llah