bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Brandon Irizarry]

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

To reproduce: in emacs invoked with '-Q', in the Scratch buffer, insert the
following
code:

(require 'cl-macs)

(cl-flet ((fn ((min max))
     (message "%d %d" min max)))
  (fn '(2 3)))

The cl-flet form evaluates fine (e.g. with C-x C-e or C-M-x), which
leads me to conclude that this construct is legal. However, attempting
to instrument the form with Edebug (C-u C-M-x) results in an enormous
backtrace starting with

Debugger entered--Lisp error: (invalid-read-syntax "Expected" "lambda")

If I get rid of the argument destructuring, Edebug works as
expected. For example, I'm able to step through the following code with
Edebug just fine:

(cl-flet ((fn (min max)
     (message "%d %d" min max)))
  (fn 2 3))

=> "2 3"

I should be able to, for example, instrument the destructuring (buggy)
cl-flet form,
run through it with 'g', and see the same output.

In GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37,
cairo version 1.16.0)
 of 2023-05-13, modified by Debian built on x86-ubc-01
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/28.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/28.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/28.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/28.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --with-cairo --with-x=yes
 --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -ffile-prefix-map=/build/emacs-mPr7Vr/emacs-28.2+1=.
-fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  paredit-mode: t
  ws-butler-mode: t
  global-corfu-mode: t
  corfu-mode: t
  fido-vertical-mode: t
  icomplete-vertical-mode: t
  icomplete-mode: t
  fido-mode: t
  electric-pair-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-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
  window-divider-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t

Load-path shadows:
/home/demo/.emacs.d/elpa/transient-20230810.1716/transient hides
/usr/share/emacs/28.2/lisp/transient
/home/demo/.emacs.d/elpa/xref-1.6.3/xref hides
/usr/share/emacs/28.2/lisp/progmodes/xref
/home/demo/.emacs.d/elpa/project-0.9.8/project hides
/usr/share/emacs/28.2/lisp/progmodes/project
/home/demo/.emacs.d/elpa/eldoc-1.14.0/eldoc hides
/usr/share/emacs/28.2/lisp/emacs-lisp/eldoc

Features:
(shadow sort mail-extr emacsbug sendmail edebug cl-print help-fns
radix-tree org-element avl-tree generator ol-eww eww xdg url-queue
mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search
eieio-opt speedbar ezimage dframe gnus-art mm-uu mml2015 mm-view
mml-smime smime dig gnus-sum shr kinsoku svg dom gnus-group gnus-undo
gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7
netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win gnus
nnheader ol-docview doc-view image-mode exif ol-bibtex ol-bbdb ol-w3m
ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-footnote org-src ob-comint org-pcomplete org-list
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-table oc-basic bibtex iso8601 ol org-keys oc
org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs ert ewoc
debug backtrace find-func movie-data-table rater-table hash-helper
csv-helper parse-csv cl-indent misearch multi-isearch magit-ediff
git-rebase goto-addr vc-mtn vc-hg vc-git vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs vc vc-dispatcher bug-reference magit-extras face-remap
magit-submodule magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff
smerge-mode diff git-commit log-edit message rmc puny rfc822 mml mml-sec
epa derived epg rfc6068 epg-config gnus-util rmail rmail-loaddefs
text-property-search time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader pcvs-util add-log magit-core
magit-autorevert autorevert filenotify magit-margin magit-transient
magit-process with-editor shell pcomplete comint server ansi-color
magit-mode transient magit-git magit-base magit-section format-spec
cursor-sensor crm dash mule-util jka-compr ediff-ptch comp comp-cstr
warnings rx cl-extra help-mode diff-mode easy-mmode ediff ediff-merg
ediff-mult ediff-wind ediff-diff ediff-help ediff-init ediff-util
dired-aux dired dired-loaddefs paredit ws-butler cus-edit cus-start
wid-edit macrostep ring pp ffap thingatpt edmacro kmacro web-mode advice
disp-table finder-inf wombat-theme corfu compat compat-29 icomplete
elec-pair cus-load pcase info package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util mailcap url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode 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 lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer 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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 727871 424820)
 (symbols 48 38766 141)
 (strings 32 238349 577478)
 (string-bytes 1 6177910)
 (vectors 16 81263)
 (vector-slots 8 1667405 1015905)
 (floats 8 484 3251)
 (intervals 56 28782 5018)
 (buffers 992 35))

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

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Brandon Irizarry <brandon.irizarry@gmail.com> writes:

> To reproduce: in emacs invoked with '-Q', in the Scratch buffer, insert the
> following
> code:
>
> (require 'cl-macs)
>
> (cl-flet ((fn ((min max))
>      (message "%d %d" min max)))
>   (fn '(2 3)))
>
> The cl-flet form evaluates fine (e.g. with C-x C-e or C-M-x), which
> leads me to conclude that this construct is legal. However, attempting
> to instrument the form with Edebug (C-u C-M-x) results in an enormous
> backtrace starting with
>
> Debugger entered--Lisp error: (invalid-read-syntax "Expected" "lambda")

Thanks.  I agree.  This doesn't seem to be a new bug.

Edebug first tries to match (symbolp form), this is the first branch in
an &or - this calls `edebug-list-form' which binds edebug-gate --> t
and tries to
(edebug-match-specs cursor '(lambda-expr body) 'edebug-match-specs)
which doesn't match but errors because of the bound `edebug-gate'.

Who knows how to fix this correctly?

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

> (cl-flet ((fn ((min max))
>      (message "%d %d" min max)))
>   (fn '(2 3)))
> 
> The cl-flet form evaluates fine (e.g. with C-x C-e or C-M-x), which
> leads me to conclude that this construct is legal. 

I think this lambda-list is invalid.  It certainly is invalid in CL, and 
I can't find any extension mentioned in the Emacs docs.

Why this doesn't signal an error at evaluation time, I don't know.  If 
you try this with defun, you get an error.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Brandon Irizarry]

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

Destructured lambda-lists appear to be an Elisp extension to Common Lisp's
argument notation,
and the Emacs docs do mention them (though not in the Elisp manual; rather,
it's in the CL-Lib manual).
They even mention your point about such a lambda-list not being valid
Common Lisp.

From the CL-Lib manual, 4.3.2 (Function Bindings):

     Functions defined by ‘cl-flet’ may use the full Common Lisp
     argument notation supported by ‘cl-defun’; also, the function body
     is enclosed in an implicit block as if by ‘cl-defun’.  *Note
     Program Structure::.

Following the reference to Chapter 2, and heading into 2.1 (Argument
Lists):

   Argument lists support “destructuring”.  In Common Lisp,
destructuring is only allowed with ‘defmacro’; this package allows it
with ‘cl-defun’ and other argument lists as well.

That is, while 'defun' doesn't support this, 'cl-defun' (and, by extension,
'cl-flet') does.

- Brandon

On Thu, Aug 17, 2023 at 1:29 AM Gerd Möllmann <gerd.moellmann@gmail.com>
wrote:

> > (cl-flet ((fn ((min max))
> >      (message "%d %d" min max)))
> >   (fn '(2 3)))
> >
> > The cl-flet form evaluates fine (e.g. with C-x C-e or C-M-x), which
> > leads me to conclude that this construct is legal.
>
> I think this lambda-list is invalid.  It certainly is invalid in CL, and
> I can't find any extension mentioned in the Emacs docs.
>
> Why this doesn't signal an error at evaluation time, I don't know.  If
> you try this with defun, you get an error.
>

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

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Eli Zaretskii]

> Cc: 65344@debbugs.gnu.org
> From: Brandon Irizarry <brandon.irizarry@gmail.com>
> Date: Thu, 17 Aug 2023 11:42:06 -0400
> 
> Destructured lambda-lists appear to be an Elisp extension to Common Lisp's argument notation,
> and the Emacs docs do mention them (though not in the Elisp manual; rather, it's in the CL-Lib
> manual).
> They even mention your point about such a lambda-list not being valid Common Lisp.
> 
> From the CL-Lib manual, 4.3.2 (Function Bindings):
> 
>      Functions defined by ‘cl-flet’ may use the full Common Lisp
>      argument notation supported by ‘cl-defun’; also, the function body
>      is enclosed in an implicit block as if by ‘cl-defun’.  *Note
>      Program Structure::.
> 
> Following the reference to Chapter 2, and heading into 2.1 (Argument Lists): 
> 
>    Argument lists support “destructuring”.  In Common Lisp,
> destructuring is only allowed with ‘defmacro’; this package allows it
> with ‘cl-defun’ and other argument lists as well. 

You may be right, but please don't rely on the documentation as if it
were a Gospel, or assume that it tells you all the truth and nothing
but the truth.  Our documentation sometimes omits stuff and other
times could allow interpretation that is factually incorrect.  IOW,
not everything the documentation says must be so, and not everything
it doesn't say doesn't exist or is false.

So when Gerd says he is not sure such lambda-lists are valid, I
suggest to trust Gerd more than you trust the docs.  The truth is, of
course, in the code, so if you find in the code evidence that such
forms are supported, that's a definitive conclusion.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

> From the CL-Lib manual, 4.3.2 (Function Bindings):
> 
>      Functions defined by ‘cl-flet’ may use the full Common Lisp
>      argument notation supported by ‘cl-defun’; also, the function body
>      is enclosed in an implicit block as if by ‘cl-defun’.  *Note
>      Program Structure::.
> 
> Following the reference to Chapter 2, and heading into 2.1 (Argument
> Lists):
> 
>    Argument lists support “destructuring”.  In Common Lisp,
> destructuring is only allowed with ‘defmacro’; this package allows it
> with ‘cl-defun’ and other argument lists as well.
> 
> That is, while 'defun' doesn't support this, 'cl-defun' (and, by extension,
> 'cl-flet') does.

Wow, that's nicely hidden.  Thanks for digging that out! I would like to 
nominate this for the obscure feature of the week award.

@Eli:

I can see that implemented in cl--do-arglist, which calls itself 
recursively for such lists like in our case used as function parameters. 
  cl--do-arglist is used (indirectly) by cl-flet, cl-labels, cl-defun, 
maybe others.

cl-defun's doc string even contains has hint

"...The full form of a Common Lisp function argument list is
...
VAR may be replaced recursively with an argument list for
destructuring,,,"

I don't think cl-flet/cl-labels have that.

@Michael:

Not remembering much about Edebug, but cl-defun seems to work with 
Edebug.  So... :-)

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Not remembering much about Edebug, but cl-defun seems to work with
> Edebug.  So... :-)

Yes, this problem does not concern `cl-defun'.  This is the Edebug spec
of `cl-flet':

#+begin_src emacs-lisp
((&rest [&or (symbolp form)
             (&define [&name symbolp "@cl-flet@"]
                      [&name [] gensym] ;Make it unique!
                      cl-lambda-list
                      cl-declarations-or-string
                      [&optional ("interactive" interactive)]
                      def-body)])
 cl-declarations body)
#+end_src

Instrumenting a binding first tries (symbolp form): This is the syntax
where you can specify an expression to evaluate whose result is then
fbound.  Although we obviously have three elements to match in the
recipe case, Edebug tries nonetheless, and FORM raises an error because
it can't match ((min max)).  It should just cause backtracking, but the
implementation disables backtracking by purpose.

When you replace (symbolp form) with (symbolp sexp) the problem is gone.
But that only silences the problem: this also disables the
instrumentation of a form in that position.

I'm too dumb to fix this correctly.  We could transpose the two &or
branches in the spec, this works but it is also only a way to avoid the
problem.


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

On 18.08.23 01:07, Michael Heerdegen wrote:
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
>> Not remembering much about Edebug, but cl-defun seems to work with
>> Edebug.  So... :-)
> 
> Yes, this problem does not concern `cl-defun'.  This is the Edebug spec
> of `cl-flet':
> 
> #+begin_src emacs-lisp
> ((&rest [&or (symbolp form)
>               (&define [&name symbolp "@cl-flet@"]
>                        [&name [] gensym] ;Make it unique!
>                        cl-lambda-list
>                        cl-declarations-or-string
>                        [&optional ("interactive" interactive)]
>                        def-body)])
>   cl-declarations body)
> #+end_src
> 
> Instrumenting a binding first tries (symbolp form): This is the syntax
> where you can specify an expression to evaluate whose result is then
> fbound.  Although we obviously have three elements to match in the
> recipe case, Edebug tries nonetheless, and FORM raises an error because
> it can't match ((min max)).  It should just cause backtracking, but the
> implementation disables backtracking by purpose.
> 
> When you replace (symbolp form) with (symbolp sexp) the problem is gone.
> But that only silences the problem: this also disables the
> instrumentation of a form in that position.
> 
> I'm too dumb to fix this correctly.  We could transpose the two &or
> branches in the spec, this works but it is also only a way to avoid the
> problem.

Thanks.

The corresponding declaration of cl-defun is

      (&define [&name sexp]   ;Allow (setf ...) additionally to symbols.
               cl-lambda-list
               cl-declarations-or-string
               [&optional ("interactive" interactive)]
               def-body)

which I would naively expect to be suitable for a single function in an 
flet/labels.  (Maybe without the (setf ...) case, I'm not sure ATM).
Do you perhaps have an insight why there are two &name in the flet spec?

Also naively asked, what does the &or in the flet case mean?  Does it 
say that that the elements of the flet can either be symbols or 
functions?  A symbol element doesn't make much sense to me.  Do you 
perhaps have a clue what that is?

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> which I would naively expect to be suitable for a single function in
> an flet/labels.  (Maybe without the (setf ...) case, I'm not sure
> ATM).

That's correct, but only one part.

> Do you perhaps have an insight why there are two &name in the flet
> spec?

Eh - not really.  That's some internal magic - to correctly associate
the code with the function names or something like that, I guess.

> Also naively asked, what does the &or in the flet case mean?  Does it
> say that that the elements of the flet can either be symbols or
> functions?

There is a second syntax to support: a function binding can also have
the syntax (fname EXPR) instead of (fname args body...).  EXPR can be a
lambda expression but also any arbitrary Lisp returning a function
value.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

On 18.08.23 07:58, Michael Heerdegen wrote:
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
>> which I would naively expect to be suitable for a single function in
>> an flet/labels.  (Maybe without the (setf ...) case, I'm not sure
>> ATM).
> 
> That's correct, but only one part.

RIght, that's what I meant.

>> Do you perhaps have an insight why there are two &name in the flet
>> spec?
> 
> Eh - not really.  That's some internal magic - to correctly associate
> the code with the function names or something like that, I guess.

Ok. It's probably not important.

>> Also naively asked, what does the &or in the flet case mean?  Does it
>> say that that the elements of the flet can either be symbols or
>> functions?
> 
> There is a second syntax to support: a function binding can also have
> the syntax (fname EXPR) instead of (fname args body...).  EXPR can be a
> lambda expression but also any arbitrary Lisp returning a function
> value.

(Another nominee for the most obscure feature of the month.  That's also 
not in CL, BTW.)

When I try something like

(cl-flet (y (x (lambda () 1)))
   (x))

I get a not-a-list error from the Y.  That's kind of what I'm wondering. 
  The debug declaratino for flet has the symbolp at the same level as 
the local-function &define.

And, if that's the problem, the next question would then be how to 
declare a binding (FN VALUE).  Maybe (%define &name ... <something>)?

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

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

On 18.08.23 08:43, Gerd Möllmann wrote:
> On 18.08.23 07:58, Michael Heerdegen wrote:
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>
>>> which I would naively expect to be suitable for a single function in
>>> an flet/labels.  (Maybe without the (setf ...) case, I'm not sure
>>> ATM).
>>
>> That's correct, but only one part.
> 
> RIght, that's what I meant.
> 
>>> Do you perhaps have an insight why there are two &name in the flet
>>> spec?
>>
>> Eh - not really.  That's some internal magic - to correctly associate
>> the code with the function names or something like that, I guess.
> 
> Ok. It's probably not important.
> 
>>> Also naively asked, what does the &or in the flet case mean?  Does it
>>> say that that the elements of the flet can either be symbols or
>>> functions?
>>
>> There is a second syntax to support: a function binding can also have
>> the syntax (fname EXPR) instead of (fname args body...).  EXPR can be a
>> lambda expression but also any arbitrary Lisp returning a function
>> value.
> 
> (Another nominee for the most obscure feature of the month.  That's also 
> not in CL, BTW.)
> 
> When I try something like
> 
> (cl-flet (y (x (lambda () 1)))
>    (x))
> 
> I get a not-a-list error from the Y.  That's kind of what I'm wondering. 
>   The debug declaratino for flet has the symbolp at the same level as 
> the local-function &define.
> 
> And, if that's the problem, the next question would then be how to 
> declare a binding (FN VALUE).  Maybe (%define &name ... <something>)?

The Elisp manual has very nice description of debug specs, indeed.
It's under Elisp > Edebug > Edebug and Macros > Specification Lists.
 From that description, I think this is the right debug spec for flet 
(patch attached)

(debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
			[&name [] gensym] ;Make it unique!
			cl-lambda-list
			cl-declarations-or-string
			[&optional ("interactive" interactive)]
			def-body)
		    (&define [&name symbolp "@cl-flet@"]
			[&name [] gensym] ;Make it unique!
			def-body)])

The second &define is for the (FN EXPR) bindings.  It comes after the 
&define for "normal" function bindings because because, for some reason, 
apparently the second &define also matches the other case.  (The 
description in the Elisp manual, BTW, also explain what the duplicate 
name does, although I'm not sure why it is done here, because the names 
of the local functions should be unique already.  I'm probably 
overlooking something.)

Seems to work for me.  WDYT?



[-- Attachment #2: flet.patch --]
[-- Type: text/plain, Size: 1091 bytes --]

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 0a3181561bd..0d91f77251e 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2064,12 +2064,14 @@ cl-flet
 
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1)
-           (debug ((&rest [&or (symbolp form)
-                               (&define [&name symbolp "@cl-flet@"]
+           (debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
                                         [&name [] gensym] ;Make it unique!
                                         cl-lambda-list
                                         cl-declarations-or-string
                                         [&optional ("interactive" interactive)]
+                                        def-body)
+                               (&define [&name symbolp "@cl-flet@"]
+                                        [&name [] gensym] ;Make it unique!
                                         def-body)])
                    cl-declarations body)))
   (let ((binds ()) (newenv macroexpand-all-environment))

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> (debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
> 			[&name [] gensym] ;Make it unique!
> 			cl-lambda-list
> 			cl-declarations-or-string
> 			[&optional ("interactive" interactive)]
> 			def-body)
> 		    (&define [&name symbolp "@cl-flet@"]
> 			[&name [] gensym] ;Make it unique!
> 			def-body)])

