master ca41981: Avoid duplicate Edebug symbols when backtracking (Bug#42701)

[Philipp Stephani]

branch: master
commit ca419812d35f252fca2708ffdd132c223d094c0f
Author: Philipp Stephani <phst@google.com>
Commit: Philipp Stephani <phst@google.com>

    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 cef97e0..d9bbf61 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 1be68f6..04a7b2f 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