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.

diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index cef97e0fb45c6524a1aa0c5f67e83010d4444774..d9bbf6129c64418ddf1da48a1fc4f9bf9f04cf06 100644 (file)
--- 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 1be68f6ff46a2bb9568d45b2f6ed242a44373775..04a7b2f5a0fb31da1cb8e1775b3c829447b0cc13 100644 (file)
--- 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