Common Lisp macro: review for code quality & leaky name escaping

Question

(defmacro with-condition-retries(retries expected-errors fail-function &body body)
  "Attempts to execute `body` `retries` times, watching for
`expected-errors`.  Optionally, if `fail-function` is set, each time a
failure occurs, prior to retrying, `fail-function` is executed.

Other conditions beside `expected-errors` will exit out of this macro"
   (let ((counter (gensym))
     (result (gensym))
     (start-tag (gensym))
     (error-handler         
      #'(lambda (c)
        (let ((r (find-restart 'handler c)))
          (when r 
        (invoke-restart r c))))))

     `(let ((,counter 0)
        (,result))
        (tagbody
           ,start-tag
           (restart-case
           (handler-bind
            ,(mapcar #'(lambda (error-val)
                     (list error-val   error-handler))
                 expected-errors)
             (setf ,result ,@body))
         ;;the restart pointed at by the error-handler lambda
         (handler (&rest args)
           (declare (ignore args))
           (incf ,counter)
           (unless (> ,counter ,retries)
             (when ,fail-function
               (funcall ,fail-function))
             (go ,start-tag)))))
    ,result)))

Answer 1

This is the sort of thing you want to get as many eyes on as possible. My gut tells me that you'd want to break it up into smaller functions, but I'm not entirely sure where I'd cut. Any chance of getting an example use or test case? Could you explain the reasoning behind using tagbody with a single go rather than a loop?

Preliminary stuff I've noticed

---

with-gensyms can save you a few lines at the beginning, and you should probably define it in a macro-heavy package.

(defmacro with-gensyms (syms &body body)
  `(let ,(loop for s in syms collect `(,s (gensym)))
     ,@body))

---

The expression (setf ,result ,@body) might do some odd things if body is composed of multiple expressions. That should probably be (setf ,result (progn ,@body)) instead.

Answer 2

You may try breaking this task up into two smaller tasks.

1. A macro that tries to execute &body, watching for errors. Calls a fail function if defined. Returns information related to error, etc., and
2. A macro that executes 1 n times.

Also, since you are watching out for unintentional variable capture, you may want to look at the defmacro! abstraction. I find the ,g! syntax even more direct than with-gensyms.