From: Mattias Engdegård Date: Thu, 23 Sep 2021 10:43:41 +0000 (+0200) Subject: Renege on anonymous &rest (bug#50268, bug#50720) X-Git-Tag: emacs-28.0.90~632 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=ed02b88bbae18caad650d76876940ffb58cab554;p=emacs.git 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. --- diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi index 77d1465c876..c856557c3cb 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 f211a98678c..61780a3a19e 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 be74195778b..d7da7a2149a 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 2bb7cfe6002..66d34808f82 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 b2b7dfefda5..3c3e7033419 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*))