bug#54119: 29.0.50; Edebug: Jumping commands in recursive definitions

bug#54119: 29.0.50; Edebug: Jumping commands in recursive definitions

[Michael Heerdegen]

Hello,

the Edebug sexp jumping commands f and o can behave surprisingly (I
would say annoyingly and wrong) when used near recursive calls of the
current function.

Let me demonstrate the problem with an example:

Instrument

#+begin_src emacs-lisp
(defun my-factorial (n) (if (< n 2) 1 (* n (my-factorial (1- n)))))
#+end_src
                                           ^
                                           |
M-: (my-factorial 5) RET and hit SPC until the cursor is before the
front of the recursive call of `my-factorial' (as indicated).  Hit f.
You get "2" as result.

This is quite unexpected, one would expect (my-factorial 4) ==> 24.

What happened is that Edebug does not really "jump" over the following
expression, it just sets a breakpoint after it and enters "go"
mode. That breakpoint "breaks" the next time it is passed, and that is,
in the above example, at the end of the innermost recursion, AFAIU.

The same thing can happen with `o'.  When that kind of thing happens, it
can become a very tricky task to edebug what you wanted to.

I don't know - can we make those "internal" breakpoints conditional and
let the condition check the recursion level, somehow?  Although, even
then, some obscure kinds of code using catch and throw might still
behave surprisingly.  Edebug would somehow have to check that the jumped
over execution frame terminates normally, or something like that.

TIA,

Michael.


In GNU Emacs 29.0.50 (build 33, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2022-02-23 built on drachen
Repository revision: be7c8c79eb874e3297c8492b1363fb0b8c433fae
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

bug#54119: 29.0.50; Edebug: Jumping commands in recursive definitions

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> #+begin_src emacs-lisp
> (defun my-factorial (n) (if (< n 2) 1 (* n (my-factorial (1- n)))))
> #+end_src

We could exploit the fact that the function is already instrumented.
Every function call of the definition is then wrapped in an
`edebug-after' call:

#+begin_src emacs-lisp
(symbol-function 'my-factorial)  ==>
 (closure (t) (n)
  (edebug-enter 'my-factorial
                (list n)
                #'(lambda nil
                    (edebug-after
                     (edebug-before 0)
                     12
                     (if
                         (edebug-after
                          (edebug-before 1)
                          3
                          (<
                           (edebug-after 0 2 n)
                           2))
                         1
                       (edebug-after
                        (edebug-before 4)
                        11 (* etc...)))))))
#+end_src

We could solve this bug if we temporarily change the function binding of
`edebug-after'.  This is a bit tricky, though, mainly because it's not
trivial to restore the original setup:

- two places must be updated: (symbol-function 'edebug-after) and the
  entry in `edebug-behavior-alist'
- `edebug-forward-sexp' terminates immediately, it's on the "meta
  level", undoing the change in that function is too early, since we
  must expect recursive calls of `edebug-after'.
- the actual code is evaluated as an argument of `edebug-after', so the
  replacement can also not restore the original function binding

Which means: either we also redefine `edebug-before', or we use
something very different.  Here is a proof of concept using
`post-command-hook' for restoring:

#+begin_src emacs-lisp
(defun my-edebug-forward-sexp--around-ad (f &rest args)
    (cl-macrolet ((after-fun ()
                    '(nth 2 (cdr (assq 'edebug edebug-behavior-alist)))))
      (let ((orig-after-fun (after-fun)))
        (cl-labels ((set-after-fun (f)
                    (setf (symbol-function 'edebug-after)
                          (setf (after-fun) f)))
                    (reset-after-fun ()
                      (remove-hook 'post-command-hook #'reset-after-fun)
                      (set-after-fun orig-after-fun)))
          (set-after-fun #'edebug-fast-after)
          (add-hook 'post-command-hook #'reset-after-fun)
          (apply f args)))))

(advice-add 'edebug-forward-sexp :around #'my-edebug-forward-sexp--around-ad)
(advice-add 'edebug-step-out     :around #'my-edebug-forward-sexp--around-ad)
#+end_src

Seems to do the job.  A patch would not need an advice of course.

Better ideas welcome.


Michael.

bug#54119: 29.0.50; Edebug: Jumping commands in recursive definitions

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Seems to do the job.

Let me add: if you wonder why I don't just add a new variable and make
the behavior of `edebug-slow-after' depend on the value, and then just
change the binding in `edebug-forward-sexp': that would not work because
it would also change the behavior of the outer `edebug-slow-after' calls
"we are already in".  We would get more or less go-nonstop: everything
would unwind up to the top.  Remember: we can't just let-bind that
variable since we are in a recursive edit and the jumping commands
immediately terminate.  We can only set it, and then edebug just goes
nonstop.

My approach only works because the "outer" `edebug-slow-after' calls are
unaffected and still stop.  So I think must we must mess with function
bindings - or use a different approach.

Michael.