From 1d47d777ef24c0be9153b0a1c8ba21918fa1025a Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Sat, 20 Jan 2018 11:27:23 -0500 Subject: [PATCH] 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 db59463235f..2d3d9d7992e 100644 --- a/doc/lispref/functions.texi +++ b/doc/lispref/functions.texi @@ -371,8 +371,8 @@ keyword @code{&rest} before one final argument. @example @group (@var{required-vars}@dots{} - @r{[}&optional @var{optional-vars}@dots{}@r{]} - @r{[}&rest @var{rest-var}@r{]}) + @r{[}&optional @r{[}@var{optional-vars}@dots{}@r{]}@r{]} + @r{[}&rest @r{[}@var{rest-var}@r{]}@r{]}) @end group @end example diff --git a/etc/NEWS b/etc/NEWS index 99f3f27486b..d1db830afa1 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 07476f1ac96..78d3071b168 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 08a73b1e4a5..a6e1d86c4ab 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 e68fd136113..59da6b7cc30 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*))