>From b019118586204357c3d20541724c63163eed695b Mon Sep 17 00:00:00 2001 From: Noam Postavsky 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