> The second &define is for the (FN EXPR) bindings.  It comes after the
> &define for "normal" function bindings because because, for some
> reason, apparently the second &define also matches the other case.

That's because you use 'def-body' which matches any number of body
expressions.  I think this is not correct: we are expecting a single
form.

Apart from that: yes, something like this... I did not yet understand
the &name magic.

And i wonder whether we should apply the distributive law and factor out
the first few identical specs inside the &or.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> (The description in the Elisp manual, BTW, also explain what the
> duplicate name does, although I'm not sure why it is done here,
> because the names of the local functions should be unique already.
> I'm probably overlooking something.)

Ok - I understand now that the names are used by Edebug internally, to
map the code pieces to a descriptor.

Then I guess the names must be unique here because shadowing local
functions would get the same name assigned.  Or any that appear
anywhere, actually... right?

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

On 20.08.23 06:39, Michael Heerdegen wrote:
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
>> (The description in the Elisp manual, BTW, also explain what the
>> duplicate name does, although I'm not sure why it is done here,
>> because the names of the local functions should be unique already.
>> I'm probably overlooking something.)
> 
> Ok - I understand now that the names are used by Edebug internally, to
> map the code pieces to a descriptor.
> 
> Then I guess the names must be unique here because shadowing local
> functions would get the same name assigned.  Or any that appear
> anywhere, actually... right?

Correct.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

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

On 20.08.23 05:57, Michael Heerdegen wrote:
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
>> (debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
>> 			[&name [] gensym] ;Make it unique!
>> 			cl-lambda-list
>> 			cl-declarations-or-string
>> 			[&optional ("interactive" interactive)]
>> 			def-body)
>> 		    (&define [&name symbolp "@cl-flet@"]
>> 			[&name [] gensym] ;Make it unique!
>> 			def-body)])
> 
>> The second &define is for the (FN EXPR) bindings.  It comes after the
>> &define for "normal" function bindings because because, for some
>> reason, apparently the second &define also matches the other case.
> 
> That's because you use 'def-body' which matches any number of body
> expressions.  I think this is not correct: we are expecting a single
> form.

