From: Noam Postavsky <npostavs@users.sourceforge.net>
To: Glenn Morris <rgm@gnu.org>
Cc: Ken Raeburn <raeburn@permabit.com>,
Andreas Schwab <schwab@linux-m68k.org>,
Philipp Stephani <p.stephani2@gmail.com>,
29165@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
Subject: bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
Date: Sat, 20 Jan 2018 17:10:17 -0500 [thread overview]
Message-ID: <87po64npva.fsf@users.sourceforge.net> (raw)
In-Reply-To: <87r2rvb7c2.fsf@users.sourceforge.net> (Noam Postavsky's message of "Fri, 15 Dec 2017 23:54:53 -0500")
[-- 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
next prev parent reply other threads:[~2018-01-20 22:10 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-06 6:57 bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24 Ken Raeburn
2017-11-06 12:44 ` Noam Postavsky
2017-11-06 14:24 ` Drew Adams
2017-11-06 14:35 ` Noam Postavsky
2017-11-06 14:40 ` Drew Adams
2017-11-06 17:20 ` Philipp Stephani
2017-11-06 17:25 ` Ken Raeburn
2017-11-06 18:10 ` Andreas Schwab
2017-11-06 19:10 ` Ken Raeburn
2017-11-06 19:16 ` Noam Postavsky
2017-11-13 18:06 ` Noam Postavsky
2017-11-13 19:42 ` Ken Raeburn
2017-11-13 20:05 ` Noam Postavsky
2017-11-27 22:24 ` Noam Postavsky
2017-12-13 22:36 ` Noam Postavsky
2017-12-15 16:48 ` Glenn Morris
2017-12-16 4:54 ` Noam Postavsky
2018-01-20 22:10 ` Noam Postavsky [this message]
2018-01-21 3:02 ` Stefan Monnier
2018-01-21 16:04 ` Eli Zaretskii
2018-01-21 16:30 ` Noam Postavsky
2018-01-21 18:01 ` Eli Zaretskii
2018-03-25 15:32 ` Noam Postavsky
2017-12-13 23:39 ` Stefan Monnier
2017-12-15 1:16 ` Noam Postavsky
2017-12-15 3:04 ` Stefan Monnier
2017-12-15 5:17 ` Ken Raeburn
2017-12-15 13:54 ` Stefan Monnier
2017-11-06 19:31 ` Drew Adams
2017-11-06 16:08 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87po64npva.fsf@users.sourceforge.net \
--to=npostavs@users.sourceforge.net \
--cc=29165@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
--cc=p.stephani2@gmail.com \
--cc=raeburn@permabit.com \
--cc=rgm@gnu.org \
--cc=schwab@linux-m68k.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.