bug#24255: 25.1; incorrect edebug spec for cl-defun

bug#24255: 25.1; incorrect edebug spec for cl-defun

[Ben Liblit]

The edebug specification for cl-defun seems to be incorrect.  I believe
it does not properly cover cases where cl-defun is used for
destructuring along the lines of cl-destructuring-bind, such as in:

	(require 'cl-lib)
	(cl-defun sum ((x y)) (+ x y))

Notice that the argument list to sum is performing a destructuring
bind.  Thus, one would call this using something like "(sum '(1 2))".

To see the bug, proceed as follows:

(1) Start emacs using "emacs -Q".

(2) Place the two-line example given above in an Emacs Lisp buffer.

(3) Type "M-x eval-buffer".  Observe that there is no error.

(4) Type "M-x edebug-all-defs".

(5) Type "M-x eval-buffer" again.  Observe the following error:

	edebug-syntax-error: Invalid read syntax: "Failed matching",
	([&rest arg] [&optional ["&optional" cl-&optional-arg &rest
	cl-&optional-arg]] [&optional ["&rest" arg]] [&optional ["&key"
	[cl-&key-arg &rest cl-&key-arg] &optional "&allow-other-keys"]]
	[&optional ["&aux" &rest &or (symbolp &optional def-form)
	symbolp]])

Destructuring in cl-defun is certainly valid.  You can see this in the
documentation for cl-defun, which notes that "VAR maybe be replaced
recursively with an argument list for destructing".  The info
documentation for cl also states that "In Common Lisp, destructuring is
only allowed with 'defmacro'; this package allows it with 'cl-defun' and
other argument lists as well."

I am reporting this bug against Emacs 25.1 rc1.  However, I see the same
problem going at least as far back as Emacs 24.3.  You definitely do not
need to build the release candidate in order to reproduce this bug.



In GNU Emacs 25.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.13)
  of 2016-08-16 built on localhost.localdomain
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
System Description:	Red Hat Enterprise Linux Server release 7.2 (Maipo)

Configured using:
  'configure --with-dbus --with-gif --with-jpeg --with-png --with-rsvg
  --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
  --with-xwidgets --prefix=/u/l/i/liblit/Downloads/emacs-25.1-rc1'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GCONF GSETTINGS NOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XWIDGETS

Important settings:
   value of $LC_CTYPE: en_US.UTF-8
   value of $LC_MONETARY: en_US.utf8
   value of $LC_NUMERIC: en_US.utf8
   value of $LC_TIME: en_US.utf8
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
   diff-auto-refine-mode: t
   tooltip-mode: t
   global-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

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Please type y, n, or !: y
Making completion list...
Edebugging all definitions is on.
edebug-syntax-error: Invalid read syntax: "Failed matching", ([&rest 
arg] [&optional ["&optional" cl-&optional-arg &rest cl-&optional-arg]] 
[&optional ["&rest" arg]] [&optional ["&key" [cl-&key-arg &rest 
cl-&key-arg] &optional "&allow-other-keys"]] [&optional ["&aux" &rest 
&or (symbolp &optional def-form) symbolp]])

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode mail-prsvr
mail-utils edebug cl-seq cl-macs gv cl-loaddefs pcase cl-lib vc-git
diff-mode easymenu easy-mmode time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core 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 charscript case-table epa-hook jka-cmpr-hook help
simple abbrev 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
xwidget-internal move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 95803 5203)
  (symbols 48 20828 0)
  (miscs 40 102 107)
  (strings 32 17346 4912)
  (string-bytes 1 501304)
  (vectors 16 14033)
  (vector-slots 8 453896 5184)
  (floats 8 172 26)
  (intervals 56 280 0)
  (buffers 976 21)
  (heap 1024 40970 1627))

bug#24255: Acknowledgement (25.1; incorrect edebug spec for cl-defun)

[Ben Liblit]

For the record, I'd like to note that this problem first came up in the 
context of "undercover.el": an Elisp coverage tool that builds upon 
edebug.  My original bug report against "undercover.el" can be found at 
<https://github.com/sviridov/undercover.el/issues/19>.  Sviridov 
Alexander deserves the credit for tracking this problem back to edebug 
and showing how to reproduce it easily.

bug#24255: 25.1; incorrect edebug spec for cl-defun

[Gemini Lasswell]

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

Here is a revised Edebug spec for cl-defun which supports
destructuring. I've included dotted destructuring since cl-defun
supports it, even though Edebug doesn't currently handle dotted
destructuring correctly due to bug#6415.