Ah, I see.  Thanks.

> 
> Apart from that: yes, something like this... I did not yet understand
> the &name magic.
> 
> And i wonder whether we should apply the distributive law and factor out
> the first few identical specs inside the &or.

I'd rather keep the two separate, I must admit.  Basically, bacause the 
first case it the important one, and the second I find obscure.

I also checked now what's up with functions whose name is (setf ...), 
which is valid CL.  That's not supported in ELisp, it seems, so I 
changed the debug spec of cl-defun accordingly in the attached patch.

If this looks good, I'd bring that to master, unless Eli wants it in 29.

[-- Attachment #2: flet.patch --]
[-- Type: text/plain, Size: 1450 bytes --]

modified   lisp/emacs-lisp/cl-macs.el
@@ -389,7 +389,7 @@ cl-defun
 \(fn NAME ARGLIST [DOCSTRING] BODY...)"
   (declare (debug
             ;; Same as defun but use cl-lambda-list.
-            (&define [&name sexp]   ;Allow (setf ...) additionally to symbols.
+            (&define [&name symbolp]
                      cl-lambda-list
                      cl-declarations-or-string
                      [&optional ("interactive" interactive)]
@@ -2064,13 +2064,15 @@ cl-flet
 
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1)
-           (debug ((&rest [&or (symbolp form)
-                               (&define [&name symbolp "@cl-flet@"]
+           (debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
                                         [&name [] gensym] ;Make it unique!
                                         cl-lambda-list
                                         cl-declarations-or-string
                                         [&optional ("interactive" interactive)]
-                                        def-body)])
+                                        def-body)
+                               (&define [&name symbolp "@cl-flet@"]
+                                        [&name [] gensym] ;Make it unique!
+                                        def-form)])
                    cl-declarations body)))
   (let ((binds ()) (newenv macroexpand-all-environment))
     (dolist (binding bindings)

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> I'd rather keep the two separate, I must admit.

Agreed - and who knows whether that would work ok for &define list
specs.

> I also checked now what's up with functions whose name is (setf ...),
> which is valid CL.  That's not supported in ELisp, it seems, so I
> [...]

Eh - aren't such names used by "gv.el"?

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

On 20.08.23 08:08, Michael Heerdegen wrote:

>> I also checked now what's up with functions whose name is (setf ...),
>> which is valid CL.  That's not supported in ELisp, it seems, so I
>> [...]
> 
> Eh - aren't such names used by "gv.el"?

AFAIU, and I'm pretty sure pf that, Gv stores setter function in a 
symbol property of the getter function name.  So, there is no need for 
more generalized function names.

That's probably more ecomical than extending function names to allow 
something like (setf ...).

(Although, I must say, adding generalized function names to CNUCL, in 
addition to (setf ...), and as a more meaningful replacement for 
gensyms, proved quite helpful to me for things like CLOS method 
functions, discriminating function etc.  Just planting an idea :-)).

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> On 20.08.23 08:08, Michael Heerdegen wrote:
>
> >> I also checked now what's up with functions whose name is (setf ...),
> >> which is valid CL.  That's not supported in ELisp, it seems, so I
> >> [...]
> > Eh - aren't such names used by "gv.el"?
>
> AFAIU, and I'm pretty sure pf that, Gv stores setter function in a
> symbol property of the getter function name.  So, there is no need for
> more generalized function names.
>
> That's probably more ecomical than extending function names to allow
> something like (setf ...).

`gv-setter' returns such names.  I see them from time to time when
debugging.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

BTW, after adding a test for this to test/emacs-lisp/cl-macs-tests.el,
I'm now a bit puzzled.  The test succeeds without my changes.

This is the new test case:

(ert-deftest cl-flet/edebug ()
  "Check that we can instrument `cl-flet' forms (bug#65344)."
  (with-temp-buffer
    (print '(cl-flet (;; "Obscure" form of binding supported by cl-flet
                      (x (progn (list 1 2) (lambda ())))
                      ;; Destructuring lambda-list
                      (y ((min max)) (list min max))
                      ;; Regular binding plus shadowing.
                      (z (a) a)
                      (z (a) a))
              (y '(1 2)))
           (current-buffer))
    (let ((edebug-all-defs t)
          (edebug-initial-mode 'Go-nonstop))
      ;; Just make sure the forms can be instrumented.
      (eval-buffer))))

Instrumenting the cl-flet of the test case interactively in "emacs -Q"
with C-M-x fails, as expected.  An interactive eval-buffer with
edebug-all-defs set to t succeeds.

So there's something else fishy somewhere, I'd guess, which is
unfortunate, because that pattern with eval-buffer is also used
elsewhere in cl-macs-tests.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> BTW, after adding a test for this to test/emacs-lisp/cl-macs-tests.el,
> I'm now a bit puzzled.  The test succeeds without my changes.

Please ignore this, I can't be trusted.  (I forgot about edebug-all-forms.)

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> BTW, after adding a test for this to test/emacs-lisp/cl-macs-tests.el,
>> I'm now a bit puzzled.  The test succeeds without my changes.
>
> Please ignore this, I can't be trusted.  (I forgot about edebug-all-forms.)

Committed to master as 18c85306ac211402e4772bdb94b63d300a2ac119.

Thanks for the report, Brandon, and thanks to Michael for proofreading.
I'll close this bug.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Committed to master as 18c85306ac211402e4772bdb94b63d300a2ac119.

Thanks.  But...

Your patch breaks this case because it changed the order in the &or:

#+begin_src emacs-lisp
(defun make-fun (n)
  (lambda () n))

(defvar k 17)

(cl-flet ((a (make-fun k))) ;; `make-fun' call not instrumented
  (a))
#+end_src

Did changing the order improve anything at all?

Second question: you are sure that the
";Allow (setf ...) additionally to symbols." thing was not something
Stefan wanted to keep intentionally?  Just wondering.  This spec was
never being used, right?

TIA,

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Your patch breaks this case because it changed the order in the &or:
>
> #+begin_src emacs-lisp
> (defun make-fun (n)
>   (lambda () n))
>
> (defvar k 17)
>
> (cl-flet ((a (make-fun k))) ;; `make-fun' call not instrumented
>   (a))
> #+end_src

I wonder if this isn't a bug in cl-flet itself.  If you change the names
a bit, this is 

(cl-flet ((fn (a b)))
  ...)

which is a perfectly valid local definition of FN with two parameter A
and B, returning nil in CL.  It signals an error in Emacs which I'd
consider a bug.

> Did changing the order improve anything at all?

It's more beautiful :-).

> Second question: you are sure that the
> ";Allow (setf ...) additionally to symbols." thing was not something
> Stefan wanted to keep intentionally?  Just wondering.

What makes you think so?

> This spec was
> never being used, right?

Right.  One cannot define functions with non-symbol function names, and
never could.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> I wonder if this isn't a bug in cl-flet itself.  If you change the names
> a bit, this is 
>
> (cl-flet ((fn (a b)))
>   ...)
>
> which is a perfectly valid local definition of FN with two parameter A
> and B, returning nil in CL.  It signals an error in Emacs which I'd
> consider a bug.

It's a known limitation, AFAIR.  It's more important to support the
(SYMBOL EXPR) than this corner case, and we don't want to guess "what is
meant", so a binding of two elements is always interpreted this way in
Elisp.  This problem has been discussed a while ago.

> > Second question: you are sure that the ";Allow (setf ...)
> > additionally to symbols." thing was not something Stefan wanted to
> > keep intentionally?  Just wondering.
>
> What makes you think so?

He definitely saw it when he made the last changes.

> > This spec was never being used, right?
>
> Right.  One cannot define functions with non-symbol function names,
> and never could.

Hmm, right...but where did I see it.  Oh, I remember, it was
`cl-defmethod' that supports such names.


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> I wonder if this isn't a bug in cl-flet itself.  If you change the names
>> a bit, this is 
>>
>> (cl-flet ((fn (a b)))
>>   ...)
>>
>> which is a perfectly valid local definition of FN with two parameter A
>> and B, returning nil in CL.  It signals an error in Emacs which I'd
>> consider a bug.
>
> It's a known limitation, AFAIR.  It's more important to support the
> (SYMBOL EXPR) than this corner case, and we don't want to guess "what is
> meant", so a binding of two elements is always interpreted this way in
> Elisp.  This problem has been discussed a while ago.

That's quite unfortunate :-(.  I wish that whole extension would be at
least be deprecated.

I'll exchange the two debug specs then.  ATM, I don't see how to test
that though.  That's also unfortunate.

> Hmm, right...but where did I see it.  Oh, I remember, it was
> `cl-defmethod' that supports such names.

Looks like it does, indeed, by constructing a symbol.  One couldn't tell
from the doc string :-).

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Brandon Irizarry]

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

Hi all,

First, thanks for all the attention this has received. Eli, your comments
are duly noted.

Trying my best to follow this thread, I took the patch submitted by Gerd,
pasted it in my scratch buffer, renamed the macro 'my-cl-flet', but
switched the order of the two '&define' clauses. I evaluated this macro and
started using it on some small test cases, which check out. I believe I may
merely be echoing Gerd's comment about exchanging the two debug specs, but
I wanted to flesh this aspect out a bit more thoroughly in case. Note that
I've used Org source blocks (which I've noticed people have been doing),
since I suspect people are reading this thread in Emacs itself, and can
subsequently avail themselves of this format.

