From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Gemini Lasswell Newsgroups: gmane.emacs.bugs Subject: bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in macros Date: Sun, 05 Nov 2017 13:45:21 -0800 Message-ID: <87tvy8h026.fsf@runbox.com> References: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1509918380 32271 195.159.176.226 (5 Nov 2017 21:46:20 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 5 Nov 2017 21:46:20 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.90 (gnu/linux) Cc: 6415@debbugs.gnu.org, Steve Yegge To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Nov 05 22:46:11 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eBSk7-0007yO-4T for geb-bug-gnu-emacs@m.gmane.org; Sun, 05 Nov 2017 22:46:11 +0100 Original-Received: from localhost ([::1]:45754 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBSkB-00063e-4m for geb-bug-gnu-emacs@m.gmane.org; Sun, 05 Nov 2017 16:46:15 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:57807) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBSk1-00063O-Ik for bug-gnu-emacs@gnu.org; Sun, 05 Nov 2017 16:46:07 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eBSjy-0000Uj-EZ for bug-gnu-emacs@gnu.org; Sun, 05 Nov 2017 16:46:05 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:44055) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eBSjx-0000U5-UY for bug-gnu-emacs@gnu.org; Sun, 05 Nov 2017 16:46:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eBSjx-0003Wy-KZ for bug-gnu-emacs@gnu.org; Sun, 05 Nov 2017 16:46:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Gemini Lasswell Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 05 Nov 2017 21:46:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 6415 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed patch Original-Received: via spool by 6415-submit@debbugs.gnu.org id=B6415.150991834013544 (code B ref 6415); Sun, 05 Nov 2017 21:46:01 +0000 Original-Received: (at 6415) by debbugs.gnu.org; 5 Nov 2017 21:45:40 +0000 Original-Received: from localhost ([127.0.0.1]:52736 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eBSjb-0003WO-LR for submit@debbugs.gnu.org; Sun, 05 Nov 2017 16:45:40 -0500 Original-Received: from aibo.runbox.com ([91.220.196.211]:52130) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eBSjZ-0003WF-EM for 6415@debbugs.gnu.org; Sun, 05 Nov 2017 16:45:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=rbselector1; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From; bh=RDlC3qQcPD3bErWC4CIZP9b26nvCxb2UQPXvsha3qQ4=; b=TjZMzSPpVkU4hKtyRXV5SqsYzT 2cLXxk/UXZIMp5bJMY4DTC9aKTnUW33bjtjotUPUK+qo3ph9HvljexgckdDKQpLH9qAyFqk7TXgL0 4gUN1evghoAnipH7n+qBCrrW60SjOKX4OH8LHpxMsCAHo9CEQ45ANEwyKDyANte6lNN99dGGMHOpu nmu4IgL2Kef3KN8RFFiBrQu7cPkSy43VKjiqpLxUz+kn9LWLUg21Q8xxJuACJPrQSgInJMSDlsKrs 6axBvCgV9sijM2Q9qUm6F/pX5azHLq8Nng/BAwRtU/1eiGAS18xcEy014u15VyL4Z03uxlXymQQae kS4F0byA==; Original-Received: from [10.9.9.212] (helo=mailfront12.runbox.com) by mailtransmit03.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1eBSjS-0001dX-T7; Sun, 05 Nov 2017 22:45:30 +0100 Original-Received: from c-24-22-244-161.hsd1.wa.comcast.net ([24.22.244.161] helo=chinook) by mailfront12.runbox.com with esmtpsa (uid:179284 ) (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) id 1eBSjN-00052U-8a; Sun, 05 Nov 2017 22:45:25 +0100 In-Reply-To: (Stefan Monnier's message of "Mon, 26 Sep 2011 21:43:46 -0400") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:139489 Archived-At: --=-=-= Content-Type: text/plain Here's a new patch for this bug, based on the ideas in Steve's patch. It also allows &rest and specs wrapped in vectors to attempt to match before a dotted tail. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=0001-Fix-Edebug-s-handling-of-dotted-specs-bug-6415.patch >From e6120334f29d97a026a3a2b2892d71ad73be0225 Mon Sep 17 00:00:00 2001 From: Gemini Lasswell Date: Wed, 1 Nov 2017 21:13:02 -0700 Subject: [PATCH] Fix Edebug's handling of dotted specs (bug#6415) * lisp/emacs-lisp/cl-macs.el (cl-destructuring-bind): Use cl-macro-list1 instead of cl-macro-list in Edebug spec. * lisp/emacs-lisp/edebug.el (edebug-after-dotted-spec): Delete unused variable. (edebug-dotted-spec): Add docstring. (edebug-match-specs): Allow &optional and &rest specs to match nothing at the tail of a dotted form. Handle matches of dotted form tails which return non-lists. * test/lisp/emacs-lisp/edebug-tests.el (edebug-tests-dotted-forms): New test. * test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el: (edebug-test-code-use-destructuring-bind): New function. --- lisp/emacs-lisp/cl-macs.el | 2 +- lisp/emacs-lisp/edebug.el | 67 +++++++++++++--------- .../edebug-resources/edebug-test-code.el | 4 ++ test/lisp/emacs-lisp/edebug-tests.el | 14 +++++ 4 files changed, 58 insertions(+), 29 deletions(-) diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el index e313af2497..5535100d4a 100644 --- a/lisp/emacs-lisp/cl-macs.el +++ b/lisp/emacs-lisp/cl-macs.el @@ -684,7 +684,7 @@ cl--arglist-args (defmacro cl-destructuring-bind (args expr &rest body) "Bind the variables in ARGS to the result of EXPR and execute BODY." (declare (indent 2) - (debug (&define cl-macro-list def-form cl-declarations def-body))) + (debug (&define cl-macro-list1 def-form cl-declarations def-body))) (let* ((cl--bind-lets nil) (cl--bind-forms nil) (cl--bind-defs nil) (cl--bind-block 'cl-none) (cl--bind-enquote nil)) (cl--do-arglist (or args '(&aux)) expr) diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el index 0e8f77e29a..dec986ae3e 100644 --- a/lisp/emacs-lisp/edebug.el +++ b/lisp/emacs-lisp/edebug.el @@ -950,7 +950,8 @@ edebug-read-vector ;;; Cursors for traversal of list and vector elements with offsets. -(defvar edebug-dotted-spec nil) +(defvar edebug-dotted-spec nil + "Set to t when matching after the dot in a dotted spec list.") (defun edebug-new-cursor (expressions offsets) ;; Return a new cursor for EXPRESSIONS with OFFSETS. @@ -1526,8 +1527,6 @@ edebug-list-form ;;; Matching of specs. -(defvar edebug-after-dotted-spec nil) - (defvar edebug-matching-depth 0) ;; initial value @@ -1588,36 +1587,48 @@ edebug-match-specs (let ((edebug-dotted-spec t));; Containing spec list was dotted. (edebug-match-specs cursor (list specs) remainder-handler))) - ;; Is the form dotted? - ((not (listp (edebug-cursor-expressions cursor)));; allow nil + ;; The reason for processing here &optional, &rest, and vectors + ;; which might contain them even when the form is dotted is to + ;; allow them to match nothing, so we can advance to the dotted + ;; part of the spec. + ((or (listp (edebug-cursor-expressions cursor)) + (vectorp (car specs)) + (memq (car specs) '(&optional &rest))) ; Process normally. + ;; (message "%scursor=%s specs=%s" + ;; (make-string edebug-matching-depth ?|) cursor (car specs)) + (let* ((spec (car specs)) + (rest) + (first-char (and (symbolp spec) (aref (symbol-name spec) 0))) + (match (cond + ((eq ?& first-char);; "&" symbols take all following specs. + (funcall (get-edebug-spec spec) cursor (cdr specs))) + ((eq ?: first-char);; ":" symbols take one following spec. + (setq rest (cdr (cdr specs))) + (funcall (get-edebug-spec spec) cursor (car (cdr specs)))) + (t;; Any other normal spec. + (setq rest (cdr specs)) + (edebug-match-one-spec cursor spec))))) + ;; The first match result may not be a list, which can happen + ;; when matching the tail of a dotted list. In that case + ;; there is no remainder. + (if (listp match) + (nconc match + (funcall remainder-handler cursor rest remainder-handler)) + match))) + + ;; Must be a dotted form, with no remaining &rest or &optional specs to + ;; match. + (t (if (not edebug-dotted-spec) (edebug-no-match cursor "Dotted spec required.")) ;; Cancel dotted spec and dotted form. (let ((edebug-dotted-spec) - (this-form (edebug-cursor-expressions cursor)) - (this-offset (edebug-cursor-offsets cursor))) - ;; Wrap the form in a list, (by changing the cursor??)... + (this-form (edebug-cursor-expressions cursor)) + (this-offset (edebug-cursor-offsets cursor))) + ;; Wrap the form in a list, by changing the cursor. (edebug-set-cursor cursor (list this-form) this-offset) - ;; and process normally, then unwrap the result. - (car (edebug-match-specs cursor specs remainder-handler)))) - - (t;; Process normally. - (let* ((spec (car specs)) - (rest) - (first-char (and (symbolp spec) (aref (symbol-name spec) 0)))) - ;;(message "spec = %s first char = %s" spec first-char) (sit-for 1) - (nconc - (cond - ((eq ?& first-char);; "&" symbols take all following specs. - (funcall (get-edebug-spec spec) cursor (cdr specs))) - ((eq ?: first-char);; ":" symbols take one following spec. - (setq rest (cdr (cdr specs))) - (funcall (get-edebug-spec spec) cursor (car (cdr specs)))) - (t;; Any other normal spec. - (setq rest (cdr specs)) - (edebug-match-one-spec cursor spec))) - (funcall remainder-handler cursor rest remainder-handler))))))) - + ;; Process normally, then unwrap the result. + (car (edebug-match-specs cursor specs remainder-handler))))))) ;; Define specs for all the symbol specs with functions used to process them. ;; Perhaps we shouldn't be doing this with edebug-form-specs since the diff --git a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el index f52a2b1896..ca49dcd213 100644 --- a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el +++ b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el @@ -126,5 +126,9 @@ edebug-test-code-current-buffer !start!(with-current-buffer (get-buffer-create "*edebug-test-code-buffer*") !body!(format "current-buffer: %s" (current-buffer)))) +(defun edebug-test-code-use-destructuring-bind () + (let ((two 2) (three 3)) + (cl-destructuring-bind (x . y) (cons two three) (+ x!x! y!y!)))) + (provide 'edebug-test-code) ;;; edebug-test-code.el ends here diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el index 02f4d1c5ab..f6c016cdf8 100644 --- a/test/lisp/emacs-lisp/edebug-tests.el +++ b/test/lisp/emacs-lisp/edebug-tests.el @@ -899,5 +899,19 @@ edebug-tests-setup-code-file "@g" (should (equal edebug-tests-@-result '(#("abcd" 1 3 (face italic)) 511)))))) +(ert-deftest edebug-tests-dotted-forms () + "Edebug can instrument code matching the tail of a dotted spec (Bug#6415)." + (edebug-tests-with-normal-env + (edebug-tests-setup-@ "use-destructuring-bind" nil t) + (edebug-tests-run-kbd-macro + "@ SPC SPC SPC SPC SPC SPC" + (edebug-tests-should-be-at "use-destructuring-bind" "x") + (edebug-tests-should-match-result-in-messages "2 (#o2, #x2, ?\\C-b)") + "SPC" + (edebug-tests-should-be-at "use-destructuring-bind" "y") + (edebug-tests-should-match-result-in-messages "3 (#o3, #x3, ?\\C-c)") + "g" + (should (equal edebug-tests-@-result 5))))) + (provide 'edebug-tests) ;;; edebug-tests.el ends here -- 2.14.3 --=-=-= Content-Type: text/plain Stefan Monnier writes: > This edebug-dotted-spec business is really ugly, I wonder if/how we > could just get rid of this variable. Or at least document clearly what > it is supposed to mean. I agree. After far too many hours of looking at this, I think the way to get rid of the variable is to make Edebug's internal representation of its specs into a cl-defstruct so there is room for a "dotted" flag, and then change all the edebug match code so that whenever it makes a new spec or modifies the one it is working with, it inherits the dotted flag. I have a branch with this partially done and am confident that it will work and not cause a performance problem, but it seems like a lot of work and a lot of changes to stable code to make it work exactly the same. So I added a docstring. --=-=-=--