From debbugs-submit-bounces@debbugs.gnu.org Sun Oct 27 17:55:26 2024 Received: (at submit) by debbugs.gnu.org; 27 Oct 2024 21:55:26 +0000 Received: from localhost ([127.0.0.1]:46609 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5BEL-00050V-Gx for submit@debbugs.gnu.org; Sun, 27 Oct 2024 17:55:26 -0400 Received: from lists.gnu.org ([209.51.188.17]:51620) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5BEJ-00050K-IA for submit@debbugs.gnu.org; Sun, 27 Oct 2024 17:55:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t5BDj-0003KX-LX for bug-gnu-emacs@gnu.org; Sun, 27 Oct 2024 17:54:47 -0400 Received: from mail.muc.de ([193.149.48.3]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t5BDh-0002qB-HH for bug-gnu-emacs@gnu.org; Sun, 27 Oct 2024 17:54:47 -0400 Received: (qmail 45760 invoked by uid 3782); 27 Oct 2024 22:54:30 +0100 Received: from muc.de (pd953a291.dip0.t-ipconnect.de [217.83.162.145]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 27 Oct 2024 22:54:30 +0100 Received: (qmail 23098 invoked by uid 1000); 27 Oct 2024 21:54:29 -0000 Date: Sun, 27 Oct 2024 21:54:29 +0000 To: bug-gnu-emacs@gnu.org Subject: Master: edebug fails to instrument nested pcase guard form. Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-DEBBUGS-CC: Stefan Monnier X-Submission-Agent: TMDA/1.3.x (Ph3nix) From: Alan Mackenzie X-Primary-Address: acm@muc.de Received-SPF: pass client-ip=193.149.48.3; envelope-from=acm@muc.de; helo=mail.muc.de X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.4 (-) X-Debbugs-Envelope-To: submit Cc: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.4 (--) 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 '¬)) 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). From debbugs-submit-bounces@debbugs.gnu.org Mon Oct 28 09:50:47 2024 Received: (at 74052) by debbugs.gnu.org; 28 Oct 2024 13:50:47 +0000 Received: from localhost ([127.0.0.1]:52961 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5Q8s-0001FA-A7 for submit@debbugs.gnu.org; Mon, 28 Oct 2024 09:50:46 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:18273) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5Q8n-0001Dd-1C for 74052@debbugs.gnu.org; Mon, 28 Oct 2024 09:50:42 -0400 Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 5DA7180964; Mon, 28 Oct 2024 09:49:58 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1730123397; bh=u7JoWaNrFdwbD8b1IsZgPcd/flNBkx5XVWDj9gRU+pQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=ANBj6Xm+zCHzVUea7wQWDVLN77YsI03lgM1pqlYe3FgfN0iBDiKJSgr7ITQJQ/WlD blHbPQ8mPAvGl7LT0a9b2gbqxTqmnZ68ppeHuB9O+ZTWAM+LT2oJqF7N9U1iqoSG2r yLU/r+3Rgz2PkYIzo4V3N12CatPCTOYEylLsi42sFN7sUQFKU8+2tOlLeD9Pc9B70L 0K9ru0mrUTxbaseNhNTSLemXWAuU2gqI4HUTnbnryDLlxDlEDXx2IpTRSm0bUmUX7C C2ufJ6fBTA+e6i0vprdDSyOsYfofbZNluPy5ppk5z5fqTYUtOTItH0Eu7K7RYBCl05 Cw/4breMpAp+g== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 4AD658004C; Mon, 28 Oct 2024 09:49:57 -0400 (EDT) Received: from pastel (69-196-161-60.dsl.teksavvy.com [69.196.161.60]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 237E612041C; Mon, 28 Oct 2024 09:49:56 -0400 (EDT) From: Stefan Monnier To: Alan Mackenzie Subject: Re: bug#74052: Master: edebug fails to instrument nested pcase guard form. In-Reply-To: (Alan Mackenzie's message of "Sun, 27 Oct 2024 21:54:29 +0000") Message-ID: References: Date: Mon, 28 Oct 2024 09:49:55 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL 0.040 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 74052 Cc: 74052@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi Alan, > ######################################################################### > > 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. +1 > 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." +1 > + ;; Note: PF is evaluated in FUN rather than in this function, so that > + ;; it can use any dynamic bindings created there. Nitpick: PF is not "evaluated" but "called". > @@ -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))) [ Having a hard time imagining how I ended up writing it like I did. ] +1 > @@ -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))))))) +1 (tho I'd hoist the common `(funcall pf` out of the `if`). And thanks! Stefan From debbugs-submit-bounces@debbugs.gnu.org Mon Oct 28 12:47:46 2024 Received: (at 74052-done) by debbugs.gnu.org; 28 Oct 2024 16:47:46 +0000 Received: from localhost ([127.0.0.1]:54531 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5Su9-0001Cj-To for submit@debbugs.gnu.org; Mon, 28 Oct 2024 12:47:46 -0400 Received: from mail.muc.de ([193.149.48.3]:13036) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5Su7-0001CT-Av for 74052-done@debbugs.gnu.org; Mon, 28 Oct 2024 12:47:44 -0400 Received: (qmail 39888 invoked by uid 3782); 28 Oct 2024 17:47:00 +0100 Received: from muc.de (p4fe15284.dip0.t-ipconnect.de [79.225.82.132]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Mon, 28 Oct 2024 17:46:59 +0100 Received: (qmail 12486 invoked by uid 1000); 28 Oct 2024 16:46:59 -0000 Date: Mon, 28 Oct 2024 16:46:59 +0000 To: Stefan Monnier Subject: Re: bug#74052: Master: edebug fails to instrument nested pcase guard form. Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Submission-Agent: TMDA/1.3.x (Ph3nix) From: Alan Mackenzie X-Primary-Address: acm@muc.de X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 74052-done Cc: Dmitry Gutov , 74052-done@debbugs.gnu.org, acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hello, Stefan. On Mon, Oct 28, 2024 at 09:49:55 -0400, Stefan Monnier wrote: > Hi Alan, [ .... ] > > + ;; Note: PF is evaluated in FUN rather than in this function, so that > > + ;; it can use any dynamic bindings created there. > Nitpick: PF is not "evaluated" but "called". Nits are important. :-) I've corrected that. [ .... ] > > @@ -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))))))) > +1 (tho I'd hoist the common `(funcall pf` out of the `if`). Done that too, by a slight rearrangement of the code. I've committed the (corrected) patch, and I'm closing the bug with this post. > And thanks! And thank you too for such a quick reply! > Stefan -- Alan Mackenzie (Nuremberg, Germany). From unknown Tue Sep 09 00:44:16 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Tue, 26 Nov 2024 12:24:06 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator