Re: [PATCH] Some improvements for cl-flet

[Stefan Monnier <monnier@iro.umontreal.ca>]

> Sorry it took so long; Emacs build broke several times, an on top of
> that Org is having some tectonic changes as well, and I've had my own
> time trouble.

Seeing how slow I am on my side, you have nothing to apologize for.

>> diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
>> index 1852471bcbb..ad0477e3b68 100644
>> --- a/lisp/emacs-lisp/cl-macs.el
>> +++ b/lisp/emacs-lisp/cl-macs.el
>> @@ -2030,7 +2030,10 @@ cl-flet
>>    (let ((binds ()) (newenv macroexpand-all-environment))
>>      (dolist (binding bindings)
>>        (let ((var (make-symbol (format "--cl-%s--" (car binding))))
>> +            (fname (car binding))
>>              (args-and-body (cdr binding)))
>> +        (if (eq (car-safe fname) 'setf)
>> +            (setq fname (gv-setter (cadr fname))))
>>          (if (and (= (length args-and-body) 1) (symbolp (car args-and-body)))
>>              ;; Optimize (cl-flet ((fun var)) body).
>>              (setq var (car args-and-body))
>> @@ -2038,7 +2041,7 @@ cl-flet
>>                                (car args-and-body)
>>                              `(cl-function (lambda . ,args-and-body))))
>>                  binds))
>> -	(push (cons (car binding)
>> +	(push (cons fname
>>                      (lambda (&rest args)
>>                        (if (eq (car args) cl--labels-magic)
>>                            (list cl--labels-magic var)
>
> Four of my tests failed which did pass for the solution I've proposed.

Indeed, I saw some serious problems later on, which basically come down
to the case:

    (cl-flet ((foo FOO)
              ((setf foo) SFOO))
      ...)

where `cl-flet`s definition of `foo` as a macro causes all the
occurrences of (foo ...) inside gv-places to be replaced with (funcall
FOO ...) at which point the connection with (setf foo) is lost.

IIUC you solve this problem by writing your own tree walker, but I'd
*really* much prefer not doing that, and reuse the existing tree walker
in `macroexpand-all`.

> The first case is not even supported with global functions in Elisp so
> I'm not sure how relevant it is.

Indeed #'(setf ..) is currently not supported anywhere, so it's
a separate issue.


        Stefan