cl-macs: default indentation for all arglists with top-level &body

cl-macs: default indentation for all arglists with top-level &body

[akater]

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



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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cl-macs: indent via top-level &body --]
[-- Type: text/x-diff, Size: 3796 bytes --]

From 59e296eaf48494c60f5dcea88b9afd3db42b5764 Mon Sep 17 00:00:00 2001
From: akater <nuclearspace@gmail.com>
Date: Sun, 11 Apr 2021 10:53:25 +0000
Subject: [PATCH] cl-macs: default indentation for all arglists with top-level
 &body

---
 lisp/emacs-lisp/cl-macs.el | 45 ++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 68211ec410..f04b9e9b5f 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -43,6 +43,8 @@
 
 ;;; Code:
 
+(eval-when-compile (require 'subr-x))
+
 (require 'cl-lib)
 (require 'macroexp)
 ;; `gv' is required here because cl-macs can be loaded before loaddefs.el.
@@ -243,6 +245,20 @@ defvar cl--bind-defs) ;(DEF . DEFS) giving the "default default" for optargs.
 (defvar cl--bind-enquote)      ;Non-nil if &cl-quote was in the formal arglist!
 (defvar cl--bind-lets) (defvar cl--bind-forms)
 
+;;;###autoload
+(defmacro cl-dolist (spec &rest body)
+  "Loop over a list.
+Evaluate BODY with VAR bound to each `car' from LIST, in turn.
+Then evaluate RESULT to get return value, default nil.
+An implicit nil block is established around the loop.
+
+\(fn (VAR LIST [RESULT]) BODY...)"
+  (declare (debug ((symbolp form &optional form) cl-declarations body))
+           (indent 1))
+  (let ((loop `(dolist ,spec ,@body)))
+    (if (advice-member-p 'cl--wrap-in-nil-block 'dolist)
+        loop `(cl-block nil ,loop))))
+
 (defun cl--transform-lambda (form bind-block)
   "Transform a function form FORM of name BIND-BLOCK.
 BIND-BLOCK is the name of the symbol to which the function will be bound,
@@ -319,6 +335,21 @@ defun cl--transform-lambda (form bind-block)
                                   (format "%S" (cons 'fn (cl--make-usage-args
                                                           orig-args))))))
                               header)))
