master ed02b88 1/7: Renege on anonymous &rest (bug#50268, bug#50720)

[Mattias Engdegård]

branch: master
commit ed02b88bbae18caad650d76876940ffb58cab554
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    Renege on anonymous &rest (bug#50268, bug#50720)
    
    Allowing &rest without a variable name following turned out not to be
    very useful, and it never worked properly.  Disallow it.
    
    * lisp/emacs-lisp/bytecomp.el (byte-compile-check-lambda-list):
    * src/eval.c (funcall_lambda):
    Signal error for &rest without variable name.
    * doc/lispref/functions.texi (Argument List): Adjust manual.
    * etc/NEWS (file): Announce.
    * test/src/eval-tests.el (eval-tests--bugs-24912-and-24913):
    Extend test, also checking with and without lexical binding.
    (eval-tests-accept-empty-optional-rest): Reduce to...
    (eval-tests-accept-empty-optional): ...this, again checking
    with and without lexical binding.
---
 doc/lispref/functions.texi  |  2 +-
 etc/NEWS                    |  7 ++++++
 lisp/emacs-lisp/bytecomp.el |  2 ++
 src/eval.c                  |  9 ++++---
 test/src/eval-tests.el      | 57 ++++++++++++++++++++++++++-------------------
 5 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index 77d1465..c856557 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -378,7 +378,7 @@ keyword @code{&rest} before one final argument.
 @group
 (@var{required-vars}@dots{}
  @r{[}&optional @r{[}@var{optional-vars}@dots{}@r{]}@r{]}
- @r{[}&rest @r{[}@var{rest-var}@r{]}@r{]})
+ @r{[}&rest @var{rest-var}@r{]})
 @end group
 @end example
 
diff --git a/etc/NEWS b/etc/NEWS
index f211a98..61780a3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3273,6 +3273,13 @@ local variable would not be heeded.  This has now 
changed, and a file
 with a 'lexical-binding' cookie is always heeded.  To revert to the
 old behavior, set 'permanently-enabled-local-variables' to nil.
 
++++
+** '&rest' in argument lists must always be followed by a variable name.
+Omitting the variable name after '&rest' was previously tolerated in
+some cases but not consistently so; it could lead to crashes or
+outright wrong results.  Since the utility was marginal at best, it is
+now an error to omit the variable.
+
 ---
 ** 'kill-all-local-variables' has changed how it handles non-symbol hooks.
 The function is documented to eliminate all buffer-local bindings
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index be74195..d7da7a2 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2930,6 +2930,8 @@ 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"))
                (when (memq (cadr list) '(&optional &rest))
diff --git a/src/eval.c b/src/eval.c
index 2bb7cfe..66d3480 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3245,6 +3245,7 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
     emacs_abort ();
 
   i = optional = rest = 0;
+  bool previous_rest = false;
   for (; CONSP (syms_left); syms_left = XCDR (syms_left))
     {
       maybe_quit ();
@@ -3255,13 +3256,14 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
 
       if (EQ (next, Qand_rest))
         {
-          if (rest)
+          if (rest || previous_rest)
             xsignal1 (Qinvalid_function, fun);
           rest = 1;
+         previous_rest = true;
         }
       else if (EQ (next, Qand_optional))
         {
-          if (optional || rest)
+          if (optional || rest || previous_rest)
             xsignal1 (Qinvalid_function, fun);
           optional = 1;
         }
@@ -3287,10 +3289,11 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
          else
            /* Dynamically bind NEXT.  */
            specbind (next, arg);
+         previous_rest = false;
        }
     }
 
-  if (!NILP (syms_left))
+  if (!NILP (syms_left) || previous_rest)
     xsignal1 (Qinvalid_function, fun);
   else if (i < nargs)
     xsignal2 (Qwrong_number_of_arguments, fun, make_fixnum (nargs));
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index b2b7dfe..3c3e703 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -39,31 +39,40 @@
 (ert-deftest eval-tests--bugs-24912-and-24913 ()
   "Check that Emacs doesn't accept weird argument lists.
 Bug#24912 and Bug#24913."
-  (dolist (args '((&rest &optional)
-                  (&rest a &optional) (&rest &optional a)
-                  (&optional &optional) (&optional &optional a)
-                  (&optional a &optional b)
-                  (&rest &rest) (&rest &rest a)
-                  (&rest a &rest b)))
-    (should-error (eval `(funcall (lambda ,args)) t) :type 'invalid-function)
-    (should-error (byte-compile-check-lambda-list args))
-    (let ((byte-compile-debug 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.
+  (dolist (lb '(t false))
+    (ert-info ((prin1-to-string lb) :prefix "lexical-binding: ")
+      (let ((lexical-binding lb))
+        (dolist (args '((&rest &optional)
+                        (&rest a &optional) (&rest &optional a)
+                        (&optional &optional) (&optional &optional a)
+                        (&optional a &optional b)
+                        (&rest &rest) (&rest &rest a)
+                        (&rest a &rest b)
+                        (&rest) (&optional &rest)
+                        ))
+          (ert-info ((prin1-to-string args) :prefix "args: ")
+            (should-error
+             (eval `(funcall (lambda ,args)) lb) :type 'invalid-function)
+            (should-error (byte-compile-check-lambda-list args))
+            (let ((byte-compile-debug t))
+              (should-error (eval `(byte-compile (lambda ,args)) lb)))))))))
+
+(ert-deftest eval-tests-accept-empty-optional ()
+  "Check that Emacs accepts empty &optional 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 (lb '(t false))
+    (ert-info ((prin1-to-string lb) :prefix "lexical-binding: ")
+      (let ((lexical-binding lb))
+        (dolist (args '((&optional) (&optional &rest a)))
+          (ert-info ((prin1-to-string args) :prefix "args: ")
+            (let ((fun `(lambda ,args 'ok)))
+              (ert-info ("eval")
+                (should (eq (funcall (eval fun lb)) '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*))