once-only lisp macro, is my implementation correct?

Question

I am trying to learn Lisp from Peter Seibel's book "Practical Common Lisp". In chapter 8 : "Macros: Defining your own", I came across this once-only macro. At the bottom of that page, an implementation is given. Now that is a pretty complicated macro to understand for me, so I saw this question on stackoverflow and there are some good explanations there.

However, even if I (still) havent fully understood the macro, I understood its purpose. So I tried to write my own implementation of it :

(defmacro my-once-only ((&rest names) &body body)
  (let
    (
      (gensyms (loop for n in names collect (gensym)))
    )

    `(list 'let
      (list ,@(loop for n in names for g in gensyms collect `(list ',g ,n)))

      (let
        ,(loop for n in names for g in gensyms collect `(,n ',g))

        ,@body))))

(Please forgive me if I am not following standard lisp conventions of indentation, I was trying to indent the code in a way so I can understand what goes where, since I am new to this)

I tested this macro much the same way as is described in that chapter I linked, ie. calling the function with arguments like (random 100) so that if they are evaluated twice, results will be wrong. I also expanded my macro (and the macro I used it in) via macroexpand/macroexpand-1 and that seems to be correct too.

So I was wondering if my implementation is correct ? or is there something I am missing (which is likely I think)...

Answer 1

Let's actually macroexpand the two implementations and see how they differ:

* (macroexpand '(once-only (foo bar) (+ foo bar)))
(LET ((#:G619 (GENSYM)) (#:G620 (GENSYM)))
  `(LET ((,#:G619 ,FOO) (,#:G620 ,BAR))
     ,(LET ((FOO #:G619) (BAR #:G620))
        (+ FOO BAR))))

* (macroexpand '(my-once-only (foo bar) (+ foo bar)))
(LIST 'LET (LIST (LIST '#:G621 FOO) (LIST '#:G622 BAR))
      (LET ((FOO '#:G621) (BAR '#:G622))
        (+ FOO BAR)))

Let's rewrite your macroexpansion to something easier for a Lisper to read:

`(LET ((#:G621 ,FOO) (#:G622 ,BAR))
   ,(LET ((FOO '#:G621) (BAR '#:G622))
      (+ FOO BAR)))

Notice that your version lacks the indirection with the additional gensym. That means each invocation of your outer macro (the one that is using my-once-only) is using the same symbols each time. If your macro calls nest (e.g., you use your outer macro inside the body of another use of the outer macro), the symbols will collide.