bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky <npostavs@users.sourceforge.net>]

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

tags 29165 = patch
quit

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> I've reverted my cl-lib code changes, so the conflict should be resolved
> now.  As for this bug, it seems the best way forward is to relax the
> checks so that an empty list of &optional variables is accepted.

Here's a patch.  Details of behaviour changes over versions are in the
commit message.

I probably should have got around to this before 26.0.91, but I think
this should still go to emacs-26 because the patch makes the behaviour
closer to what was in v25.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 8829 bytes --]

From b019118586204357c3d20541724c63163eed695b Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 20 Jan 2018 11:27:23 -0500
Subject: [PATCH v1] 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.

| arglist                   | i/c 25 | i/c 26.0.90 | i/c new |
|---------------------------+--------+-------------+---------|
| (&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     |

(require 'bytecomp)
(defun ck-args (arglist)
  (insert
   (condition-case err
       (progn (funcall `(lambda ,arglist 'ok))
              "y")
     (error (error-message-string err)
            "n"))
   "/"
   (condition-case err
       (progn (byte-compile-check-lambda-list arglist)
              "y")
     (error (error-message-string err)
            "n"))))

(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)))
    (ck-args arglist)
    (insert "\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.
---
 etc/NEWS                    | 28 ++++++++++++++++++++++------
 lisp/emacs-lisp/bytecomp.el | 11 ++++-------
 src/eval.c                  | 10 +++-------
 test/src/eval-tests.el      | 20 +++++++++++++++++---
 4 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 67915c7024..f5859d7a60 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1493,15 +1493,31 @@ support, you should set 'eldoc-documentation-function' instead of
 calling 'eldoc-message' directly.
 
 ---
-** Using '&rest' or '&optional' incorrectly is now an error.
-For example giving '&optional' without a following variable, or
-passing '&optional' multiple times:
+** Emacs is now more consistent about use of '&rest' and '&optional'.
+
+---
+*** Using them twice is now an error.
 
-    (defun foo (&optional &rest x))
     (defun bar (&optional &optional x))
+    (defun bar (&rest &rest x))
+
+Previously, Emacs ignored the extra keyword.
+
+---
+*** Putting '&optional' after '&rest' is now an error.
+
+    (defun foo (&rest &optional x))
+
+Previously, it was only a compilation error, but the interpreter
+accepted it.
+
+---
+*** Omitting variables after '&optional' or '&rest' is now accepted.
+
+    (defun foo (&optional))
 
-Previously, Emacs would just ignore the extra keyword, or give
-incorrect results in certain cases.
+Previously, it was accepted only in certain cases, e.g., '&optional'
+if it was followed immediately by '&rest'.
 
 ---
 ** The pinentry.el library has been removed.
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index eb00725776..222aca05f2 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2749,15 +2749,12 @@ 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")))
+		 (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 e05a17f7b4..9ec937f1a2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2980,7 +2980,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 ();
@@ -2991,17 +2990,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
 	{
@@ -3025,11 +3022,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 201382da9c..57faa0feae 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -37,8 +37,7 @@ 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 '((&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 @@ byte-compile-debug
     (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*))
-- 
2.11.0