[PATCH] Some improvements for cl-flet

[akater]

Sorry, it took longer than I expected, partially due to my desire to “do
everything right” the first time.

* Issues with cl-flet
There are several issues with ~cl-flet~.

** No error on illegal function names
#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1 `(cl-flet ((nil () t))
                  (funcall nil)))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(let*
    ((--cl-nil--
      (cl-function
       (lambda nil t))))
  (progn
    (funcall nil)))
#+end_example

#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1 `(cl-flet (((f) () t))
                  (funcall (f))))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(let*
    ((--cl-\(f\)--
      (cl-function
       (lambda nil t))))
  (progn
    (funcall
     (f))))
#+end_example

** No error on malformed specs
#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1 `(cl-flet ((f arglist yet-more))))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(let*
    ((--cl-f--
      (cl-function
       (lambda arglist yet-more))))
  (progn))
#+end_example

#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1 `(cl-flet ((f))))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(let*
    ((--cl-f--
      (cl-function
       (lambda))))
  (progn))
#+end_example

** Incorrectly treated (setf ..) local functions This will attempt to
use a (non-existent, in this case) global function instead of the
generated local one:

#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1
 `(cl-flet (((setf kar) (new cons) (setf (car cons) new)))
    (let ((cons (cons nil nil)))
      (setf (kar cons) t)
      cons)))
#+end_src

It would be nice to have a support for local setf functions, as in
Common Lisp.

** No warning on duplicated definitions
#+begin_src emacs-lisp :tangle no :results raw :wrap quote
(macroexpand-1 `(cl-flet ((f (x) (1+ x))
                          (f (x) x))
                  (f 42)))
(or (get-buffer "*Warnings*")
    "No warnings")
#+end_src

#+RESULTS:
#+begin_quote
No warnings
#+end_quote

** No warning on unused definitions
#+begin_src emacs-lisp :tangle no :results raw :wrap quote
(macroexpand-1 `(cl-flet ((f (x) x))
                  nil))
(or (get-buffer "*Warnings*")
    "No warnings")
#+end_src

#+RESULTS:
#+begin_quote
No warnings
#+end_quote

Unused definitions better be simply removed from the macroexpanded
code.

** No way to capture definitions present in the body
This is relevant for fixing an issue with ~cl--generic-lambda~: it needs
to record the used definitions; right now it parses with an expensive
and unreliable procedure.  The issue was discussed with Stefan Monnier
at https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg00092.html
and we agreed it's best to alter ~cl-flet~ to record the actually
encountered definitions during macroexpansion.

* Patch overview
The attached patch fixes all of the above.

Note: Supporting local (setf ..) functions requires significantly more
code.  However, I think it's worth it and I hope it gets accepted.

Issue: I tried to alias ~cl--generic-with-memoization~ to
~cl--with-memoization~ but that failed at build time.  For a working
prototype, I just duplicate the definition.  The macro itself is
exactly the same.

Re: capture definitions present in the body,
I decided that simply collecting the actually encountered definitions
during macroexpansion and returning the list would be too ad-hoc so I
implemented expander that could evaluate arbitrary user-provided code
during macroexpansion.  Said code should be provided as 0-argument
lambdas; they are only executed once; the return value is then memoized
and reused during macroexpansion.  The table of memoized values is used
to report the “missing” and “duplicate” warnings.

In a nutshell,
#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(macroexpand-1 `(cl-flet ((f (x y) do f stuff)
                          (g expr))
                  body))
#+end_src

now amounts to
#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(cl--expand-flet macroexpand-all-environment body
  'g (lambda () (list nil expr))
  'f (lambda () (list (make-symbol (format "--cl-%s--" 'f))
                      `(cl-function (lambda (x y) do f stuff)))))
#+end_src

The first element of the returned list indicates whether the second
element should be bound to a local variable, or substituted as is.
There is no lighter alternative as type of expr may vary significantly,
particularly due to keeping the (func exp) syntax for c~l-flet~.

We could use cons rather than list for values but this way, return
values can be employed as let bindings as is.

It might be worth it to make lambdas accept arguments passed to local
functions in the body of ~cl-flet~ and make them run each time.  I found
this interface and the corresponding implementation too cumbersome and
not worth it in the absence of specific applications.  One possible such
application might be found in FIXME entry from =cl-generic.el=:
#+begin_example emacs-lisp
;; FIXME: Optimize the case where call-next-method is
;; only called with explicit arguments.
#+end_example

but I'm not sure I'm correct.

The new implementation of ~cl-flet~ can be macroexpanded with
~macroexpand-1~ just like the old one, so everything is compatible at
this level as well:

#+begin_src emacs-lisp :tangle no :results code :wrap example emacs-lisp
(cl-values
 (macroexpand-1 `(cl--flet/new ((f (x y) (+ x y)))
                   (f 1 2)))
 (macroexpand-1 `(cl--flet/old ((f (x y) (+ x y)))
                   (f 1 2))))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
((let*
     ((--cl-f--
       (cl-function
        (lambda
          (x y)
          (+ x y)))))
   (funcall --cl-f-- 1 2))
 (let*
     ((--cl-f--
       (cl-function
        (lambda
          (x y)
          (+ x y)))))
   (progn
     (funcall --cl-f-- 1 2))))
#+end_example

See the links below for ≈50 examples and tests in Org markup,
for general feel of what the patch does and what I've checked.

* External Links
- 
[[https://framagit.org/akater/elisp-cl-flet-improvement/-/raw/master/cl-flet-improvement.org?inline=true][for
 EWW and org-mode (recommended; see Tests section)]]
- 
[[https://framagit.org/akater/elisp-cl-flet-improvement/-/blob/master/cl-flet-improvement.org#L295][for
 other browsers]]

signature.asc
Description: PGP signature