GNU bug report logs - #74052
Master: edebug fails to instrument nested pcase guard form.

Previous Next

Full log

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: bug-gnu-emacs <at> gnu.org
Cc: acm <at> muc.de
Subject: Master: edebug fails to instrument nested pcase guard form.
Date: Sun, 27 Oct 2024 21:54:29 +0000

Hello, Emacs.

With a recent master version (and most likely, and emacs-30 branch
version):

(i) emacs -Q
(ii) insert the following into *scratch*:

(defun foo ()
  (let (
      )
    (match-b

  , leaving point after match-b.
(iii) C-x C-f path/to/lisp/progmodes/elisp-mode.el.
(iv) Move to function elisp-completion-at-point and instrument it for
  edebug with C-u C-M-x.
(v) C-x b *scratch* RET.
(vi) M-TAB # to invoke completion on match-b.
(vii) Step through elisp-completion-at-point using edebug commands
  aiming to step through the pcase guard clause at L788.
(viii) Note that point doesn't stop in this guard clause.  This is a
  bug.

#########################################################################

Diagnosis:
----------

In pcase.el pcase--edebug-match-pat-args, which is the function of an
edebug &interpose clause, the code fails to call PF (the supplied
parsing function).  This call is absolutely required by the interface of
the pertinent version of function edebug--match-&-spec-op.  It's absence
leads to the lack of instrumentation of the guard form.

#########################################################################

Proposed patch:
---------------

As well as fixing pcase--edebug-match-pat-args, the patch also corrects
bugs in the doc string of the pertinent version of
edebug--match-&-spec-op, and simplifies a use of `apply'.



diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index b96d2437b8a..f6710de126c 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -1803,12 +1803,21 @@ edebug-&optional-wrapper
 
 (cl-defmethod edebug--match-&-spec-op ((_ (eql '&interpose)) cursor specs)
   "Compute the specs for `&interpose SPEC FUN ARGS...'.
-Extracts the head of the data by matching it against SPEC,
-and then matches the rest by calling (FUN HEAD PF ARGS...)
-where PF is the parsing function which FUN can call exactly once,
-passing it the specs that it needs to match.
-Note that HEAD will always be a list, since specs are defined to match
-a sequence of elements."
+SPECS is a list (SPEC FUN ARGS...), where SPEC is an edebug
+specification, FUN is the function from the &interpose form which
+transforms the edebug spec, and the optional ARGS is a list of final
+arguments to be supplied to FUN.
+
+Extracts the head of the data by matching it against SPEC, and then
+matches the rest by calling (FUN HEAD PF ARGS...).  PF is the parsing
+function which FUN must call exactly once, passing it one argument, the
+specs that it needs to match.  FUN's value must be the value of this PF
+call, which in turn will be the value of this function.
+
+Note that HEAD will always be a list, since specs is defined to match a
+sequence of elements."
+  ;; Note: PF is evaluated in FUN rather than in this function, so that
+  ;; it can use any dynamic bindings created there.
   (pcase-let*
       ((`(,spec ,fun . ,args) specs)
        (exps (edebug-cursor-expressions cursor))
@@ -1817,14 +1826,14 @@ edebug-&optional-wrapper
                     (length (edebug-cursor-expressions cursor))))
        (head (seq-subseq exps 0 consumed)))
     (cl-assert (eq (edebug-cursor-expressions cursor) (nthcdr consumed exps)))
-    (apply fun `(,head
-                 ,(lambda (newspecs)
-                    ;; FIXME: What'd be the difference if we used
-                    ;; `edebug-match-sublist', which is what
-                    ;; `edebug-list-form-args' uses for the similar purpose
-                    ;; when matching "normal" forms?
-                    (append instrumented-head (edebug-match cursor newspecs)))
-                 ,@args))))
+    (apply fun head
+               (lambda (newspecs)
+                 ;; FIXME: What'd be the difference if we used
+                 ;; `edebug-match-sublist', which is what
+                 ;; `edebug-list-form-args' uses for the similar purpose
+                 ;; when matching "normal" forms?
+                 (append instrumented-head (edebug-match cursor newspecs)))
+               args)))
 
 (cl-defmethod edebug--match-&-spec-op ((_ (eql '&not)) cursor specs)
   ;; If any specs match, then fail
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 898d460c144..bd0868b3ff0 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -84,14 +84,17 @@ 'pcase-FUN
 (defun pcase--edebug-match-pat-args (head pf)
   ;; (cl-assert (null (cdr head)))
   (setq head (car head))
-  (or (alist-get head '((quote sexp)
-                        (or    &rest pcase-PAT)
-                        (and   &rest pcase-PAT)
-                        (guard form)
-                        (pred  &or ("not" pcase-FUN) pcase-FUN)
-                        (app   pcase-FUN pcase-PAT)))
+  (let ((specs
+         (alist-get head '((quote sexp)
+                           (or    &rest pcase-PAT)
+                           (and   &rest pcase-PAT)
+                           (guard form)
+                           (pred  &or ("not" pcase-FUN) pcase-FUN)
+                           (app   pcase-FUN pcase-PAT)))))
+    (if specs
+        (funcall pf specs)
       (let ((me (pcase--get-macroexpander head)))
-        (funcall pf (and me (symbolp me) (edebug-get-spec me))))))
+        (funcall pf (and me (symbolp me) (edebug-get-spec me)))))))
 
 (defun pcase--get-macroexpander (s)
   "Return the macroexpander for pcase pattern head S, or nil."


-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.