loop through forms in a macro

[White_Owl]

The task:

Time is a macro that can be wrapped around any Lisp call to time how long it takes. It will print out various statistics: (time (sleep 2))
Write a macro (time-many &body forms) that is like TIME but takes multiple forms as arguments. For each argument, it first prints the form being timed and then the stats (by calling TIME itself). (time-many (sin 20.0) (sleep 1) (dotimes (i 100000000) (sqrt i)))

My solution:

Code: Select all

(defmacro time-many (&body forms)
  (let ((form (gensym)))
    `(dolist (,form ,forms)
       (format "Timing: ~a~%" ,form)
       (time ,form))))

But when I run it:

Code: Select all

[2]> (time-many (sin 20.0) (sleep 1) (dotimes (i 100000000) (sqrt i)))
*** - EVAL: (SIN 20.0) is not a function name; try using a symbol instead

What am I doing wrong???

[nuntius]

A couple tips when working on macros: First, print out the expansion, either using macroexpand or by injecting print statements into the body. Second, try splitting the macro into a "syntax wrapper" that calls a normal function to manipulate the form. This helps with visibility into expansion and makes things easier to debug.

The issue you are seeing is a subtle quoting bug. In the dolist, you need to protect the body list from evaluation as a function call. Hence the error "(SIN 20.0) is not a function name". It is literally trying to run the function named "(SIN 20.0)".

You also forgot to set the format destination. I specified t (standard output) below.

Code: Select all

(defmacro time-many (&body forms)
  (let ((form (gensym)))
    `(dolist (,form ',forms)
       (format t "Timing: ~a~%" ,form)
       (time ,form))))

One bug remains. The format line prints the form without evaluating it. The "(time ,form)" line needs to actually evaluate the form.

The simple fix is to call eval.

A bigger change is to perform the dolist during the macroexpansion. This approach could do the string formatting at compile time and also eliminate the gensym.

[sylwester]

The macro gets the code unevaluated and you should use that when making the output, but the forms also needs to be in the resulting code, wrapped in time.
Here is one way to do it:

Code: Select all

(defmacro time-many (&body forms)
  `(progn ,@(loop :for form :in forms
                  :collect `(princ ,(format nil "Timing: ~a" form))
                  :collect `(time ,form))))

Testing that it does what you want:

Code: Select all

(macroexpand '(time-many (sin 20.0) (sleep 1) (dotimes (i 100000000) (sqrt i))))
==>
(progn 
  (princ "Timing: (sin 20.0)") 
  (time (sin 20.0)) 
  (princ "Timing: (sleep 1)")
  (time (sleep 1)) 
  (princ "Timing: (dotimes (i 100000000) (sqrt i))")
  (time (dotimes (i 100000000) (sqrt i))))

[White_Owl]

Thank you guys.
I think, I understand it now a little better...