From ca419812d35f252fca2708ffdd132c223d094c0f Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Mon, 3 Aug 2020 21:07:32 +0200 Subject: [PATCH] Avoid duplicate Edebug symbols when backtracking (Bug#42701) When Edebug backtracks, it nevertheless generates definitions for the non-matching branches, see Bug#41988 and Bug#42701. This should be fixed eventually (probably by deferring the definition until a branch is known to match), but for now add a band-aid to avoid these duplicate symbols, at least for anonymous forms. * lisp/emacs-lisp/edebug.el (edebug-make-enter-wrapper): Regenerate anonymous names. * test/lisp/emacs-lisp/edebug-tests.el (edebug-tests-duplicate-symbol-backtrack): New regression test. --- lisp/emacs-lisp/edebug.el | 7 ++++++ test/lisp/emacs-lisp/edebug-tests.el | 32 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el index cef97e0fb45..d9bbf6129c6 100644 --- a/lisp/emacs-lisp/edebug.el +++ b/lisp/emacs-lisp/edebug.el @@ -1240,6 +1240,13 @@ purpose by adding an entry to this alist, and setting ;; since it wraps the list of forms with a call to `edebug-enter'. ;; Uses the dynamically bound vars edebug-def-name and edebug-def-args. ;; Do this after parsing since that may find a name. + (when (string-match-p (rx bos "edebug-anon" (+ digit) eos) + (symbol-name edebug-old-def-name)) + ;; FIXME: Due to Bug#42701, we reset an anonymous name so that + ;; backtracking doesn't generate duplicate definitions. It would + ;; be better to not define wrappers in the case of a non-matching + ;; specification branch to begin with. + (setq edebug-old-def-name nil)) (setq edebug-def-name (or edebug-def-name edebug-old-def-name (gensym "edebug-anon"))) `(edebug-enter diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el index 1be68f6ff46..04a7b2f5a0f 100644 --- a/test/lisp/emacs-lisp/edebug-tests.el +++ b/test/lisp/emacs-lisp/edebug-tests.el @@ -1000,5 +1000,37 @@ clashes (Bug#41853)." inner@cl-flet@10005 edebug-tests-cl-flet-2)))))) +(ert-deftest edebug-tests-duplicate-symbol-backtrack () + "Check that Edebug doesn't create duplicate symbols when +backtracking (Bug#42701)." + (with-temp-buffer + (dolist (form '((require 'subr-x) + (defun edebug-tests-duplicate-symbol-backtrack () + (if-let (x (funcall (lambda (y) 1) 2)) 3 4)))) + (print form (current-buffer))) + (let* ((edebug-all-defs t) + (edebug-initial-mode 'Go-nonstop) + (instrumented-names ()) + (edebug-new-definition-function + (lambda (name) + (when (memq name instrumented-names) + (error "Duplicate definition of `%s'" name)) + (push name instrumented-names) + (edebug-new-definition name))) + ;; Make generated symbols reproducible. + (gensym-counter 10000)) + (eval-buffer) + ;; The anonymous symbols are uninterned. Use their names so we + ;; can perform the assertion. The names should still be unique. + (should (equal (mapcar #'symbol-name (reverse instrumented-names)) + ;; The outer definition comes after the inner + ;; ones because its body ends later. + ;; FIXME: There are twice as many inner + ;; definitions as expected due to Bug#42701. + ;; Once that bug is fixed, remove the duplicates. + '("edebug-anon10000" + "edebug-anon10001" + "edebug-tests-duplicate-symbol-backtrack")))))) + (provide 'edebug-tests) ;;; edebug-tests.el ends here -- 2.39.2