bug#65516: 30.0.50; Edebug behavior of signaling errors in &or

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

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

>> I'd change all these, but I can understand if that's too radical for
>> your taste :-).
>
> In any case, we first have to find another solution to bug#41988.
> I.e. a way to "undo" the effects of `edebug-make-form-wrapper`
> or to change `edebug-make-form-wrapper` so that it matches SPECS
> *first* and only when/if that's done do the rest (so there's no need to
> "undo").

Right.

To summarize: The "culprit" is this commit:

commit 53dfd85a7f971875e716a55f010ee508bce89eed
Author: Philipp Stephani <phst@google.com>
Date:   Thu Mar 18 12:40:08 2021 +0100

    Edebug: Disable backtracking when hitting a &define keyword.
    
    Edebug doesn't deal well with backtracking out of definitions, see
    Bug#41988.  Rather than trying to support this rare situation (e.g. by
    implementing a multipass parser), prevent it by adding an implicit
    gate.
    
    * lisp/emacs-lisp/edebug.el (edebug--match-&-spec-op): Disable
    backtracking when hitting a &define keyword.
    
    * test/lisp/emacs-lisp/edebug-tests.el
    (edebug-tests-duplicate-&define): New unit test.
    (edebug-tests--duplicate-&define): New helper macro.
    
    * doc/lispref/edebug.texi (Backtracking): Mention &define in the list
    of constructs that disable backtracking.
    
    * etc/NEWS: Document new behavior.

This commit causes &define to signal if it doesn't match:

--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -1942,14 +1942,16 @@ edebug--match-&-spec-op
   ;; Normally, &define is interpreted specially other places.
   ;; This should only be called inside of a spec list to match the remainder
   ;; of the current list.  e.g. ("lambda" &define args def-body)
-   (edebug-make-form-wrapper
-    cursor
-    (edebug-before-offset cursor)
-    ;; Find the last offset in the list.
-    (let ((offsets (edebug-cursor-offsets cursor)))
-      (while (consp offsets) (setq offsets (cdr offsets)))
-      offsets)
-    specs))
+  (prog1 (edebug-make-form-wrapper
+          cursor
+          (edebug-before-offset cursor)
+          ;; Find the last offset in the list.
+          (let ((offsets (edebug-cursor-offsets cursor)))
+            (while (consp offsets) (setq offsets (cdr offsets)))
+            offsets)
+          specs)
+    ;; Stop backtracking here (Bug#41988).
+    (setq edebug-gate t)))

The reason for making it signal is that e-m-f-wrapper doesn't take into
account that matching might fail, and instead modifies global state.
For example, it changes symbol properties.  (Maybe also other global
state, I haven't checked.)

Do you think it would be possible to let e-m-f-wrapper just put
something on a new "list of actions to be performed oncee the whole
debug spec has been matched"?  I'm thinking of closures as "actions",
here.

That would be the first thing coming to my mind.  I'd find that easier
to follow than an undo.