Under 'emacs -Q':

#+begin_src emacs-lisp
(require 'cl-macs)

;; insert 'my-cl-flet' macro definition here

(my-cl-flet ((cool-fn ((min max))
                      (cons min max)))
  (cool-fn '(2 3)))
#+end_src

Edebug here runs as smooth as butter. Nice!

Next, I tried another example mentioned in this thread:

#+begin_src emacs-lisp
(defun make-fun (n)
  (message "the function")
  (lambda () n))

(defvar k 17)

(my-cl-flet ((a (make-fun k))) ;; `make-fun' call not instrumented
  (a))
#+end_src

Again, everything looks good. Finally, a more conventional example:

#+begin_src emacs-lisp
(my-cl-flet ((fn (a b)
                 (cons a b)))
  (fn 1 2))
#+end_src

Works out, as expected. So it looks like we may have a happy ending after
all?

WDYT?

On Tue, Aug 22, 2023 at 4:05 AM Gerd Möllmann <gerd.moellmann@gmail.com>
wrote:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
> > Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> >
> >> I wonder if this isn't a bug in cl-flet itself.  If you change the names
> >> a bit, this is
> >>
> >> (cl-flet ((fn (a b)))
> >>   ...)
> >>
> >> which is a perfectly valid local definition of FN with two parameter A
> >> and B, returning nil in CL.  It signals an error in Emacs which I'd
> >> consider a bug.
> >
> > It's a known limitation, AFAIR.  It's more important to support the
> > (SYMBOL EXPR) than this corner case, and we don't want to guess "what is
> > meant", so a binding of two elements is always interpreted this way in
> > Elisp.  This problem has been discussed a while ago.
>
> That's quite unfortunate :-(.  I wish that whole extension would be at
> least be deprecated.
>
> I'll exchange the two debug specs then.  ATM, I don't see how to test
> that though.  That's also unfortunate.
>
> > Hmm, right...but where did I see it.  Oh, I remember, it was
> > `cl-defmethod' that supports such names.
>
> Looks like it does, indeed, by constructing a symbol.  One couldn't tell
> from the doc string :-).
>

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

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> That's quite unfortunate :-(.  I wish that whole extension would be at
> least be deprecated.

That's the opinion of one part of the people.  The other part likes that
extension.

The root of this is the old discussion of how strictly cl-lib should
follow the Common Lisp originals.  We will not pacify this discussion.
I think we found a good compromise in most cases, even when it is not
the optimum for everyone.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Brandon Irizarry <brandon.irizarry@gmail.com> writes:

> Works out, as expected. So it looks like we may have a happy ending
> after all?
>
> WDYT?

Yes, I think we are done here.

Thanks to Gerd for working bravely on this black Edebug spec magic.


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Drew Adams]

> The root of this is the old discussion of how strictly cl-lib should
> follow the Common Lisp originals.  We will not pacify this discussion.
> I think we found a good compromise in most cases, even when it is not
> the optimum for everyone.

FWIW.

Dunno which part(s) of the people I fall in, but my
opinion is that we should have separated, and we
still should try to separate (1) actual Common Lisp 
emulation/reproduction/whatever-you-want-to-call-it,
which should be quite faithful to the standard, from
(2) non-CL constructs (functions, variables, macros,
special forms) that might seem a bit CL-like or that
might share some of the underlying implementation
with some of #1.

I think it was a mistake to add things to cl-lib.el,
or cl.el, or cl-<anything>.el that is not directly
related to Common Lisp.

I see no reason not to do that.  It shouldn't be any
harder to separate the two than to cram them into
the same library and give everything the prefix `cl-'.

Each time this has come up (some new thingie that's
unrelated to CL that we stick in a `cl-*' file and
give the `cl-' prefix) I've spoken against doing so.
I've never understood why this has been done, and
I've never heard a good argument for doing it.

You think we've found a good compromise in most cases.
My question is why there's any need to compromise.

(This is different from an incomplete or not-so-strong
implementation of some CL thingie.  I'm talking about
our adding functions that are unrelated to CL support.)

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Drew Adams <drew.adams@oracle.com> writes:

>> The root of this is the old discussion of how strictly cl-lib should
>> follow the Common Lisp originals.  We will not pacify this discussion.
>> I think we found a good compromise in most cases, even when it is not
>> the optimum for everyone.
>
> FWIW.
>
> Dunno which part(s) of the people I fall in, but my
> opinion is that we should have separated, and we
> still should try to separate (1) actual Common Lisp 
> emulation/reproduction/whatever-you-want-to-call-it,
> which should be quite faithful to the standard, from
> (2) non-CL constructs (functions, variables, macros,
> special forms) that might seem a bit CL-like or that
> might share some of the underlying implementation
> with some of #1.

Agree 100&, but that ship seems to have saild.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Mattias Engdegård]

Gerd, edebug-tests fail on master after your changes (cc0f546825). Would you do something about that?

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Mattias Engdegård]

> Gerd, edebug-tests fail on master after your changes (cc0f546825). Would you do something about that?

And cl-macs-tests.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> Gerd, edebug-tests fail on master after your changes
> (cc0f546825). Would you do something about that?

Yes thanks, Michael Albinus sent me a mail as well.  See emacs-devel.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

>> Gerd, edebug-tests fail on master after your changes (cc0f546825). Would you do something about that?
>
> And cl-macs-tests.

Strange stuff, I guess I'll need some time to understand what is going
on.  As a temporary measure I've reverted my last commit, the reversing
of the &defines.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
>
>>> Gerd, edebug-tests fail on master after your changes (cc0f546825). Would you do something about that?
>>
>> And cl-macs-tests.
>
> Strange stuff, I guess I'll need some time to understand what is going
> on.  As a temporary measure I've reverted my last commit, the reversing
> of the &defines.

Maybe it's not so temporary after all.

The problem seems to be the attempt to first match the spec

  (&define [&name symbolp "@cl-flet@"]
           [&name [] gensym] ;Make it unique!
           def-form)])

with (for example)

  (fn ((x y)) ...) or
  (fn (&rest x) ...)

Edebug then matches def-form with ((x y)) or (&rest x), and it finds it
doesn't match.  I would have expected that this leads to backtracking
and trying the second alternative of the &or, but that's not happening.

So, at the moment we're kinda stuck with the situation that local
functions that look like local-functions with empty body, like

  (cl-flet ((fn (...)))
    ...)

are not instrumented like the cl-flet extension case.  That's not a
catastrphy, I guess.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Drew Adams]

> >> The root of this is the old discussion of how strictly cl-lib should
> >> follow the Common Lisp originals.  We will not pacify this discussion.
> >> I think we found a good compromise in most cases, even when it is not
> >> the optimum for everyone.
> >
> > FWIW.
> >
> > Dunno which part(s) of the people I fall in, but my
> > opinion is that we should have separated, and we
> > still should try to separate (1) actual Common Lisp
> > emulation/reproduction/whatever-you-want-to-call-it,
> > which should be quite faithful to the standard, from
> > (2) non-CL constructs (functions, variables, macros,
> > special forms) that might seem a bit CL-like or that
> > might share some of the underlying implementation
> > with some of #1.
> 
> Agree 100&, but that ship seems to have saild.

I think it's (at least at this point) about deciding
and stating the intention.  At first there we none
or few non-CL things offered in the cl-*.el code.
Then a very few more.  Then a bunch more.  If no
intention is declared that just adding non-CL stuff
almost/seemingly willy-nilly to cl-*.el is OK, such
addition might well be increasingly likely.

IOW, let's not hope for perfect, and give up because
things are already imperfect (that ship has sailed).
Instead, why not declare that it's better to not add
non-CL stuff to cl-*.el files, and work to keep it
out.  Not adding more, and declaring that policy, is
at least better than adding more with no such policy.

Just one opinion.  We could improve things a bit here.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Edebug then matches def-form with ((x y)) or (&rest x), and it finds it
> doesn't match.  I would have expected that this leads to backtracking
> and trying the second alternative of the &or, but that's not
> happening.

It's very similar to the originally reported problem.  As I said - maybe
look at `edebug-list-form' which is called in both cases and
intentionally inhibits backtracking, AFAIU.


> So, at the moment we're kinda stuck with the situation that local
> functions that look like local-functions with empty body, like
>
>   (cl-flet ((fn (...)))
>     ...)
>
> are not instrumented like the cl-flet extension case.  That's not a
> catastrphy, I guess.

Nothing in Emacs is a catastrophe (it's not the real world), but
debugging bugs are the least thing I want to cope with when debugging a
bug.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> IOW, let's not hope for perfect, and give up because
> things are already imperfect (that ship has sailed).
> Instead, why not declare that it's better to not add
> non-CL stuff to cl-*.el files, and work to keep it
> out.  Not adding more, and declaring that policy, is
> at least better than adding more with no such policy.
>
> Just one opinion.  We could improve things a bit here.

My opinion is different.  cl-lib enhancing Emacs Lisp well is more
important than emulating Common Lisp 100 percent perfectly - which is
impossible anyway (no multiple return values, etc.).  And we don't have
the manpower to separate cl-lib into two libraries - one with the goal
to be maximally Common Lisp compliant, the other being a maximally good
enhancement for Elisp.  So unless someone is willing to invest this not
small amount of work, now and in the future (and I want to ask: would
this be a good investment of time, given the huge amount of open real
bugs)...

... we have to live with the situation that we have only 1 library, and
only one definition for every CL thing.  And the compromise is that we
don't break CL semantics wherever possible, and try to add only stuff
that is more or less diagonal to the existing semantics.  And this is
the case apart from some corner cases that are negligible, compared to
the language features we don't provide at all.

I don't understand what your actual problems with this situation are.
The thing developed into a different direction than you expected, and,
IMO, became better, more useful for Emacs.  You like CL, ok, you are
nonetheless not disallowed to run Common Lisp on your computer or even
in Emacs.

So, what is goal, why is it better than what we have now, and how do you
want to reach that goal?


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Edebug then matches def-form with ((x y)) or (&rest x), and it finds it
>> doesn't match.  I would have expected that this leads to backtracking
>> and trying the second alternative of the &or, but that's not
>> happening.
>
> It's very similar to the originally reported problem.  As I said - maybe
> look at `edebug-list-form' which is called in both cases and
> intentionally inhibits backtracking, AFAIU.

That's correct.  Setting edebug-gate to t makes not matching an error.
That's the, confusing to me, invalid-read-syntax.  I understand that.

What I don't understand is if that's intended, or maybe an oversight.
Oversight, because it prevents a, in my opinion, valid-looking use of
def-form like in our case, inside an &or.

Difficult.

One could of course try filtering out constructs that def-form doesn't
like with a preceeding &not, but that's such an effing solution that my
arm hairs stand up.  I don't think I'll do that.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Drew Adams <drew.adams@oracle.com> writes:

> I think it's (at least at this point) about deciding
> and stating the intention.  At first there we none
> or few non-CL things offered in the cl-*.el code.
> Then a very few more.  Then a bunch more.  If no
> intention is declared that just adding non-CL stuff
> almost/seemingly willy-nilly to cl-*.el is OK, such
> addition might well be increasingly likely.
>
> IOW, let's not hope for perfect, and give up because
> things are already imperfect (that ship has sailed).
> Instead, why not declare that it's better to not add
> non-CL stuff to cl-*.el files, and work to keep it
> out.  Not adding more, and declaring that policy, is
> at least better than adding more with no such policy.
>
> Just one opinion.  We could improve things a bit here.

Just my 2 cents.

I would appreciate that.  At least let's try not make things incompable
with their CL counterparts, like in the flet case (which I meant with
the sailed ship).

On the other hand, I don't think the usual suspects do something like
that deliberatly.  And not excluding to have something like cl-defmethod
etc. that's not 100% CLOS but tries to comes close.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> On the other hand, I don't think the usual suspects do something like
> that deliberatly.

It was not an oversight, though.  The decision to extend `cl-flet' and
`cl-labels' in that way had been made in the knowledge that it will
break CL semantics in this corner case.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Drew Adams]

> > On the other hand, I don't think the usual suspects do something like
> > that deliberatly.
> 
> It was not an oversight, though.  The decision to extend `cl-flet' and
> `cl-labels' in that way had been made in the knowledge that it will
> break CL semantics in this corner case.

If something is somewhat like a CL construct,
but it is intentionally different in some way
(and not just because we've implemented only
partial support for it), then why use the
prefix `cl-' for it?  Why not use the prefix
`el-' or whatever?

Nothing says that Elisp needs to have the
same things as CL.  But why call something
different "CL support" or "CL emulation", and
use the same prefix, `cl-', that we use for
things that are really intended to emulate
CL constructs?

It's like we have no guideline or map now.
To what avail?  There's no shortage of
prefixes and nothing forcing things with
different purposes or natures to be in the
same file.

The initial inspiration for some Elisp
thingie might, or might not, have come from
some CL thingie.  But that's not a reason
to put, or keep. it in a cl-*.el file, or to
have it use the `cl-' prefix.  Things can
change over time, and their names and
libraries can change too, to reflect changes
in behavior/meaning.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> If something is somewhat like a CL construct,
> but it is intentionally different in some way
> (and not just because we've implemented only
> partial support for it), then why use the
> prefix `cl-' for it?  Why not use the prefix
> `el-' or whatever?

That would be much more confusing IMO.  `cl-flet' is not just "somewhat
like a CL construct".  "cl inspired" would be a bad description.  The
manual clearly describes the limits of the "emulation", and, as I said,
even more limiting incompatibilities do not stem from such extensions.


> Nothing says that Elisp needs to have the
> same things as CL.  But why call something
> different "CL support" or "CL emulation", and
> use the same prefix, `cl-', that we use for
> things that are really intended to emulate
> CL constructs?

The library is somewhere between an "CL emulation" and a "CL inspired
extension library".  It is hard to find a really good name and
description.

> It's like we have no guideline or map now.

Naming being hard or not satisfactory doesn't imply anything.  I doesn't
tell what we must do.  It just means it is hard to find a "perfect for
everybody" name.  That naming something is hard might mean that there is
a problem with that thing, or it might mean nothing.

> To what avail?  There's no shortage of
> prefixes and nothing forcing things with
> different purposes or natures to be in the
> same file.

Changing this prefix would cause work and trouble.  If you think it is
worth it - what's your suggestion?  "el-" is much worse.  What in `flet'
is more "Emacs Lisp"y than in `let'?  Everything in Emacs Lisp is Emacs
Lisp.  The "Emacs Lisp" version of `flet'?  Of which `flet'?  Ahh - of
the Common Lisp `flet' - but it's only 99.9% compatible, so we don't
call it "cl-".

This line of argument is not convincing me.  If a user has looked at the
documentation (one has to anyway to get a start), the "cl-" is also
hardly a source of confusion.  So I still don't see a relevant problem.


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Difficult.
>
> One could of course try filtering out constructs that def-form doesn't
> like with a preceeding &not, but that's such an effing solution that my
> arm hairs stand up.  I don't think I'll do that.

Looks like a bad design choice made very early in the development of
Edebug.  Maybe the developers just didn't expect that this would be used
inside an &or alternative.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> That's correct.  Setting edebug-gate to t makes not matching an error.
> That's the, confusing to me, invalid-read-syntax.  I understand that.

Would it maybe work (as a workaround) to use &interpose and bind
edebug-gate -> nil in the matching function?

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Difficult.
>>
>> One could of course try filtering out constructs that def-form doesn't
>> like with a preceeding &not, but that's such an effing solution that my
>> arm hairs stand up.  I don't think I'll do that.
>
> Looks like a bad design choice made very early in the development of
> Edebug.  Maybe the developers just didn't expect that this would be used
> inside an &or alternative.

The docs say

   Edebug specifications provide the power of regular expression
   matching, plus some context-free grammar constructs:

I have a hard time imagining that somone with that spiration overlooks
something like what we have here.

I'll submit a new issue for this.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> That's correct.  Setting edebug-gate to t makes not matching an error.
>> That's the, confusing to me, invalid-read-syntax.  I understand that.
>
> Would it maybe work (as a workaround) to use &interpose and bind
> edebug-gate -> nil in the matching function?

I don't think so.  The docs say for &interpose

     means that Edebug will first match SPEC against the code and then
     call FUN with the code that matched ‘spec’

i.e. it first tries to match, which will give an error.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Drew Adams]

> > If something is somewhat like a CL construct,
> > but it is intentionally different in some way
> > (and not just because we've implemented only
> > partial support for it), then why use the
> > prefix `cl-' for it?  Why not use the prefix
> > `el-' or whatever?
> 
> That would be much more confusing IMO.  `cl-flet' is not just "somewhat
> like a CL construct".  "cl inspired" would be a bad description.  The
> manual clearly describes the limits of the "emulation", and, as I said,
> even more limiting incompatibilities do not stem from such extensions.

You seem to be guessing that by "something" I
meant `cl-flet' or at least I meant to include
`cl-flet'.  I did not.

I was speaking generally - the "If something"
is key.

Yes, I can tell from the Subject that this
thread is specifically about `cl-flet'.  I
tried to make clear that my comment was not
really on-topic, but was more general: about
our handling of CL and non-CL stuff. 
 
> > Nothing says that Elisp needs to have the
> > same things as CL.  But why call something
> > different "CL support" or "CL emulation", and
> > use the same prefix, `cl-', that we use for
> > things that are really intended to emulate
> > CL constructs?
> 
> The library is somewhere between an "CL emulation" and a "CL inspired
> extension library".  It is hard to find a really good name and
> description.

Exactly my point.  Instead of "the library",
which is a congeries and can become a
catch-all, let's work toward having two (or
more, if needed) separate libraries: for
(1) CL emulation, however limited or complete
and (2) other, non-CL constructs, however
much they may have been inspired in some way
by something in CL.

> > It's like we have no guideline or map now.
> 
> Naming being hard or not satisfactory doesn't imply anything.  I doesn't
> tell what we must do.  It just means it is hard to find a "perfect for
> everybody" name.  That naming something is hard might mean that there is
> a problem with that thing, or it might mean nothing.

Agreed that naming is hard.  But that's not
what my point is about.  Currently, we don't
have two separate bins in which to place
things: (1) CL-emulation and (2) other.  So
there's no reflection at the name level that's
preceded by deciding _what_ something is: (1)
CL support or (2) non-CL.

> > To what avail?  There's no shortage of
> > prefixes and nothing forcing things with
> > different purposes or natures to be in the
> > same file.
> 
> Changing this prefix would cause work and trouble.

I wasn't speaking of changing any specific
name or its prefix.  I wasn't speaking of
`cl-flet', for example.  I was speaking in
general, proposing a policy of trying to
avoid mixing in stuff that doesn't support
our CL emulation with stuff that does.

> If you think it is worth it - what's your
> suggestion?  "el-" is much worse.

I don't care what the non-CL library or
libraries might be called, or what prefixes
they might use - other than not using `cl-'.

And it might well be that some such constructs
would belong in existing standard files.  And
maybe, as such, some might not need any prefix.

> What in `flet' is more "Emacs Lisp"y than in
> `let'?

Again, I haven't said a word about `flet' or
`let' or ANY specific `cl-*' construct.

Dunno what gave you that impression; apologies
if I said something that could be misread to
make someone think that.

As for `let': IIUC it's pretty close to the
`let' in CL now (but yes, we have buffer-local
etc.).  I also don't have a problem with Emacs
_not_ using any `cl-' prefix for something
that does the same as what's in CL.

We don't call our `if' `cl-if', but (I think?)
it's the same as the `if' of CL.  Likewise,
our `cond' etc.  And rightfully so.

> Everything in Emacs Lisp is Emacs
> Lisp.  The "Emacs Lisp" version of `flet'?  Of which `flet'?  Ahh - of
> the Common Lisp `flet' - but it's only 99.9% compatible, so we don't
> call it "cl-".

FWIW (as kinda said above), I'm OK with 99%
(or even less) support for some CL construct,
and I'm even OK with dropping the `cl-'
prefix.

The `cl-' prefix distinguishes something that
emulates a CL construct from something else
that might otherwise have the same name but
is not CL.

E.g., our regular `defun' is different from
`cl-defun'.

If we didn't have our own macro named `defun'
and we had only `cl-defun' then (IMO) it
would be OK to call that just `defun'.  And
for clarity - to let users know that it's
essentially CL `defun', we'd alias it to
`cl-defun'.

What I disagree with, as expressed in this
thread, is the addition to a `cl*.el' library
of something that's _not_ a CL construct, and
giving it a `cl-' prefix.  _Why do that?_
That's the question I asked - to what avail?

> This line of argument is not convincing me.  If a user has looked at the
> documentation (one has to anyway to get a start), the "cl-" is also
> hardly a source of confusion.  So I still don't see a relevant problem.

OK, so you don't see a problem.  Do you see
a reason why we would add some `cl-foobar' to
`cl-lib.el', if it's a function that has no
relation to Common Lisp?  Does it make any
sense to you that someone might expect it to
be housed elsewhere, with a different prefix
(or with no prefix)?

That's all I'm trying to say here.  Let's avoid
stuffing non-CL stuff in `cl*.el' files.  Is it
necessary to clean house wholesale, moving and
renaming things to fix this mixup?  No.  I'm
not proposing disruption or extra work - just a
recognition of a will to avoid adding not CL
stuff to `cl*.el' files and giving it prefix
`cl-'.  Nothing revolutionary or heavy-handed
intended.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> I don't think so.  The docs say for &interpose
>
>      means that Edebug will first match SPEC against the code and then
>      call FUN with the code that matched ‘spec’
>
> i.e. it first tries to match, which will give an error.

The idea is that we would match with 'sexp' or some catchall pattern and
then instrument using 'form' inside the function.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> OK, so you don't see a problem.  Do you see
> a reason why we would add some `cl-foobar' to
> `cl-lib.el', if it's a function that has no
> relation to Common Lisp?  Does it make any
> sense to you that someone might expect it to
> be housed elsewhere, with a different prefix
> (or with no prefix)?
>
> That's all I'm trying to say here.  Let's avoid
> stuffing non-CL stuff in `cl*.el' files.  Is it
> necessary to clean house wholesale, moving and
> renaming things to fix this mixup?  No.  I'm
> not proposing disruption or extra work - just a
> recognition of a will to avoid adding not CL
> stuff to `cl*.el' files and giving it prefix
> `cl-'.  Nothing revolutionary or heavy-handed
> intended.

Sorry about mentioning flet again, but it's a good example to discuss.
Would you want that we have cl-flet, which is an restricted version
of the current implementation, and cl-flet*, which is like the current
cl-flet?  Or two constructs with non-overlapping semantics?  Or was
extending cl-flet as had been done ok for you?

Your suggestion sounds logical and objective, but what has "a relation to
CL" or is "not CL" is a bit subjective, it is an individual decision
where to draw the line.  I mean, anyone could agree with your claim but
some may still come to other decision than you would expect.

Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Drew Adams]

