[Emacs-diffs] master 1d47d77 1/2: Allow `&rest' or `&optional' without following variable (Bug#29165)

[Noam Postavsky]

branch: master
commit 1d47d777ef24c0be9153b0a1c8ba21918fa1025a
Author: Noam Postavsky <address@hidden>
Commit: Noam Postavsky <address@hidden>

    Allow `&rest' or `&optional' without following variable (Bug#29165)
    
    This is sometimes convenient when writing macros, so that the empty
    variable case doesn't need to be handled specially.  Older versions of
    Emacs accepted this in some cases (especially the interpreter in Emacs
    25 and below was very accepting).
    
                                |   interpreted/compiled   |
    | arglist                   | 25 & earlier | 26  | 27  |
    |---------------------------+--------------+-----+-----|
    | (&rest)                   | y/n          | n/n | y/y |
    | (&rest &rest)             | y/n          | n/n | n/n |
    | (&rest &rest x)           | y/n          | n/n | n/n |
    | (&rest x &rest)           | y/n          | n/n | n/n |
    | (&rest x &rest y)         | y/n          | n/n | n/n |
    |---------------------------+--------------+-----+-----|
    | (&optional)               | y/n          | n/n | y/y |
    | (&optional &optional)     | y/n          | n/n | n/n |
    | (&optional x &optional)   | y/n          | n/n | n/n |
    | (&optional x &optional y) | y/y          | n/n | n/n |
    |---------------------------+--------------+-----+-----|
    | (&optional &rest)         | y/n          | n/n | y/y |
    | (&optional x &rest)       | y/n          | n/n | y/y |
    | (&optional &rest y)       | y/y          | n/n | y/y |
    |---------------------------+--------------+-----+-----|
    | (&rest &optional)         | y/n          | n/n | n/n |
    | (&rest &optional y)       | y/n          | n/n | n/n |
    | (&rest x &optional y)     | y/n          | n/n | n/n |
    
    The values in the table above can be produced with the following code:
    
    (with-current-buffer (get-buffer-create "*ck-args*")
      (erase-buffer)
      (dolist (arglist '((&rest)
                         (&rest &rest)
                         (&rest &rest x)
                         (&rest x &rest)
                         (&rest x &rest y)
                         (&optional)
                         (&optional &optional)
                         (&optional x &optional)
                         (&optional x &optional y)
                         (&optional &rest)
                         (&optional x &rest)
                         (&optional &rest y)
                         (&rest &optional)
                         (&rest &optional y)
                         (&rest x &optional y)))
        (insert
         (format "%c/%c\n"
                 (condition-case err
                     (progn (funcall `(lambda ,arglist 'ok))
                            ?y)
                   (error ?n))
                 (condition-case err
                     (progn (byte-compile-check-lambda-list arglist)
                            ?y)
                   (error ?n))))
        (display-buffer (current-buffer))))
    
    * src/eval.c (funcall_lambda):
    * lisp/emacs-lisp/bytecomp.el (byte-compile-check-lambda-list): Don't
    check for missing variables after `&rest' and `&optional'.
    * test/src/eval-tests.el (eval-tests--bugs-24912-and-24913)
    (eval-tests-accept-empty-optional-rest): Update tests accordingly.
    * etc/NEWS: Update announcement accordingly.
    * doc/lispref/functions.texi (Argument List): Update manual to
    indicate that variable names are optional.
---
 doc/lispref/functions.texi  |  4 ++--
 etc/NEWS                    |  7 +++++++
 lisp/emacs-lisp/bytecomp.el | 11 ++++-------
 src/eval.c                  | 10 +++-------
 test/src/eval-tests.el      | 20 +++++++++++++++++---
 5 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index db59463..2d3d9d7 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -371,8 +371,8 @@ keyword @code{&rest} before one final argument.
 @example
 @group
 (@address@hidden
- @r{[}&optional @address@hidden@r{]}
- @r{[}&rest @address@hidden)
+ @r{[}&optional @address@hidden@address@hidden@r{]}
+ @r{[}&rest @address@hidden@address@hidden)
 @end group
 @end example
 
diff --git a/etc/NEWS b/etc/NEWS
index 99f3f27..d1db830 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -380,6 +380,13 @@ backslash.  For example:
     (read "‘smart") => (invalid-read-syntax "strange quote" "‘")
     (read "\\‘smart") == (intern "‘smart")
 
++++
+** Omitting variables after '&optional' and '&rest' is now allowed.
+For example (defun foo (&optional)) is no longer an error.  This is
+sometimes convenient when writing macros.  See the ChangeLog entry
+titled "Allow `&rest' or `&optional' without following variable" for a
+full listing of which arglists are accepted across versions.
+
 ** Internal parsing commands now use syntax-ppss and disregard
 open-paren-in-column-0-is-defun-start.  This affects mostly things like
 forward-comment, scan-sexps, and forward-sexp when parsing backward.
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 07476f1..78d3071 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2737,15 +2737,12 @@ If FORM is a lambda or a macro, byte-compile it as a 
function."
                   (macroexp--const-symbol-p arg t))
               (error "Invalid lambda variable %s" arg))
              ((eq arg '&rest)
-              (unless (cdr list)
-                (error "&rest without variable name"))
               (when (cddr list)
-                (error "Garbage following &rest VAR in lambda-list")))
+                (error "Garbage following &rest VAR in lambda-list"))
+               (when (memq (cadr list) '(&optional &rest))
+                 (error "%s following &rest in lambda-list" (cadr list))))
              ((eq arg '&optional)
-              (when (or (null (cdr list))
-                         (memq (cadr list) '(&optional &rest)))
-                (error "Variable name missing after &optional"))
-               (when (memq '&optional (cddr list))
+               (when (memq '&optional (cdr list))
                  (error "Duplicate &optional")))
              ((memq arg vars)
               (byte-compile-warn "repeated variable %s in lambda-list" arg))
diff --git a/src/eval.c b/src/eval.c
index 08a73b1..a6e1d86 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3035,7 +3035,6 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
     emacs_abort ();
 
   i = optional = rest = 0;
-  bool previous_optional_or_rest = false;
   for (; CONSP (syms_left); syms_left = XCDR (syms_left))
     {
       maybe_quit ();
@@ -3046,17 +3045,15 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
 
       if (EQ (next, Qand_rest))
         {
-          if (rest || previous_optional_or_rest)
+          if (rest)
             xsignal1 (Qinvalid_function, fun);
           rest = 1;
-          previous_optional_or_rest = true;
         }
       else if (EQ (next, Qand_optional))
         {
-          if (optional || rest || previous_optional_or_rest)
+          if (optional || rest)
             xsignal1 (Qinvalid_function, fun);
           optional = 1;
-          previous_optional_or_rest = true;
         }
       else
        {
@@ -3080,11 +3077,10 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
          else
            /* Dynamically bind NEXT.  */
            specbind (next, arg);
-          previous_optional_or_rest = false;
        }
     }
 
-  if (!NILP (syms_left) || previous_optional_or_rest)
+  if (!NILP (syms_left))
     xsignal1 (Qinvalid_function, fun);
   else if (i < nargs)
     xsignal2 (Qwrong_number_of_arguments, fun, make_number (nargs));
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index e68fd13..59da6b7 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -37,8 +37,7 @@
 (ert-deftest eval-tests--bugs-24912-and-24913 ()
   "Check that Emacs doesn't accept weird argument lists.
 Bug#24912 and Bug#24913."
-  (dolist (args '((&optional) (&rest) (&optional &rest) (&rest &optional)
-                  (&optional &rest a) (&optional a &rest)
+  (dolist (args '((&rest &optional)
                   (&rest a &optional) (&rest &optional a)
                   (&optional &optional) (&optional &optional a)
                   (&optional a &optional b)
@@ -47,7 +46,22 @@ Bug#24912 and Bug#24913."
     (should-error (eval `(funcall (lambda ,args)) t) :type 'invalid-function)
     (should-error (byte-compile-check-lambda-list args))
     (let ((byte-compile-debug t))
-      (should-error (eval `(byte-compile (lambda ,args)) t)))))
+      (ert-info ((format "bytecomp: args = %S" args))
+       (should-error (eval `(byte-compile (lambda ,args)) t))))))
+
+(ert-deftest eval-tests-accept-empty-optional-rest ()
+  "Check that Emacs accepts empty &optional and &rest arglists.
+Bug#24912."
+  (dolist (args '((&optional) (&rest) (&optional &rest)
+                  (&optional &rest a) (&optional a &rest)))
+    (let ((fun `(lambda ,args 'ok)))
+      (ert-info ("eval")
+        (should (eq (funcall (eval fun t)) 'ok)))
+      (ert-info ("byte comp check")
+        (byte-compile-check-lambda-list args))
+      (ert-info ("bytecomp")
+        (let ((byte-compile-debug t))
+          (should (eq (funcall (byte-compile fun)) 'ok)))))))
 
 
 (dolist (form '(let let*))