[-- Attachment #2: 0001-Fix-Edebug-spec-for-cl-defun-bug-24255.patch --]
[-- Type: text/plain, Size: 4141 bytes --]

From 09ff5fa398bc1061fd5a8ee1bb880f2d6f8035ba Mon Sep 17 00:00:00 2001
From: Gemini Lasswell <gazally@runbox.com>
Date: Fri, 6 Oct 2017 09:55:55 -0700
Subject: [PATCH] Fix Edebug spec for cl-defun (bug#24255)

* lisp/emacs-lisp/cl-macs.el: Modify the Edebug spec for
`cl-lambda-list' to support destructuring argument lists.
(cl-defun, cl-defmacro): Fix spelling errors in docstrings.
* lisp/textmodes/rst.el: Remove alternate version of Edebug
specs for `cl-lambda-list' and `cl-type-spec'.
---
 lisp/emacs-lisp/cl-macs.el | 32 +++++++++++++++++++++++---------
 lisp/textmodes/rst.el      | 16 ----------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 32ba0ac309..cb4c7ad93c 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -193,20 +193,34 @@ cl-declarations-or-string
   (&or stringp cl-declarations))
 
 (def-edebug-spec cl-lambda-list
-  (([&rest arg]
+  (([&rest cl-lambda-arg]
     [&optional ["&optional" cl-&optional-arg &rest cl-&optional-arg]]
-    [&optional ["&rest" arg]]
+    [&optional ["&rest" cl-lambda-arg]]
     [&optional ["&key" [cl-&key-arg &rest cl-&key-arg]
 		&optional "&allow-other-keys"]]
     [&optional ["&aux" &rest
 		&or (symbolp &optional def-form) symbolp]]
-    )))
+    . [&or arg nil])))
 
 (def-edebug-spec cl-&optional-arg
-  (&or (arg &optional def-form arg) arg))
+  (&or (cl-lambda-arg &optional def-form arg) arg))
 
 (def-edebug-spec cl-&key-arg
-  (&or ([&or (symbolp arg) arg] &optional def-form arg) arg))
+  (&or ([&or (symbolp cl-lambda-arg) arg] &optional def-form arg) arg))
+
+(def-edebug-spec cl-lambda-arg
+  (&or arg cl-lambda-list1))
+
+(def-edebug-spec cl-lambda-list1
+  (([&optional ["&whole" arg]]  ;; only allowed at lower levels
+    [&rest cl-lambda-arg]
+    [&optional ["&optional" cl-&optional-arg &rest cl-&optional-arg]]
+    [&optional ["&rest" cl-lambda-arg]]
+    [&optional ["&key" cl-&key-arg &rest cl-&key-arg
+                &optional "&allow-other-keys"]]
+    [&optional ["&aux" &rest
+                &or (symbolp &optional def-form) symbolp]]
+    . [&or arg nil])))
 
 (def-edebug-spec cl-type-spec sexp)
 
@@ -336,8 +350,8 @@ cl-defun
     [&key (([KEYWORD] VAR) [INITFORM [SVAR]])... [&allow-other-keys]]
     [&aux (VAR [INITFORM])...])
 
-VAR maybe be replaced recursively with an argument list for
-destructing, `&whole' is supported within these sublists.  If
+VAR may be replaced recursively with an argument list for
+destructuring, `&whole' is supported within these sublists.  If
 SVAR, INITFORM, and KEYWORD are all omitted, then `(VAR)' may be
 written simply `VAR'.  See the Info node `(cl)Argument Lists' for
 more details.
@@ -430,8 +444,8 @@ cl-defmacro
     [&aux (VAR [INITFORM])...]
     [&environment VAR])
 
-VAR maybe be replaced recursively with an argument list for
-destructing, `&whole' is supported within these sublists.  If
+VAR may be replaced recursively with an argument list for
+destructuring, `&whole' is supported within these sublists.  If
 SVAR, INITFORM, and KEYWORD are all omitted, then `(VAR)' may be
 written simply `VAR'.  See the Info node `(cl)Argument Lists' for
 more details.
diff --git a/lisp/textmodes/rst.el b/lisp/textmodes/rst.el
index 5534294738..404b8eb04d 100644
--- a/lisp/textmodes/rst.el
+++ b/lisp/textmodes/rst.el
@@ -109,22 +109,6 @@
 (def-edebug-spec push
   (&or [form symbolp] [form gv-place]))
 
-;; Correct wrong declaration. This still doesn't support dotted destructuring
-;; though.
-(def-edebug-spec cl-lambda-list
-  (([&rest cl-macro-arg]
-    [&optional ["&optional" cl-&optional-arg &rest cl-&optional-arg]]
-    [&optional ["&rest" arg]]
-    [&optional ["&key" [cl-&key-arg &rest cl-&key-arg]
-		&optional "&allow-other-keys"]]
-    [&optional ["&aux" &rest
-		&or (symbolp &optional def-form) symbolp]]
-    )))
-
-;; Add missing declaration.
-(def-edebug-spec cl-type-spec sexp) ;; This is not exactly correct but good
-				    ;; enough.
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Support for `testcover'
 
-- 
2.14.1

bug#24255: 25.1; incorrect edebug spec for cl-defun

[Gemini Lasswell]

I've pushed the patch to emacs-26.