Vicious metacircle in computing effective method of function called from macroexpand-hook

[Paul F. Dietz]

(defgeneric f2 (form-car fn form env))

(defmethod f2 (form-car fn form env)
  (funcall fn form env))

(defmethod f2 ((form-car (eql 'defun)) fn form env)
  (format t "Boo!!%")
  (call-next-method))

(defun f1 (fn form env)
  (if (consp form)
      (f2 (car form) fn form env)
      (funcall fn form env)))

(defun g ()
  (let ((*macroexpand-hook* #'f1))
    (eval '(defun h () nil))))

(g) ==>

; in: LAMBDA ()
; (LAMBDA (SB-PCL::|.P0.|)
; (DECLARE (OPTIMIZE (SPEED 3) (SAFETY 0) (INHIBIT-WARNINGS 3) (DEBUG 0)))
; (BLOCK NIL
; (WHEN
; (SB-KERNEL:LAYOUT-INVALID
; #<SB-KERNEL:LAYOUT for SWANK/GRAY::SLIME-OUTPUT-STREAM {50483483}>)
; (SB-PCL::INSTALL-INITIAL-CONSTRUCTOR
; #<SB-PCL::CTOR SWANK/GRAY::SLIME-OUTPUT-STREAM #>)
; (RETURN (FUNCALL # SB-PCL::|.P0.|)))
; (LET ((SB-PCL::.INSTANCE. #) (SB-PCL::.SLOTS. #))
; (SETF # #)
; (SETF # SB-PCL::.SLOTS.)
; (LET ()
; (DECLARE #)
; (FLET #
; #
; #))
; SB-PCL::.INSTANCE.)))
;
; caught ERROR:
; during macroexpansion of (LAMBDA (SB-PCL::|.P0.|) (DECLARE #) ...). Use
; *BREAK-ON-SIGNALS* to intercept.
;
; vicious metacircle: The computation of an effective method of
; #<STANDARD-GENERIC-FUNCTION COMMON-LISP-USER::F2 (2)> for arguments of types
; (#<BUILT-IN-CLASS COMMON-LISP:SYMBOL> #<SB-PCL:SYSTEM-CLASS COMMON-LISP:T>
; #<SB-PCL:SYSTEM-CLASS COMMON-LISP:T> #<SB-PCL:SYSTEM-CLASS COMMON-LISP:T>)
; uses the effective method being computed.

[...]
;

[Douglas Katzman (dougk)]

The error is telling you exactly what not to do, which isn't strictly speaking just a CLOS-related issue: don't make a macroexpand hook that invokes the macroexpand hook.

Suppose that instead of your generic function, the macroexpand hook were an interpreted function.
The exact same problem occurs, but it manifests as stack overflow:

(defun f2 (form-car fn form env)
  (cond ((eq form-car 'defun)
         (format t "~&boo~%")))
  (funcall fn form env))

* #'f2
#<SB-KERNEL:INTERPRETED-FUNCTION F2>

* (g)
INFO: Control stack guard page unprotected
Control stack guard page temporarily disabled: proceed with caution

debugger invoked on a SB-KERNEL::CONTROL-STACK-EXHAUSTED in thread
#<THREAD "main thread" RUNNING {10005885B3}>:
  Control stack exhausted (no more space for function call frames).

So to be clear, are you asking for this to "work", or are you asking not to run into CLOS limitation? I'll grant that there are CLOS implementations that do not encounter this problem, but if this is purely a contrived test, it seems like it's doing what it should.

One more thing - I had to cheat to get stack overflow- I remove the test for infinite looping in the interpreted function case, but that test could succumb to the same failure even if I did not - a compiled function that calls an interpreted function would fail.

Maybe the thing to do is prevent recursive use of *macroexpand-hook* in our logic for MACROEXPAND.

[Douglas Katzman (dougk)]

I suspect I'll apply this diff after freeze. It makes the test case work.

diff --git a/src/pcl/fngen.lisp b/src/pcl/fngen.lisp
index c6b51b63d..6721e3959 100644
--- a/src/pcl/fngen.lisp
+++ b/src/pcl/fngen.lisp
@@ -116,7 +116,12 @@
                                (declare (muffle-conditions compiler-note)
                                         (optimize (sb-c:store-source-form 0)))
                                (function ,code))))
- (let ((generator (compile nil generator-lambda)))
+ ;; Reliance on COMPILE is a hidden detail - for all people know,
+ ;; we turn EMFs into assembly code via an oracle.
+ ;; As such we need to minimize externally visible effects
+ ;; like invoking user-supplied macroexpander hooks.
+ (let ((generator (let ((*macroexpand-hook* #'funcall))
+ (compile nil generator-lambda))))
         (ensure-fgen test gensyms generator generator-lambda nil)
         generator))))

@@ -133,6 +138,7 @@

 (defun compute-code (lambda code-converter)
   (let ((*walk-form-expand-macros-p* t)
+ (*macroexpand-hook* #'funcall) ; See comment at GET-NEW-FUN-GENERATOR
         (gensyms ()))
     (values (walk-form lambda
                        nil
@@ -149,6 +155,7 @@

 (defun compute-constants (lambda constant-converter)
   (let ((*walk-form-expand-macros-p* t) ; doesn't matter here.
+ (*macroexpand-hook* #'funcall) ; See comment at GET-NEW-FUN-GENERATOR
         collect)
     (walk-form lambda
                nil

[Paul F. Dietz (paul-f-dietz)]

What led to this was trying to use a generic function in the macroexpand hook. That seems a perfectly reasonable thing to want to do -- but the CLOS implementation compiles the effective method function on demand, and it happens to do that using the user defined macroexpand hook. Boom.

BTW, this is in service to a little project I have for a mutation testing framework for Common Lisp, done internally to CL. It's not contrived; it came up working on that project. The macroexpand hook will intercept macros like DEFUN and record information there at expansion time, to be used later when the function is mutated. I know Google has a mutation testing framework for Common Lisp (and other languages), tied into the code review process, but I suspect it's more text oriented.

[Paul F. Dietz (paul-f-dietz)]

To make my previous note clear: I am not interested in mutation testing the innards of the functions generated by the CLOS implementation, so your solution of rebinding *macroexpand-hook* to #'funcall would be just fine. You will have to be careful that there isn't user code being compiled along with that (the forms in eql specializers, the bodies of methods) because compiling that could visibly depend on the user's *macroexpand-hook*.