unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: akater <nuclearspace@gmail.com>
To: emacs-devel@gnu.org
Cc: Stefan Monnier <monnier@iro.umontreal.ca>
Subject: [PATCH] Some improvements for cl-flet
Date: Thu, 23 Sep 2021 22:37:33 +0000	[thread overview]
Message-ID: <87mto2gbpu.fsf@gmail.com> (raw)
In-Reply-To: <87bl4zqnqn.fsf@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 6675 bytes --]

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]]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

  parent reply	other threads:[~2021-09-23 22:37 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-11 12:51 Some improvements for cl-flet akater
2021-09-11 23:32 ` Michael Heerdegen
2021-09-12  3:35   ` akater
2021-09-12 15:38     ` Stefan Monnier
2021-09-13  0:14     ` Michael Heerdegen
2021-09-13  2:26       ` Stefan Monnier
2021-10-07  2:32       ` akater
2021-10-07 18:03         ` Stefan Monnier
2021-10-08 21:57           ` Richard Stallman
2021-10-09  5:23             ` akater
2021-10-09  6:01               ` Michael Heerdegen
2021-10-09 23:37                 ` Richard Stallman
2021-10-10 10:41                   ` Po Lu
2021-10-10 20:27                     ` João Távora
2021-10-10 21:57                       ` Stefan Monnier
2021-10-11  0:45                       ` [External] : " Drew Adams
2021-10-11 21:16                     ` Richard Stallman
2021-10-11 21:26                       ` João Távora
2021-10-12 22:42                         ` Richard Stallman
2021-10-12  0:05                       ` Po Lu
2021-10-12  0:29                       ` Robin Tarsiger
2021-10-12 22:43                         ` Richard Stallman
2021-10-09 23:33               ` Richard Stallman
2021-10-09 23:33               ` Richard Stallman
2021-10-14  4:00               ` Michael Heerdegen
2021-09-23 22:37 ` akater [this message]
2021-09-23 22:41   ` [PATCH] " akater
2021-09-24  7:11     ` João Távora
2021-09-24 15:20       ` [PATCH] Some improvements for cl-flet, and some more akater
2021-09-24 16:22         ` João Távora
2021-09-25  1:28         ` Lars Ingebrigtsen
2021-09-25  8:37           ` João Távora
2021-09-24 20:30     ` [PATCH] Some improvements for cl-flet akater
2021-09-26  6:54     ` Lars Ingebrigtsen
2021-09-26 12:04       ` akater
2021-09-26 12:36         ` Lars Ingebrigtsen
2021-10-03  3:51     ` Stefan Monnier
2021-10-07  5:02       ` akater
2021-10-07 18:23         ` Stefan Monnier
2021-11-03 12:59           ` akater
2021-11-09 20:37             ` Stefan Monnier
2021-10-09  5:33       ` akater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mto2gbpu.fsf@gmail.com \
    --to=nuclearspace@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).