bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Ken Raeburn]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Drew Adams]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Drew Adams]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Eli Zaretskii]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Philipp Stephani]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Ken Raeburn]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Andreas Schwab]

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."

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Ken Raeburn]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Drew Adams]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Ken Raeburn]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Stefan Monnier]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Stefan Monnier]

> 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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Ken Raeburn]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Stefan Monnier]

> 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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Glenn Morris]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Stefan Monnier]

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

LGTM,


        Stefan

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Eli Zaretskii]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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.

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Eli Zaretskii]

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

bug#29165: 26.0.90; can't use some code byte-compiled under emacs 24

[Noam Postavsky]

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