all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
@ 2017-11-06  6:57 Ken Raeburn
  2017-11-06 12:44 ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Ken Raeburn @ 2017-11-06  6:57 UTC (permalink / raw)
  To: 29165


Create a file with the contents:

    (require 'cl)
    (defun* foobar (init-fun test-fun cleanup-fun
                             &optional &key
                             (test #'eql)
                             (size 65))
      "Blah blah blah"
      (list 'x init-fun test-fun cleanup-fun test size))


Use Emacs 24.3 to byte-compile the file.

Start the Emacs 26 pretest, and load the .elc file.

Type into the scratch buffer:

    (foobar 1 2 3 :test 4)

And use eval-last-sexp to try to evaluate it.  You should get a list
back.  I get an invalid-function error instead.  With the debugger
enabled, I see:

    Debugger entered--Lisp error: (invalid-function #f(compiled-function (init-fun test-fun cleanup-fun &optional &rest --cl-rest--) "Blah blah blah" #<bytecode 0x501733>))
      foobar(1 2 3 :test 4)
      eval((foobar 1 2 3 :test 4) nil)
      elisp--eval-last-sexp(nil)
      eval-last-sexp(nil)
      funcall-interactively(eval-last-sexp nil)
      call-interactively(eval-last-sexp nil nil)
      command-execute(eval-last-sexp)

It appears that the eval.c code of 26.0.90 doesn't like to see &optional
and &rest together in the argument list of a compiled function.  But in
this case, the byte compiler from version 24 does generate that.  In
version 25, only &rest is put into the list.


In GNU Emacs 26.0.90 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2017-10-27 built on just-testing.permabit.com
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 14.04.4 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Loading /tmp/foo.elc...done
Entering debugger...
Back to top level

Configured using:
 'configure --prefix=/u1/ken/install/emacs-26.0.90
 --with-x-toolkit=lucid --enable-checking --without-sound --without-gif'

Configured features:
XAW3D XPM JPEG TIFF PNG RSVG DBUS GSETTINGS NOTIFY ACL LIBSELINUX GNUTLS
LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11

Important settings:
  locale-coding-system: nil

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq dired
dired-loaddefs format-spec rfc822 mml mml-sec password-cache epa derived
epg epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils help-mode
easymenu cl-print byte-opt bytecomp byte-compile cconv debug cl gv
cl-loaddefs cl-lib elec-pair time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 98378 10227)
 (symbols 48 20669 1)
 (miscs 40 45 197)
 (strings 32 29669 1021)
 (string-bytes 1 765521)
 (vectors 16 14315)
 (vector-slots 8 495072 6314)
 (floats 8 53 146)
 (intervals 56 314 138)
 (buffers 992 12)
 (heap 1024 32780 1041))





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  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 16:08   ` Eli Zaretskii
  0 siblings, 2 replies; 30+ messages in thread
From: Noam Postavsky @ 2017-11-06 12:44 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Philipp Stephani, 29165

tags 29165 + notabug
quit

Ken Raeburn <raeburn@permabit.com> writes:

>     (defun* foobar (init-fun test-fun cleanup-fun
>                              &optional &key

> Use Emacs 24.3 to byte-compile the file.
> 
> Start the Emacs 26 pretest, and load the .elc file.

>     Debugger entered--Lisp error: (invalid-function #f(compiled-function (init-fun test-fun cleanup-fun &optional &rest --cl-rest--) "Blah blah blah" 

> It appears that the eval.c code of 26.0.90 doesn't like to see &optional
> and &rest together in the argument list of a compiled function.

Yes, see Bug#24912 and Bug#24913.

> But in this case, the byte compiler from version 24 does generate
> that.  In version 25, only &rest is put into the list.

It's not the byte compiler specifically, the defun* macro (aka cl-defun)
expands &key into &rest, so the expansion has &optional &rest.  At any
rate, using &optional is meaningless there, so the solution is just to
remove it.

We should perhaps put something about throwing error on '&option &rest'
into NEWS though.






^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 12:44 ` Noam Postavsky
@ 2017-11-06 14:24   ` Drew Adams
  2017-11-06 14:35     ` Noam Postavsky
  2017-11-06 16:08   ` Eli Zaretskii
  1 sibling, 1 reply; 30+ messages in thread
From: Drew Adams @ 2017-11-06 14:24 UTC (permalink / raw)
  To: Noam Postavsky, Ken Raeburn; +Cc: Philipp Stephani, 29165

> It's not the byte compiler specifically, the defun* macro (aka cl-defun)
> expands &key into &rest, so the expansion has &optional &rest.  At any
> rate, using &optional is meaningless there, so the solution is just to
> remove it.
> 
> We should perhaps put something about throwing error on '&option &rest'
> into NEWS though.

I don't understand.  In Common Lisp it is perfectly correct
to use both &optional and &rest.  If Emacs's emulation of
CL's `defun' is defective in this regard, is that not a bug?

https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node64.html

Or are you saying that there was a bug in Emacs 24's handling
of the CL `defun' emulation, where it expanded &key into
&rest, producing multiple &optional or &rest, and that now
this has been fixed - hence Emacs 24 byte-code does not work
in Emacs 26?

If you do add something to NEWS, please make clear just what
the problem is/was.  Does Emacs now correctly emulate CL
`defun' wrt &* keywords?  Is it an error to repeat &optional
or &rest?  If not, what is the behavior in that case?

Whatever the explanation is, please make it very clear in
NEWS.  Thx.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 14:24   ` Drew Adams
@ 2017-11-06 14:35     ` Noam Postavsky
  2017-11-06 14:40       ` Drew Adams
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-11-06 14:35 UTC (permalink / raw)
  To: Drew Adams; +Cc: Ken Raeburn, Philipp Stephani, 29165

On Mon, Nov 6, 2017 at 9:24 AM, Drew Adams <drew.adams@oracle.com> wrote:

>> We should perhaps put something about throwing error on '&option &rest'
>> into NEWS though.
>
> I don't understand.  In Common Lisp it is perfectly correct
> to use both &optional and &rest.

What's rejected is (&optional &rest other-vars), whereas (&optional
var1 &rest other-vars) is okay. Does CL accept the first form (and if
yes, what does it mean)? I couldn't tell from the page you linked to.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  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
  0 siblings, 2 replies; 30+ messages in thread
From: Drew Adams @ 2017-11-06 14:40 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Ken Raeburn, Philipp Stephani, 29165

> >> We should perhaps put something about throwing error on '&option &rest'
> >> into NEWS though.
> >
> > I don't understand.  In Common Lisp it is perfectly correct
> > to use both &optional and &rest.
> 
> What's rejected is (&optional &rest other-vars), whereas (&optional
> var1 &rest other-vars) is okay. Does CL accept the first form (and if
> yes, what does it mean)? I couldn't tell from the page you linked to.

CL accepts a single variable after &rest. And there must be
a variable after &optional.  (&optional foo &rest bar) is OK.

(&optional &rest foo) is not OK.
(&optional foo &rest bar toto titi) is not OK.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 12:44 ` Noam Postavsky
  2017-11-06 14:24   ` Drew Adams
@ 2017-11-06 16:08   ` Eli Zaretskii
  1 sibling, 0 replies; 30+ messages in thread
From: Eli Zaretskii @ 2017-11-06 16:08 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: raeburn, p.stephani2, 29165

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Mon, 06 Nov 2017 07:44:47 -0500
> Cc: Philipp Stephani <p.stephani2@gmail.com>, 29165@debbugs.gnu.org
> 
> We should perhaps put something about throwing error on '&option &rest'
> into NEWS though.

Please do, and thanks.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 14:40       ` Drew Adams
@ 2017-11-06 17:20         ` Philipp Stephani
  2017-11-06 17:25         ` Ken Raeburn
  1 sibling, 0 replies; 30+ messages in thread
From: Philipp Stephani @ 2017-11-06 17:20 UTC (permalink / raw)
  To: Drew Adams; +Cc: Ken Raeburn, 29165, Noam Postavsky

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

Drew Adams <drew.adams@oracle.com> schrieb am Mo., 6. Nov. 2017 um
15:41 Uhr:

> > >> We should perhaps put something about throwing error on '&option
> &rest'
> > >> into NEWS though.
> > >
> > > I don't understand.  In Common Lisp it is perfectly correct
> > > to use both &optional and &rest.
> >
> > What's rejected is (&optional &rest other-vars), whereas (&optional
> > var1 &rest other-vars) is okay. Does CL accept the first form (and if
> > yes, what does it mean)? I couldn't tell from the page you linked to.
>
> CL accepts a single variable after &rest. And there must be
> a variable after &optional.  (&optional foo &rest bar) is OK.
>
> (&optional &rest foo) is not OK.
> (&optional foo &rest bar toto titi) is not OK.
>

That should match the current behavior in Emacs Lisp now.

[-- Attachment #2: Type: text/html, Size: 1246 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  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:31           ` Drew Adams
  1 sibling, 2 replies; 30+ messages in thread
From: Ken Raeburn @ 2017-11-06 17:25 UTC (permalink / raw)
  To: Drew Adams; +Cc: Philipp Stephani, 29165, Noam Postavsky


On Nov 6, 2017, at 09:40, Drew Adams <drew.adams@oracle.com> wrote:

>>>> We should perhaps put something about throwing error on '&option &rest'
>>>> into NEWS though.
>>> 
>>> I don't understand.  In Common Lisp it is perfectly correct
>>> to use both &optional and &rest.
>> 
>> What's rejected is (&optional &rest other-vars), whereas (&optional
>> var1 &rest other-vars) is okay. Does CL accept the first form (and if
>> yes, what does it mean)? I couldn't tell from the page you linked to.
> 
> CL accepts a single variable after &rest. And there must be
> a variable after &optional.  (&optional foo &rest bar) is OK.
> 
> (&optional &rest foo) is not OK.
> (&optional foo &rest bar toto titi) is not OK.

Is this CL in general or a particular CL implementation? The web page you sent the URL for earlier reads like a specification, and from its use of “*” looks to me like it allows the (admittedly useless) form of &optional with no variables.

Ken




^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  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:31           ` Drew Adams
  1 sibling, 1 reply; 30+ messages in thread
From: Andreas Schwab @ 2017-11-06 18:10 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Philipp Stephani, 29165, Noam Postavsky

On Nov 06 2017, Ken Raeburn <raeburn@permabit.com> wrote:

> On Nov 6, 2017, at 09:40, Drew Adams <drew.adams@oracle.com> wrote:
>
>>>>> We should perhaps put something about throwing error on '&option &rest'
>>>>> into NEWS though.
>>>> 
>>>> I don't understand.  In Common Lisp it is perfectly correct
>>>> to use both &optional and &rest.
>>> 
>>> What's rejected is (&optional &rest other-vars), whereas (&optional
>>> var1 &rest other-vars) is okay. Does CL accept the first form (and if
>>> yes, what does it mean)? I couldn't tell from the page you linked to.
>> 
>> CL accepts a single variable after &rest. And there must be
>> a variable after &optional.  (&optional foo &rest bar) is OK.
>> 
>> (&optional &rest foo) is not OK.
>> (&optional foo &rest bar toto titi) is not OK.
>
> Is this CL in general or a particular CL implementation? The web page you sent the URL for earlier reads like a specification, and from its use of “*” looks to me like it allows the (admittedly useless) form of &optional with no variables.

clisp accepts it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 18:10           ` Andreas Schwab
@ 2017-11-06 19:10             ` Ken Raeburn
  2017-11-06 19:16               ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Ken Raeburn @ 2017-11-06 19:10 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Philipp Stephani, 29165, Noam Postavsky

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


On Nov 6, 2017, at 13:10, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Nov 06 2017, Ken Raeburn <raeburn@permabit.com <mailto:raeburn@permabit.com>> wrote:
> 
>> On Nov 6, 2017, at 09:40, Drew Adams <drew.adams@oracle.com> wrote:
>> 
>>>>>> We should perhaps put something about throwing error on '&option &rest'
>>>>>> into NEWS though.
>>>>> 
>>>>> I don't understand.  In Common Lisp it is perfectly correct
>>>>> to use both &optional and &rest.
>>>> 
>>>> What's rejected is (&optional &rest other-vars), whereas (&optional
>>>> var1 &rest other-vars) is okay. Does CL accept the first form (and if
>>>> yes, what does it mean)? I couldn't tell from the page you linked to.
>>> 
>>> CL accepts a single variable after &rest. And there must be
>>> a variable after &optional.  (&optional foo &rest bar) is OK.
>>> 
>>> (&optional &rest foo) is not OK.
>>> (&optional foo &rest bar toto titi) is not OK.
>> 
>> Is this CL in general or a particular CL implementation? The web page you sent the URL for earlier reads like a specification, and from its use of “*” looks to me like it allows the (admittedly useless) form of &optional with no variables.
> 
> clisp accepts it.

It appears that the emacs-26 version of defun* is happy with it (the original Lisp code I posted, using &optional &key) as well, as long as I provide the source, or a byte-compiled file from Emacs 25 or 26; it’s the .elc file generated by the older Emacs that’s causing me a problem. The (new?) checks are incompatible with the the old compiled file, even though the Lisp code itself *appears* to be acceptable still.

[-- Attachment #2: Type: text/html, Size: 5380 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 19:10             ` Ken Raeburn
@ 2017-11-06 19:16               ` Noam Postavsky
  2017-11-13 18:06                 ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-11-06 19:16 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165

On Mon, Nov 6, 2017 at 2:10 PM, Ken Raeburn <raeburn@permabit.com> wrote:

> It appears that the emacs-26 version of defun* is happy with it (the
> original Lisp code I posted, using &optional &key) as well, as long as I
> provide the source, or a byte-compiled file from Emacs 25 or 26; it’s the
> .elc file generated by the older Emacs that’s causing me a problem. The
> (new?) checks are incompatible with the the old compiled file, even though
> the Lisp code itself *appears* to be acceptable still.

It looks like the cl-defun in newer Emacs throws away the &optional
for you in this case.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 17:25         ` Ken Raeburn
  2017-11-06 18:10           ` Andreas Schwab
@ 2017-11-06 19:31           ` Drew Adams
  1 sibling, 0 replies; 30+ messages in thread
From: Drew Adams @ 2017-11-06 19:31 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Philipp Stephani, 29165, Noam Postavsky

> > CL accepts a single variable after &rest. And there must be
> > a variable after &optional.  (&optional foo &rest bar) is OK.
> >
> > (&optional &rest foo) is not OK.
> > (&optional foo &rest bar toto titi) is not OK.
> 
> Is this CL in general or a particular CL implementation? The web page you
> sent the URL for earlier reads like a specification, and from its use of
> “*” looks to me like it allows the (admittedly useless) form of &optional
> with no variables.

What I wrote was from memory.  What counts is the definition
of CL.  If it doesn't forbid such constructions then an
implementation or emulation) of CL is free to support them.

If it doesn't define them then an implementation is free
to define them any way it wants.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-06 19:16               ` Noam Postavsky
@ 2017-11-13 18:06                 ` Noam Postavsky
  2017-11-13 19:42                   ` Ken Raeburn
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-11-13 18:06 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165

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

On Mon, Nov 6, 2017 at 2:16 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Mon, Nov 6, 2017 at 2:10 PM, Ken Raeburn <raeburn@permabit.com> wrote:
>
>> It appears that the emacs-26 version of defun* is happy with it (the
>> original Lisp code I posted, using &optional &key) as well, as long as I
>> provide the source, or a byte-compiled file from Emacs 25 or 26
>
> It looks like the cl-defun in newer Emacs throws away the &optional
> for you in this case.

I think we should make cl-defun reject this kind of code, to be
consistent with plain defun. See attached.

[-- Attachment #2: v1-0001-Mention-new-strictness-for-optional-rest-in-argli.patch --]
[-- Type: application/octet-stream, Size: 4689 bytes --]

From 82af67b84a663fa750b375e483d4b3e8e1f26a6e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 13 Nov 2017 12:46:13 -0500
Subject: [PATCH v1] Mention new strictness for &optional, &rest in arglists
 (Bug#29165)

* etc/NEWS: Explain that '&optional' not followed by a variable is now
an error.
* lisp/emacs-lisp/cl-macs.el (cl--transform-lambda)
(cl--do-arglist): Also reject '&optional' or '&rest' not followed by a
variable for consistency.
* test/lisp/emacs-lisp/cl-macs-tests.el (cl-macs-bad-arglist): New
test.
---
 etc/NEWS                              | 11 +++++++++++
 lisp/emacs-lisp/cl-macs.el            | 16 +++++++++++-----
 test/lisp/emacs-lisp/cl-macs-tests.el | 25 +++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f79c2cb..b7af6e0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1468,6 +1468,17 @@ them through 'format' first.  Even that is discouraged: for ElDoc
 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:
+
+    (defun foo (&optional &rest x))
+    (defun bar (&optional &optional x))
+
+Previously, Emacs would just ignore the extra keyword, or give
+incorrect results in certain cases.
+
 \f
 * Lisp Changes in Emacs 26.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index e313af2..fccf6da 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -281,8 +281,13 @@ cl--transform-lambda
                   (or (not optional)
                       ;; Optional args whose default is nil are simple.
                       (null (nth 1 (assq (car args) (cdr cl--bind-defs)))))
-                  (not (and (eq (car args) '&optional) (setq optional t)
-                            (car cl--bind-defs))))
+                  (not (and (eq (car args) '&optional)
+                            (progn
+                              (when (memq (cadr args)
+                                          '(nil &rest &body &key &aux))
+                                (error "Variable missing after &optional"))
+                              (setq optional t)
+                              (car cl--bind-defs)))))
         (push (pop args) simple-args))
       (when optional
         (if args (push '&optional args))
@@ -558,9 +563,10 @@ cl--do-arglist
 	  (keys nil)
 	  (laterarg nil) (exactarg nil) minarg)
       (or num (setq num 0))
-      (setq restarg (if (listp (cadr restarg))
-                        (make-symbol "--cl-rest--")
-                      (cadr restarg)))
+      (setq restarg (or (if (consp (cadr restarg))
+                            (make-symbol "--cl-rest--")
+                          (cadr restarg))
+                        (error "Variable missing after &rest")))
       (push (list restarg expr) cl--bind-lets)
       (if (eq (car args) '&whole)
 	  (push (list (cl--pop2 args) restarg) cl--bind-lets))
diff --git a/test/lisp/emacs-lisp/cl-macs-tests.el b/test/lisp/emacs-lisp/cl-macs-tests.el
index 575f170..e43ff23 100644
--- a/test/lisp/emacs-lisp/cl-macs-tests.el
+++ b/test/lisp/emacs-lisp/cl-macs-tests.el
@@ -497,4 +497,29 @@
                           vconcat (vector (1+ x)))
                  [2 3 4 5 6])))
 
+\f
+;;; cl-lib lambda list handling
+
+(ert-deftest cl-macs-bad-arglist ()
+  "Check that `cl-defun' and friends reject weird argument lists.
+See Bug#29165, and similar `eval-tests--bugs-24912-and-24913' in
+eval-tests.el."
+  (dolist (args (cl-mapcan
+                 ;; For every &rest variant, check also the same thing
+                 ;; with &key instead.
+                 (lambda (arglist) (if (memq '&rest arglist)
+                                  (list arglist (cl-subst '&key '&rest arglist))
+                                (list arglist)))
+                 '((&optional) (&rest) (&optional &rest) (&rest &optional)
+                   (&optional &rest _a) (&optional _a &rest)
+                   (&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 (cl-function (lambda ,args))) t))
+    (should-error (cl--transform-lambda (cons args t)))
+    (let ((byte-compile-debug t))
+      (should-error (eval `(byte-compile (cl-function (lambda ,args))) t)))))
+
 ;;; cl-macs-tests.el ends here
-- 
2.6.2.windows.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-13 18:06                 ` Noam Postavsky
@ 2017-11-13 19:42                   ` Ken Raeburn
  2017-11-13 20:05                     ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Ken Raeburn @ 2017-11-13 19:42 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Andreas Schwab, Philipp Stephani, 29165


On Nov 13, 2017, at 13:06, Noam Postavsky <npostavs@users.sourceforge.net> wrote:

> On Mon, Nov 6, 2017 at 2:16 PM, Noam Postavsky
> <npostavs@users.sourceforge.net> wrote:
>> On Mon, Nov 6, 2017 at 2:10 PM, Ken Raeburn <raeburn@permabit.com> wrote:
>> 
>>> It appears that the emacs-26 version of defun* is happy with it (the
>>> original Lisp code I posted, using &optional &key) as well, as long as I
>>> provide the source, or a byte-compiled file from Emacs 25 or 26
>> 
>> It looks like the cl-defun in newer Emacs throws away the &optional
>> for you in this case.
> 
> I think we should make cl-defun reject this kind of code, to be
> consistent with plain defun. See attached.

I’m of two minds about it… it’s a useless but harmless degenerate case, and based on the link Drew posted and the test Andreas did, making it an error would be a gratuitous incompatibility with CL or at least one implementation.  And maybe it’s not entirely useless if it simplifies someone’s macro so that they can treat zero-or-more optional variables with a single, simple common code path.  But even if we do make it an error, isn’t there usually a stage where it’s just a warning?

(And if we’re going to make that sort of thing an error, we should probably check whether empty &key or &aux variable lists are similarly rejected.  I haven’t looked.)

Also, what the CL macros do going forward is arguably a separate question from whether byte-code processing should reject the byte code generated for the same construct by older releases.  That’s what we’ve got now — Emacs 26.0.90 accepts the forms for CL macros but rejects them in byte code, and Emacs 24 CL macros produced such forms in byte code when given such forms as input.  So currently-acceptable code, or at least code still treated as acceptable under Emacs 25, when compiled by an older release, is no longer accepted. If the source isn’t going to be rejected (e.g., if it’s quietly accepted or only produces a warning), then the byte-code for it probably ought not to be rejected.

Ken




^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-13 19:42                   ` Ken Raeburn
@ 2017-11-13 20:05                     ` Noam Postavsky
  2017-11-27 22:24                       ` Noam Postavsky
  2017-12-13 23:39                       ` Stefan Monnier
  0 siblings, 2 replies; 30+ messages in thread
From: Noam Postavsky @ 2017-11-13 20:05 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165

On Mon, Nov 13, 2017 at 2:42 PM, Ken Raeburn <raeburn@permabit.com> wrote:

> But even if we do make it an error, isn’t there usually a stage where it’s just a warning?

Maybe. There hasn't been this time (for plain defun, I mean).

> (And if we’re going to make that sort of thing an error, we should probably check whether empty &key or &aux variable lists are similarly rejected.  I haven’t looked.)

I believe empty &key would be tested in my patch, though not &aux.

> If the source isn’t going to be rejected (e.g., if it’s quietly accepted or only produces a warning), then the byte-code for it probably ought not to be rejected.

Yes, that's why my patch rejects the source as well.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-13 20:05                     ` Noam Postavsky
@ 2017-11-27 22:24                       ` Noam Postavsky
  2017-12-13 22:36                         ` Noam Postavsky
  2017-12-13 23:39                       ` Stefan Monnier
  1 sibling, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-11-27 22:24 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165

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

On Mon, Nov 13, 2017 at 3:05 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Mon, Nov 13, 2017 at 2:42 PM, Ken Raeburn <raeburn@permabit.com> wrote:
>
>> But even if we do make it an error, isn’t there usually a stage where it’s just a warning?
>
> Maybe. There hasn't been this time (for plain defun, I mean).

As another case, there wasn't any warning stage for changing setq to
only accept an even number of arguments.

>> (And if we’re going to make that sort of thing an error, we should probably check whether empty &key or &aux variable lists are similarly rejected.  I haven’t looked.)
>
> I believe empty &key would be tested in my patch, though not &aux.

Updated patch which handles &aux as well. I also tested a bootstrap
(doing this I found the previous patch messed up some positive cases).

[-- Attachment #2: v2-0001-Mention-new-strictness-for-optional-rest-in-argli.patch --]
[-- Type: application/octet-stream, Size: 6603 bytes --]

From 31615e2acf6e44e7e1d84b88e53db7751281d2bf Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 13 Nov 2017 12:46:13 -0500
Subject: [PATCH v2] Mention new strictness for &optional, &rest in arglists
 (Bug#29165)

* etc/NEWS: Explain that '&optional' not followed by a variable is now
an error.
* lisp/emacs-lisp/cl-macs.el (cl--transform-lambda, cl--do-&aux)
(cl--do-arglist): Also reject '&optional', '&rest', or '&aux' not
followed by a variable for consistency.
* test/lisp/emacs-lisp/cl-macs-tests.el (cl-macs-bad-arglist): New
test.
---
 etc/NEWS                              | 11 ++++++++++
 lisp/emacs-lisp/cl-macs.el            | 38 +++++++++++++++++++++++++----------
 test/lisp/emacs-lisp/cl-macs-tests.el | 31 ++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f7a9feb..5b66661 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1462,6 +1462,17 @@ them through 'format' first.  Even that is discouraged: for ElDoc
 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:
+
+    (defun foo (&optional &rest x))
+    (defun bar (&optional &optional x))
+
+Previously, Emacs would just ignore the extra keyword, or give
+incorrect results in certain cases.
+
 \f
 * Lisp Changes in Emacs 26.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 5535100..6aed060 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -281,8 +281,13 @@ cl--transform-lambda
                   (or (not optional)
                       ;; Optional args whose default is nil are simple.
                       (null (nth 1 (assq (car args) (cdr cl--bind-defs)))))
-                  (not (and (eq (car args) '&optional) (setq optional t)
-                            (car cl--bind-defs))))
+                  (not (and (eq (car args) '&optional)
+                            (progn
+                              (when (memq (cadr args)
+                                          '(nil &rest &body &key &aux))
+                                (error "Variable missing after &optional"))
+                              (setq optional t)
+                              (car cl--bind-defs)))))
         (push (pop args) simple-args))
       (when optional
         (if args (push '&optional args))
@@ -534,14 +539,17 @@ cl--make-usage-args
               arglist))))
 
 (defun cl--do-&aux (args)
-  (while (and (eq (car args) '&aux) (pop args))
-    (while (and args (not (memq (car args) cl--lambda-list-keywords)))
-      (if (consp (car args))
-          (if (and cl--bind-enquote (cl-cadar args))
-              (cl--do-arglist (caar args)
-                              `',(cadr (pop args)))
-            (cl--do-arglist (caar args) (cadr (pop args))))
-        (cl--do-arglist (pop args) nil))))
+  (when (eq (car args) '&aux)
+    (pop args)
+    (when (null args)
+      (error "Variable missing after &aux")))
+  (while (and args (not (memq (car args) cl--lambda-list-keywords)))
+    (if (consp (car args))
+        (if (and cl--bind-enquote (cl-cadar args))
+            (cl--do-arglist (caar args)
+                            `',(cadr (pop args)))
+          (cl--do-arglist (caar args) (cadr (pop args))))
+      (cl--do-arglist (pop args) nil)))
   (if args (error "Malformed argument list ends with: %S" args)))
 
 (defun cl--do-arglist (args expr &optional num)   ; uses cl--bind-*
@@ -558,6 +566,9 @@ cl--do-arglist
 	  (keys nil)
 	  (laterarg nil) (exactarg nil) minarg)
       (or num (setq num 0))
+      (when (and restarg (or (null (cdr restarg))
+                             (memq (cadr restarg) cl--lambda-list-keywords)))
+        (error "Variable missing after &rest"))
       (setq restarg (if (listp (cadr restarg))
                         (make-symbol "--cl-rest--")
                       (cadr restarg)))
@@ -609,7 +620,12 @@ cl--do-arglist
                                       `',cl--bind-block)
                                 (+ ,num (length ,restarg)))))
                   cl--bind-forms)))
-      (while (and (eq (car args) '&key) (pop args))
+      (while (eq (car args) '&key)
+        (pop args)
+        (when (or (null args) (memq (car args) cl--lambda-list-keywords))
+          (error "Missing variable after &key"))
+        (when keys
+          (error "Multiple occurrences of &key"))
 	(while (and args (not (memq (car args) cl--lambda-list-keywords)))
 	  (let ((arg (pop args)))
 	    (or (consp arg) (setq arg (list arg)))
diff --git a/test/lisp/emacs-lisp/cl-macs-tests.el b/test/lisp/emacs-lisp/cl-macs-tests.el
index 575f170..bf2e7e1 100644
--- a/test/lisp/emacs-lisp/cl-macs-tests.el
+++ b/test/lisp/emacs-lisp/cl-macs-tests.el
@@ -497,4 +497,35 @@
                           vconcat (vector (1+ x)))
                  [2 3 4 5 6])))
 
+\f
+;;; cl-lib lambda list handling
+
+(ert-deftest cl-macs-bad-arglist ()
+  "Check that `cl-defun' and friends reject weird argument lists.
+See Bug#29165, and similar `eval-tests--bugs-24912-and-24913' in
+eval-tests.el."
+  (dolist (args (cl-mapcan
+                 ;; For every &rest and &optional variant, check also
+                 ;; the same thing with &key and &aux respectively
+                 ;; instead.
+                 (lambda (arglist)
+                   (let ((arglists (list arglist)))
+                     (when (memq '&rest arglist)
+                       (push (cl-subst '&key '&rest arglist) arglists))
+                     (when (memq '&optional arglist)
+                       (push (cl-subst '&aux '&optional arglist) arglists))
+                     arglists))
+                 '((&optional) (&rest) (&optional &rest) (&rest &optional)
+                   (&optional &rest _a) (&optional _a &rest)
+                   (&rest _a &optional) (&rest &optional _a)
+                   (&optional &optional) (&optional &optional _a)
+                   (&optional _a &optional _b)
+                   (&rest &rest) (&rest &rest _a)
+                   (&rest _a &rest _b))))
+    (ert-info ((prin1-to-string args) :prefix "arglist: ")
+      (should-error (eval `(funcall (cl-function (lambda ,args))) t))
+      (should-error (cl--transform-lambda (cons args t)))
+      (let ((byte-compile-debug t))
+        (should-error (eval `(byte-compile (cl-function (lambda ,args))) t))))))
+
 ;;; cl-macs-tests.el ends here
-- 
2.6.2.windows.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-27 22:24                       ` Noam Postavsky
@ 2017-12-13 22:36                         ` Noam Postavsky
  2017-12-15 16:48                           ` Glenn Morris
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-12-13 22:36 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165

close 29165 26.1
quit

On Mon, Nov 27, 2017 at 5:24 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Mon, Nov 13, 2017 at 3:05 PM, Noam Postavsky
> <npostavs@users.sourceforge.net> wrote:
>> On Mon, Nov 13, 2017 at 2:42 PM, Ken Raeburn <raeburn@permabit.com> wrote:
>>
>>> But even if we do make it an error, isn’t there usually a stage where it’s just a warning?
>>
>> Maybe. There hasn't been this time (for plain defun, I mean).
>
> As another case, there wasn't any warning stage for changing setq to
> only accept an even number of arguments.
>
>>> (And if we’re going to make that sort of thing an error, we should probably check whether empty &key or &aux variable lists are similarly rejected.  I haven’t looked.)
>>
>> I believe empty &key would be tested in my patch, though not &aux.
>
> Updated patch which handles &aux as well. I also tested a bootstrap
> (doing this I found the previous patch messed up some positive cases).

Pushed to emacs-26.

[1: e7b1111]: 2017-12-13 17:31:27 -0500
  Mention new strictness for &optional, &rest in arglists (Bug#29165)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e7b1111155b3116d0c7b137e0e1d312db0f1ca80





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-11-13 20:05                     ` Noam Postavsky
  2017-11-27 22:24                       ` Noam Postavsky
@ 2017-12-13 23:39                       ` Stefan Monnier
  2017-12-15  1:16                         ` Noam Postavsky
  1 sibling, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2017-12-13 23:39 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Ken Raeburn, 29165, Philipp Stephani, Andreas Schwab

>> (And if we’re going to make that sort of thing an error, we should
>> probably check whether empty &key or &aux variable lists are similarly
>> rejected.  I haven’t looked.)

I recently installed a patch to fix/improve the behavior of &aux with no
keyword variable (I bumped into it while working on some Elisp package,
tho I can't remember which right now).

I think it's usually worth the small extra effort to support &optional
not followed by any var (as well as &aux not followed by any var) since
it sometimes comes in handy.  But not if it costs extra at run-time.

> Updated patch which handles &aux as well.  I also tested a bootstrap
> (doing this I found the previous patch messed up some positive cases).

To the extent that &aux is only handled by macro-expansion, accepting an
empty &aux never costs anything at run-time, so I think rejecting it is
a disservice to our users.


        Stefan





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-13 23:39                       ` Stefan Monnier
@ 2017-12-15  1:16                         ` Noam Postavsky
  2017-12-15  3:04                           ` Stefan Monnier
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-12-15  1:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Ken Raeburn, Andreas Schwab, Philipp Stephani, 29165

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> (And if we’re going to make that sort of thing an error, we should
>>> probably check whether empty &key or &aux variable lists are similarly
>>> rejected.  I haven’t looked.)
>
> I recently installed a patch to fix/improve the behavior of &aux with no
> keyword variable (I bumped into it while working on some Elisp package,
> tho I can't remember which right now).
>
> I think it's usually worth the small extra effort to support &optional
> not followed by any var (as well as &aux not followed by any var) since
> it sometimes comes in handy.  But not if it costs extra at run-time.

I don't think there is any performance penalty (when running
byte-compiled code anyway).  Although I see that &optional at the end of
the arglist has been a compile error for a long time (I tested back to
24.3).  E.g., (defun foo (&optional)) fails to compile.  (defun foo
(&optional &rest)) happened to work, though I wouldn't exactly call that
"support".

>> Updated patch which handles &aux as well.  I also tested a bootstrap
>> (doing this I found the previous patch messed up some positive cases).
>
> To the extent that &aux is only handled by macro-expansion, accepting an
> empty &aux never costs anything at run-time, so I think rejecting it is
> a disservice to our users.

I don't see the use case for empty &aux, but I don't mind reverting that
change.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-15  1:16                         ` Noam Postavsky
@ 2017-12-15  3:04                           ` Stefan Monnier
  2017-12-15  5:17                             ` Ken Raeburn
  0 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2017-12-15  3:04 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Ken Raeburn, Andreas Schwab, Philipp Stephani, 29165

> I don't see the use case for empty &aux, but I don't mind reverting that
> change.

It's typically when you define a function from within a macro so you may
have something like

   `(...
     (cl-defun ,foo (,@args &aux ,@auxargs)
       ,@body)
     ...)

where `auxargs` can have any length including 0.  It's not
terribly hard to strip the &aux when `auxargs` is empty, but it's
convenient to be able to use the above form without having to worry
about a possibly empty `auxargs`.


        Stefan





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-15  3:04                           ` Stefan Monnier
@ 2017-12-15  5:17                             ` Ken Raeburn
  2017-12-15 13:54                               ` Stefan Monnier
  0 siblings, 1 reply; 30+ messages in thread
From: Ken Raeburn @ 2017-12-15  5:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Andreas Schwab, Philipp Stephani, 29165, Noam Postavsky


On Dec 14, 2017, at 22:04, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> I don't see the use case for empty &aux, but I don't mind reverting that
>> change.
> 
> It's typically when you define a function from within a macro so you may
> have something like
[…]

Why is this different from "… &optional ,@optargs” in a similar form, when optargs could have no elements?  Rejecting that was (part of) the point of Noam’s change (and the previous change for bug 24913).  I think I’ve already made my objection to that part plain, but it seems like rejecting an empty &optional and accepting an empty &aux would be inconsistent.

Ken




^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-15  5:17                             ` Ken Raeburn
@ 2017-12-15 13:54                               ` Stefan Monnier
  0 siblings, 0 replies; 30+ messages in thread
From: Stefan Monnier @ 2017-12-15 13:54 UTC (permalink / raw)
  To: Ken Raeburn; +Cc: Andreas Schwab, Philipp Stephani, 29165, Noam Postavsky

> Why is this different from "… &optional ,@optargs” in a similar form, when
> optargs could have no elements?  Rejecting that was (part of) the point of
> Noam’s change (and the previous change for bug 24913).

It's not, and I also think rejecting it is generally unadvisable unless
it has a performance impact.

> I think I’ve already made my objection to that part plain, but it
> seems like rejecting an empty &optional and accepting an empty &aux
> would be inconsistent.

Inconsistency is part of life, tho.


        Stefan





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-13 22:36                         ` Noam Postavsky
@ 2017-12-15 16:48                           ` Glenn Morris
  2017-12-16  4:54                             ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Glenn Morris @ 2017-12-15 16:48 UTC (permalink / raw)
  To: Noam Postavsky
  Cc: Ken Raeburn, Andreas Schwab, Philipp Stephani, 29165,
	Stefan Monnier

Noam Postavsky wrote:

> Pushed to emacs-26.
>
> [1: e7b1111]: 2017-12-13 17:31:27 -0500
>   Mention new strictness for &optional, &rest in arglists (Bug#29165)
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e7b1111155b3116d0c7b137e0e1d312db0f1ca80

When attempting to merge emacs-26 to master, this conflicts with

commit cea0bca54f
Date:   Mon Nov 27 12:45:16 2017 -0500

    * lisp/emacs-lisp/cl-macs.el: Fix &key with no key arg

Please could someone indicate how this is to be resolved.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-15 16:48                           ` Glenn Morris
@ 2017-12-16  4:54                             ` Noam Postavsky
  2018-01-20 22:10                               ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2017-12-16  4:54 UTC (permalink / raw)
  To: Glenn Morris
  Cc: Ken Raeburn, 29165, Philipp Stephani, Andreas Schwab,
	Stefan Monnier

reopen 29165
quit

Glenn Morris <rgm@gnu.org> writes:

> Noam Postavsky wrote:
>
>> Pushed to emacs-26.
>>
>> [1: e7b1111]: 2017-12-13 17:31:27 -0500
>>   Mention new strictness for &optional, &rest in arglists (Bug#29165)
>>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e7b1111155b3116d0c7b137e0e1d312db0f1ca80
>
> When attempting to merge emacs-26 to master, this conflicts with
>
> commit cea0bca54f
> Date:   Mon Nov 27 12:45:16 2017 -0500
>
>     * lisp/emacs-lisp/cl-macs.el: Fix &key with no key arg
>
> Please could someone indicate how this is to be resolved.

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.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2017-12-16  4:54                             ` Noam Postavsky
@ 2018-01-20 22:10                               ` Noam Postavsky
  2018-01-21  3:02                                 ` Stefan Monnier
  2018-01-21 16:04                                 ` Eli Zaretskii
  0 siblings, 2 replies; 30+ messages in thread
From: Noam Postavsky @ 2018-01-20 22:10 UTC (permalink / raw)
  To: Glenn Morris
  Cc: Ken Raeburn, Andreas Schwab, Philipp Stephani, 29165,
	Stefan Monnier

[-- 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


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2018-01-20 22:10                               ` Noam Postavsky
@ 2018-01-21  3:02                                 ` Stefan Monnier
  2018-01-21 16:04                                 ` Eli Zaretskii
  1 sibling, 0 replies; 30+ messages in thread
From: Stefan Monnier @ 2018-01-21  3:02 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Ken Raeburn, Philipp Stephani, Andreas Schwab, 29165

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

LGTM,


        Stefan





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2018-01-20 22:10                               ` Noam Postavsky
  2018-01-21  3:02                                 ` Stefan Monnier
@ 2018-01-21 16:04                                 ` Eli Zaretskii
  2018-01-21 16:30                                   ` Noam Postavsky
  1 sibling, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2018-01-21 16:04 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 29165, raeburn, p.stephani2, schwab, monnier

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 20 Jan 2018 17:10:17 -0500
> 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>
> 
> 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.

Is it important enough to justify one more pretest, after 26.0.91, and
delay the release by a few more weeks?  Because that's what would be
needed if we install this on the release branch.

Besides, this change was introduced more than a year ago, and we did
that in clear understanding that it changes behavior.  So "closer to
Emacs 25" doesn't sound like a convincing argument in this case,
because we gave up that similarity up front.

Your call.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2018-01-21 16:04                                 ` Eli Zaretskii
@ 2018-01-21 16:30                                   ` Noam Postavsky
  2018-01-21 18:01                                     ` Eli Zaretskii
  0 siblings, 1 reply; 30+ messages in thread
From: Noam Postavsky @ 2018-01-21 16:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 29165, raeburn, p.stephani2, schwab, monnier

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Sat, 20 Jan 2018 17:10:17 -0500
>> 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>
>> 
>> 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.
>
> Is it important enough to justify one more pretest, after 26.0.91, and
> delay the release by a few more weeks?  Because that's what would be
> needed if we install this on the release branch.

Hmm, no, I think these edge cases are not important enough to delay the
release.

> Besides, this change was introduced more than a year ago, and we did
> that in clear understanding that it changes behavior.  So "closer to
> Emacs 25" doesn't sound like a convincing argument in this case,
> because we gave up that similarity up front.

Well, the unfortunate thing about waiting is that we get more strict in
v26 and then turn around in v27 and (partially) reverse the decision.
But still, as I said above, not worth delaying the release over it.






^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2018-01-21 16:30                                   ` Noam Postavsky
@ 2018-01-21 18:01                                     ` Eli Zaretskii
  2018-03-25 15:32                                       ` Noam Postavsky
  0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2018-01-21 18:01 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 29165, raeburn, p.stephani2, schwab, monnier

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: rgm@gnu.org,  29165@debbugs.gnu.org,  raeburn@permabit.com,  p.stephani2@gmail.com,  schwab@linux-m68k.org,  monnier@iro.umontreal.ca
> Date: Sun, 21 Jan 2018 11:30:02 -0500
> 
> > Is it important enough to justify one more pretest, after 26.0.91, and
> > delay the release by a few more weeks?  Because that's what would be
> > needed if we install this on the release branch.
> 
> Hmm, no, I think these edge cases are not important enough to delay the
> release.
> 
> > Besides, this change was introduced more than a year ago, and we did
> > that in clear understanding that it changes behavior.  So "closer to
> > Emacs 25" doesn't sound like a convincing argument in this case,
> > because we gave up that similarity up front.
> 
> Well, the unfortunate thing about waiting is that we get more strict in
> v26 and then turn around in v27 and (partially) reverse the decision.
> But still, as I said above, not worth delaying the release over it.

OK, then please push to master, and thanks.





^ permalink raw reply	[flat|nested] 30+ messages in thread

* bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24
  2018-01-21 18:01                                     ` Eli Zaretskii
@ 2018-03-25 15:32                                       ` Noam Postavsky
  0 siblings, 0 replies; 30+ messages in thread
From: Noam Postavsky @ 2018-03-25 15:32 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: 29165, Noam Postavsky, raeburn, p.stephani2, schwab, monnier

tags 29165 fixed
close 29165 27.1
quit

Eli Zaretskii <eliz@gnu.org> writes:
> OK, then please push to master, and thanks.

Right, done (finally).

[1: 1d47d777ef]: 2018-03-25 07:56:35 -0400
  Allow `&rest' or `&optional' without following variable (Bug#29165)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1d47d777ef24c0be9153b0a1c8ba21918fa1025a





^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-03-25 15:32 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

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.