bug#50268: 28.0.50; Assertion warning during native compilation

["Mattias Engdegård" <mattiase@acm.org>]

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

23 sep. 2021 kl. 03.46 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Not all of it, no.  Only the part which makes (&rest) acceptable.

Patch.


[-- Attachment #2: 0001-Renege-on-anonymous-rest-bug-50268-bug-50720.patch --]
[-- Type: application/octet-stream, Size: 7575 bytes --]

From c69c3b8c97e9aa9ff350b009c06b4e3e8842ba83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 23 Sep 2021 12:43:41 +0200
Subject: [PATCH] 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 77d1465c87..c856557c3c 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -378,7 +378,7 @@ Argument List
 @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 fb7a7f628a..cf3f90304d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3215,6 +3215,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 be74195778..d7da7a2149 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2930,6 +2930,8 @@ byte-compile-check-lambda-list
 		   (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 2bb7cfe600..66d34808f8 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 b2b7dfefda..3c3e703341 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -39,31 +39,40 @@ byte-compile-debug
 (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*))
-- 
2.21.1 (Apple Git-122.3)