+                (when-let ((according-to-&body
+                            (let ((counter 0))
+                              (cl-dolist (x orig-args)
+                                (cond
+                                 ((eq '&body x) (cl-return counter))
+                                 ((not (memq x cl--lambda-list-keywords))
+                                  (cl-incf counter)))))))
+                  ;; Placing the declaration in the end
+                  ;; allows overriding the indentation setting
+                  ;; with an explicit (declare (indent ..)) statement
+                  ;; manually written in the form being expanded.
+                  (setq header
+                        (nconc header
+                               (list
+                                `(declare (indent ,according-to-&body))))))
                 ;; FIXME: we'd want to choose an arg name for the &rest param
                 ;; and pass that as `expr' to cl--do-arglist, but that ends up
                 ;; generating code with a redundant let-binding, so we instead
@@ -1821,20 +1852,6 @@ defun cl--expand-do-loop (steps endtest body star)
                              (apply #'append sets))))))
       ,@(or (cdr endtest) '(nil)))))
 
-;;;###autoload
-(defmacro cl-dolist (spec &rest body)
-  "Loop over a list.
-Evaluate BODY with VAR bound to each `car' from LIST, in turn.
-Then evaluate RESULT to get return value, default nil.
-An implicit nil block is established around the loop.
-
-\(fn (VAR LIST [RESULT]) BODY...)"
-  (declare (debug ((symbolp form &optional form) cl-declarations body))
-           (indent 1))
-  (let ((loop `(dolist ,spec ,@body)))
-    (if (advice-member-p 'cl--wrap-in-nil-block 'dolist)
-        loop `(cl-block nil ,loop))))
-
 ;;;###autoload
 (defmacro cl-dotimes (spec &rest body)
   "Loop a certain number of times.
-- 
2.26.2


[-- Attachment #3: Type: text/plain, Size: 3148 bytes --]


(This has been discussed earlier on the list --- more on this later.)

In Common Lisp, ~&body~ keyword is the same as ~&rest~ modulo
indentation:

#+begin_quote
&body is identical in function to &rest, but it can be used to inform
certain output-formatting and editing functions that the remainder of the
form is treated as a body, and should be indented accordingly.
#+end_quote
--- [[http://clhs.lisp.se/Body/03_dd.htm][CLHS, 3.4.4 Macro Lambda Lists]]

cl-macs does support ~&body~; however toplevel ~&body~ does not affect
indentation.  This can be fixed by augmenting the definition of
~cl--transform-lambda~ as follows:
#+begin_example emacs-lisp
(when-let ((according-to-&body
            (let ((counter 0))
              (cl-dolist (x orig-args)
                (cond
                 ((eq '&body x) (cl-return counter))
                 ((not (memq x cl--lambda-list-keywords))
                  (cl-incf counter)))))))
  ;; Placing the declaration in the end
  ;; allows overriding the indentation setting
  ;; with an explicit (declare (indent ..)) statement
  ;; manually written in the form being expanded.
  (setq header
        (nconc header
               (list
                `(declare (indent ,according-to-&body))))))
#+end_example

See the attached patch for details.  I hesitate to recommend using the
patch as is because my use of ~cl-dolist~ likely interrupts the flow of
cl-macs.  In fact, the patch as is might actually break the build:
unfortunately, I cannot do a test build of Emacs right now.  I thus did
not try to polish something I wouldn't actually be able to test.  The
patch is provided for the sake of completeness.

Now to (possibly incomplete) summary of the 2005-07 thread and
comparison.

The feature was
- suggested to be applied to usual ~defmacro~ as well
- not implemented as there is no ~&body~ in ~defmacro~
- apparently shelved and then forgotten about

I only suggest to alter ~cl-defmacro~.

In addition, 2005-07 solution
- is implemented with ~(put .. 'lisp-indent-function ..)~
- does not check for preceding keywords

The link:
https://lists.gnu.org/archive/html/emacs-devel/2005-07/msg00457.html

I check for all ~cl--lambda-list-keywords~.  This is excessive but it
also makes for a cleaner code, properly treats arglists with
~&optional~, ~&whole~ (if ever implmeneted) and whatever else happens to
be in the arglist; note that according to Common Lisp specification,
arglsits can actually have arbitrary keywords.  ~&whole~ is only
relevant for macro definitions; arguably, it does make sense for
~cl--transform-lambda~ to take these types into account but I believe
the presented approach is good enough, and using &-keywords as regular
arguments in, say, ~cl-defun~ is certainly not a good idea.

As to whether the macro better inject ~(put ..)~ or ~(declare ..)~ form,
my preference is for the latter because
- I find it preferable to modify headers rather than the body
- I strongly prefer to generate code I myself would have written.  This
is apparently not a conventional practice in Lisp; mistakenly, I
believe, but that's offtopic.

Just in case, I have signed the papers already.

Re: cl-macs: default indentation for all arglists with top-level &body

[Stefan Monnier]

>   ;; Placing the declaration in the end
>   ;; allows overriding the indentation setting
>   ;; with an explicit (declare (indent ..)) statement
>   ;; manually written in the form being expanded.
>   (setq header
>         (nconc header
>                (list
>                 `(declare (indent ,according-to-&body))))))

I see one "problem" with this: it provides the `indent` but not the
`debug` spec.  Maybe another way to go about it is to try and infer the
`indent` spec from the `debug` spec (since I think it's hopeless to try
and guess the debug spec from the arglist).

> The feature was
> - suggested to be applied to usual ~defmacro~ as well
> - not implemented as there is no ~&body~ in ~defmacro~
> - apparently shelved and then forgotten about
>
> I only suggest to alter ~cl-defmacro~.

It would clearly be a lot more useful to make it work for `defmacro` as
well since the very vast majority of macros are defined with `defmacro`.

> As to whether the macro better inject ~(put ..)~ or ~(declare ..)~ form,
> my preference is for the latter because

FWIW, I think injecting `declare` is cleaner, indeed (but of course it
needs to be careful not to override a pre-existing `declare`).


        Stefan