> > OK, so you don't see a problem.  Do you see
> > a reason why we would add some `cl-foobar' to
> > `cl-lib.el', if it's a function that has no
> > relation to Common Lisp?  Does it make any
> > sense to you that someone might expect it to
> > be housed elsewhere, with a different prefix
> > (or with no prefix)?
> >
> > That's all I'm trying to say here.  Let's avoid
> > stuffing non-CL stuff in `cl*.el' files.  Is it
> > necessary to clean house wholesale, moving and
> > renaming things to fix this mixup?  No.  I'm
> > not proposing disruption or extra work - just a
> > recognition of a will to avoid adding not CL
> > stuff to `cl*.el' files and giving it prefix
> > `cl-'.  Nothing revolutionary or heavy-handed
> > intended.
> 
> Sorry about mentioning flet again, but it's a good example to discuss.
> Would you want that we have cl-flet, which is an restricted version
> of the current implementation, and cl-flet*, which is like the current
> cl-flet?  Or two constructs with non-overlapping semantics?  Or was
> extending cl-flet as had been done ok for you?

I have nothing to say about `flet', `cl-flet', or `cl-flet*'.  I'm pretty ignorant of all that.

> Your suggestion sounds logical and objective, but what has "a relation to
> CL" or is "not CL" is a bit subjective, it is an individual decision
> where to draw the line.  I mean, anyone could agree with your claim but
> some may still come to other decision than you would expect.

Judgment calls, yes.  Especially if trying not
to be black-&-white, given where things are at
(not starting from scratch).  A judgment call,
but it need not be only up to an individual.
No different from having and following other
conventions, we have - e.g., wrt coding style.

Wrt any concrete change, there would presumably
be some discussion, with those closer to things
likely weighing in with more authority.

I'm only saying that AFAIK there hasn't been any
such attempt to avoid putting extraneous stuff
in `cl*.el'.  I've brought up the question in
the past wrt specific things (don't recall what),
but it wasn't thought to be important to keep
`cl-' for CL.  That approach/attitude tends to
lead to more such mix-up, not less.

It's not about perfection.  It's about having a
will/goal not to add unrelated stuff to `cl*.el'.

Not such a big deal or so hard to grasp, I think.
And doesn't require everyone to agree about each
detail.  It's about a general wish/commitment to
aim to have `cl*.el' for CL stuff.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> I don't think so.  The docs say for &interpose
>>
>>      means that Edebug will first match SPEC against the code and then
>>      call FUN with the code that matched ‘spec’
>>
>> i.e. it first tries to match, which will give an error.
>
> The idea is that we would match with 'sexp' or some catchall pattern and
> then instrument using 'form' inside the function.

That might work, but it's a bit like with the &not-filtering I
mentioned: It is a lot of effort for an edge case, and it doesn't
address the underlying problem, if it is one, (bug#65516).

Maybe you could give it a spin?  I don't think I will.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Michael Heerdegen]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Maybe you could give it a spin?  I don't think I will.

I did now.  I can take control using &interpose using [] as its spec so
that nothing is consumed when the specified function is called.  But
then it gets very complicated.

This approach would only work if we catch the 'no-match tag thrown by
`edebug-no-match', and also catch the 'invalid-read-syntax' signal,
(there are several different types of failure involved, this makes it
much more troublesome).

This can all be done, but it breaks the control flow of the Edebug
parser.  The Edebug 'cursor' (current parsing position AFAIU - there can
be multiple cursors in use) is implicit, so I don't know and can't
control where to continue in the 'no-match case.  I gave up at this
point because it was obvious that we will not want to do this.


Michael.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Maybe you could give it a spin?  I don't think I will.
>
> I did now.  I can take control using &interpose using [] as its spec so
> that nothing is consumed when the specified function is called.  But
> then it gets very complicated.
>
> This approach would only work if we catch the 'no-match tag thrown by
> `edebug-no-match', and also catch the 'invalid-read-syntax' signal,
> (there are several different types of failure involved, this makes it
> much more troublesome).
>
> This can all be done, but it breaks the control flow of the Edebug
> parser.  The Edebug 'cursor' (current parsing position AFAIU - there can
> be multiple cursors in use) is implicit, so I don't know and can't
> control where to continue in the 'no-match case.  I gave up at this
> point because it was obvious that we will not want to do this.

Ok.  Thanks for trying it out. 

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The problem seems to be the attempt to first match the spec
>
>   (&define [&name symbolp "@cl-flet@"]
>            [&name [] gensym] ;Make it unique!
>            def-form)])

I think this is not right:

- `def-form` is for use by forms which will be run "later", meaning that
  we can start executing those instrumented form while running
  non-instrumented code  (like the inside of a lambda), so it should not
  be needed here: we should use just `form`.

- The &define+&name isn't quite right here.  In `(cl-flet ((F EXP)) ...)`
  it tells Edebug that EXP should be given the name F whereas the `cl-flet`
  actually gives the name F to the value returned by EXP.
  [ In terms of code coverage, F will then always be considered as
    covered just because we have to execute EXP in order to discover the
    function to bind to F.  ]

I think that's why the old code had just `(symbolp form)`:
it's difficult with `(cl-flet ((F EXP)) ...)` to give a name to the
right thing :-(


        Stefan

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

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

>> The problem seems to be the attempt to first match the spec
>>
>>   (&define [&name symbolp "@cl-flet@"]
>>            [&name [] gensym] ;Make it unique!
>>            def-form)])
>
> I think this is not right:
>
> - `def-form` is for use by forms which will be run "later", meaning that
>   we can start executing those instrumented form while running
>   non-instrumented code  (like the inside of a lambda), so it should not
>   be needed here: we should use just `form`.

I don't think I get that yet.  Let's say we have

  (cl-flet ((f (lambda () (list 1 2))))
    (f))

In this case, the code for F will be run in the body of the
flet. Doesn't that qualify as being run later, as you describe above, 
ignoring the "non-instrumented" part, maybe?

(Just from the perspective of behaviour, C-u C-M-x on the flet above does
seem to do what I'd expect: the call to F let's me step into the
body of the lambda.)

>
> - The &define+&name isn't quite right here.  In `(cl-flet ((F EXP)) ...)`
>   it tells Edebug that EXP should be given the name F whereas the `cl-flet`
>   actually gives the name F to the value returned by EXP.
>   [ In terms of code coverage, F will then always be considered as
>     covered just because we have to execute EXP in order to discover the
>     function to bind to F.  ]
>
> I think that's why the old code had just `(symbolp form)`:
> it's difficult with `(cl-flet ((F EXP)) ...)` to give a name to the
> right thing :-(

I'm afraid I know nothing about coverage, so I can't say something
halfways intelligent to that.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> The problem seems to be the attempt to first match the spec
>>>
>>>   (&define [&name symbolp "@cl-flet@"]
>>>            [&name [] gensym] ;Make it unique!
>>>            def-form)])
>>
>> I think this is not right:
>>
>> - `def-form` is for use by forms which will be run "later", meaning that
>>   we can start executing those instrumented form while running
>>   non-instrumented code  (like the inside of a lambda), so it should not
>>   be needed here: we should use just `form`.
>
> I don't think I get that yet.  Let's say we have
>
>   (cl-flet ((f (lambda () (list 1 2))))
>     (f))
>
> In this case, the code for F will be run in the body of the
> flet. Doesn't that qualify as being run later, as you describe above, 
> ignoring the "non-instrumented" part, maybe?

No, in the above case the `def-form` is

    (lambda () (list 1 2))

which will be "run" right when we enter the `cl-flet` ("run" is an
exaggeration here since this lambda is a constant so it'll just
self-evaluate) and not when `f` is called.

> (Just from the perspective of behaviour, C-u C-M-x on the flet above does
> seem to do what I'd expect: the call to F let's me step into the
> body of the lambda.)

Yes, it is harmless for debugging purposes (at most it makes things ever
so slightly less efficient by having to process a few more
`edebug-enter`s than needed).

>> - The &define+&name isn't quite right here.  In `(cl-flet ((F EXP)) ...)`
>>   it tells Edebug that EXP should be given the name F whereas the `cl-flet`
>>   actually gives the name F to the value returned by EXP.
>>   [ In terms of code coverage, F will then always be considered as
>>     covered just because we have to execute EXP in order to discover the
>>     function to bind to F.  ]
>>
>> I think that's why the old code had just `(symbolp form)`:
>> it's difficult with `(cl-flet ((F EXP)) ...)` to give a name to the
>> right thing :-(
>
> I'm afraid I know nothing about coverage, so I can't say something
> halfways intelligent to that.

The whole `&name` business is of no use for the single-step debugger,
it's only useful for other uses of the instrumentation such as
code-coverage.


        Stefan

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> In this case, the code for F will be run in the body of the
>> flet. Doesn't that qualify as being run later, as you describe above, 
>> ignoring the "non-instrumented" part, maybe?
>
> No, in the above case the `def-form` is
>
>     (lambda () (list 1 2))
>
> which will be "run" right when we enter the `cl-flet` ("run" is an
> exaggeration here since this lambda is a constant so it'll just
> self-evaluate) and not when `f` is called.

More specifically, the annotated code of

    (defun sm-foo (x)
      (cl-flet ((f (prog1 (lambda (y) (+ x y)) 0)))
        (f 5)))

stored into (symbol-function 'sm-foo) now looks like:

    (closure (t) (x)
     (edebug-enter 'sm-foo (list x)
      #'(lambda nil :closure-dont-trim-context
          (edebug-after (edebug-before 0) 3
           (let* ((--cl-f--
                   (edebug-enter 'f@cl-flet@4 nil
                    #'(lambda nil :closure-dont-trim-context
                        (edebug-after (edebug-before 0) 1
                         (prog1
                             #'(lambda (y)
                                (edebug-enter 'edebug-anon5 (list y)
                                 #'(lambda nil :closure-dont-trim-context
                                    (edebug-after (edebug-before 0) 3
                                     (+ (edebug-after 0 1 x)
                                      (edebug-after 0 2 y))))))
                           0))))))
             (progn
               (edebug-after (edebug-before 1) 2
                (funcall --cl-f-- 5))))))))

As you can see for `sm-foo` itself, the

    (edebug-enter 'sm-foo (list x)
     #'(lambda nil :closure-dont-trim-context

is (correctly) placed at the very beginning of the body of the function,
so that code coverage can track whether `sm-foo` was called or not.

In contrast the

    (edebug-enter 'f@cl-flet@4 nil
     #'(lambda nil :closure-dont-trim-context

is placed around the code which will compute&return the `f@cl-flet@4`
function, but not inside its body, which instead gets

    (edebug-enter 'edebug-anon5 (list y)
     #'(lambda nil :closure-dont-trim-context

It's actually difficult (and in general impossible) to associate the
name `f@cl-flet@4` with the corresponding `lambda`, so the use of
`edebug-anon5` is largely unavoidable here.  But making the code-coverage
say that `f@cl-flet@4` is called just because we computed that function
(regardless if it's been called) is a problem.


        Stefan

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

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

>>> In this case, the code for F will be run in the body of the
>>> flet. Doesn't that qualify as being run later, as you describe above, 
>>> ignoring the "non-instrumented" part, maybe?
>>
>> No, in the above case the `def-form` is
>>
>>     (lambda () (list 1 2))
>>
>> which will be "run" right when we enter the `cl-flet` ("run" is an
>> exaggeration here since this lambda is a constant so it'll just
>> self-evaluate) and not when `f` is called.

Thanks.  I've skimmed through the docs and Edebug code a bit this today,
and I think I understand you a bit better now.

>
> More specifically, the annotated code of
>
>     (defun sm-foo (x)
>       (cl-flet ((f (prog1 (lambda (y) (+ x y)) 0)))
>         (f 5)))
>
> stored into (symbol-function 'sm-foo) now looks like:
>

For posterity, if somone reads this in a few years, including myself,
maybe...

If I understand correctly, the edebug-before and -after instrumentation
forms lead, via edebug-behavior, to calls to edebug-slow-{before,after},
when test coverage is demanded by edebug-test-coverage being non-nil.
Otherwise, when e-t-c is nil, another pair of functions with 'fast' in
their names is used.

The slow functions update edebug-freq-count.  E-f-q seems to be a
special variable that edebug-default-enter binds to a vector
that is obtained from the symbol-plist of the function name, which is an
argument of e-d-e.

I haven't checked what exactly e-default-e is.  I'm just assuming from
its name it gets called when encountering edebug-enter.

>     (closure (t) (x)
>      (edebug-enter 'sm-foo (list x)

So, in the line above, we obtain the frequency vector from the name
'sm-foo.

>       #'(lambda nil :closure-dont-trim-context
>           (edebug-after (edebug-before 0) 3

And the line above accesses that vector found in edebug-frequency-count.

>            (let* ((--cl-f--
>                    (edebug-enter 'f@cl-flet@4 nil
>                     #'(lambda nil :closure-dont-trim-context
>                         (edebug-after (edebug-before 0) 1

Here the same mechanism for the local-function

>                          (prog1
>                              #'(lambda (y)
>                                 (edebug-enter 'edebug-anon5 (list y)
>                                  #'(lambda nil :closure-dont-trim-context
>                                     (edebug-after (edebug-before 0) 3

And here again the same mechanism, but with a bogus name.  That's what
you are referring to, right?

>                                      (+ (edebug-after 0 1 x)
>                                       (edebug-after 0 2 y))))))
>                            0))))))
>              (progn
>                (edebug-after (edebug-before 1) 2
>                 (funcall --cl-f-- 5))))))))
>
> As you can see for `sm-foo` itself, the
>
>     (edebug-enter 'sm-foo (list x)
>      #'(lambda nil :closure-dont-trim-context
>
> is (correctly) placed at the very beginning of the body of the function,
> so that code coverage can track whether `sm-foo` was called or not.

Understood (I think).  The lambda will land in edebug-default-enter,
which funcalls it.  That lands in the "(edebug-enter 'f@cl-flet@4 ...",
and so on.

>
> In contrast the
>
>     (edebug-enter 'f@cl-flet@4 nil
>      #'(lambda nil :closure-dont-trim-context
>
> is placed around the code which will compute&return the `f@cl-flet@4`
> function, but not inside its body, which instead gets
>
>     (edebug-enter 'edebug-anon5 (list y)
>      #'(lambda nil :closure-dont-trim-context

Right.  I think I found that, too.

> It's actually difficult (and in general impossible) to associate the
> name `f@cl-flet@4` with the corresponding `lambda`, so the use of
> `edebug-anon5` is largely unavoidable here.  But making the code-coverage
> say that `f@cl-flet@4` is called just because we computed that function
> (regardless if it's been called) is a problem.

Ok, understood.  I hope, or did I get lost somewhere?

Do you perhaps an idea how to solve that?

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>                          (prog1
>>                              #'(lambda (y)
>>                                 (edebug-enter 'edebug-anon5 (list y)
>>                                  #'(lambda nil :closure-dont-trim-context
>>                                     (edebug-after (edebug-before 0) 3
>
> And here again the same mechanism, but with a bogus name.  That's what
> you are referring to, right?

While in this specific case it would be technically possible to give it
a better name, it's somewhere between hard and impossible to do it in
general, so I'm not bothered by this "bogus name": that's already what
is used in all other cases where anonymous functions are encountered.

> Do you perhaps an idea how to solve that?

I'm proposing we revert to the code we had originally, which avoids
creating the spurious `f@cl-flet@4`.
[ Tho with extra comments to explain why there's no `&name` on the
  (symbolp form) case.  ]

After all, your original report was due to a misunderstanding, thinking
that `(cl-flet ((a ((b c)))) body)` treated `((b c))` as an arg list
(for a function with an empty body) like Common Lisp's `flet` does,
whereas `cl-flet` treats it as an expression (that returns the function).


        Stefan


diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 850fbc69180..8d92acb0c95 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2075,15 +2075,16 @@ cl-flet
 
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1)
-           (debug ((&rest [&or (&define [&name symbolp "@cl-flet@"]
+           ;; The first (symbolp form) case doesn't use `&name' because
+           ;; it's hard to associate this name with the body of the function
+           ;; that `form' will return (bug#65344).
+           (debug ((&rest [&or (symbolp form)
+                               (&define [&name symbolp "@cl-flet@"]
                                         [&name [] gensym] ;Make it unique!
                                         cl-lambda-list
                                         cl-declarations-or-string
                                         [&optional ("interactive" interactive)]
-                                        def-body)
-                               (&define [&name symbolp "@cl-flet@"]
-                                        [&name [] gensym] ;Make it unique!
-                                        def-form)])
+                                        def-body)])
                    cl-declarations body)))
   (let ((binds ()) (newenv macroexpand-all-environment))
     (dolist (binding bindings)

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Gerd Möllmann]

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

>> Do you perhaps an idea how to solve that?
>
> I'm proposing we revert to the code we had originally, which avoids
> creating the spurious `f@cl-flet@4`.
> [ Tho with extra comments to explain why there's no `&name` on the
>   (symbolp form) case.  ]

Please feel free.

bug#65344: 28.2; Unable to Edebug cl-flet form which uses argument destructuring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Please feel free.

Done, thanks,


        Stefan