Subject: Re: bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions

From: Michael Heerdegen <[email-masked]>
Date: Tue, 26 Nov 2019 22:01:32 +0100

Michael Heerdegen <[email-masked]> writes:

> BTW, I also wonder if we should enhance the command
> `edebug-remove-instrumentation' so that it is able to reload source
> files.  It could look at the SYMOL's `symbol-file's, collect these, load
> the files, and only do what it does now for the symbols that are still
> wrapped.

Here is a draft.  Any thoughts (Lars)?

From 681370954b2f5168e6e0793a9a7ded76db671682 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <[email-masked]>
Date: Tue, 26 Nov 2019 19:23:45 +0100
Subject: [PATCH] WIP: edebug: see 38195, ask whether to reload files to
 deinstrument

---
 lisp/emacs-lisp/edebug.el | 73 ++++++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 16 deletions(-)

diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index d68ed966f8..29bb27fc0d 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -4586,36 +4586,77 @@ edebug--unwrap*-symbol-function
         (was-macro               `(macro . ,unwrapped))
         (t                       unwrapped))))))

+(defcustom edebug-reload-files 'ask
+  "Whether `edebug-remove-instrumentation' should reload files.
+
+When non-nil, `edebug-remove-instrumentation' will reload files
+where possible to get rid of instrumentation.  When the non-nil
+value is the symbol 'ask, ask for every individual file before
+loading it.
+
+When nil or when no defining file can be found remove edebug
+instrumentation by manipulating symbol functions."
+  :type '(choice (const :tag "On" t)
+                 (const :tag "Ask for every file" ask)
+                 (const :tag "Off" nil)))
+
+(defun edebug-get-instrumented-functions ()
+  (let ((functions '()))
+    (mapatoms
+     (lambda (symbol)
+       (when (and (get symbol 'edebug)
+                  (or (functionp symbol)
+                      (macrop symbol))
+                  (edebug--unwrap*-symbol-function
+                   symbol))
+         (push symbol functions)))
+     obarray)
+    functions))
+
 (defun edebug-remove-instrumentation (functions)
   "Remove Edebug instrumentation from FUNCTIONS.
 Interactively, the user is prompted for the function to remove
 instrumentation for, defaulting to all functions."
   (interactive
    (list
-    (let ((functions nil))
-      (mapatoms
-       (lambda (symbol)
-         (when (and (get symbol 'edebug)
-                    (or (functionp symbol)
-                        (macrop symbol))
-                    (edebug--unwrap*-symbol-function
-                     symbol))
-           (push symbol functions)))
-       obarray)
+    (let ((functions (edebug-get-instrumented-functions)))
       (unless functions
         (error "Found no functions to remove instrumentation from"))
       (let ((name
              (completing-read
               "Remove instrumentation from (default all functions): "
               functions)))
-        (if (and name
-                 (not (equal name "")))
+        (if (and name (not (equal name "")))
             (list (intern name))
-          functions)))))
-  ;; Remove instrumentation.
+          t)))))
+  (unless (listp functions)
+    (setq functions (edebug-get-instrumented-functions)))
+  (when edebug-reload-files
+    (let ((files '()))
+      (dolist (f functions)
+        (when-let ((file (symbol-file f 'defun)))
+          (unless (cl-some (apply-partially #'file-equal-p file) files)
+            (push file files))))
+      (let ((do-all (eq edebug-reload-files t))
+            file)
+        (while files
+          (setq file (pop files))
+          (when (or do-all
+                    (pcase (car (read-multiple-choice
+                                 (format "Load %s ?" file)
+                                 (list (list ?y "y" "Reload this file")
+                                       (list ?Y "Y" "\
+Reload this and all following files")
+                                       (list ?n "n" "Don't load this file")
+                                       (list ?N "N" "\
+Don't load this and any following files"))))
+                      (?y t)
+                      (?Y (setq do-all t)  t)
+                      (?n nil)
+                      (?N (setq files nil) nil)))
+            (load file 'noerror nil 'nosuffix))))))
   (dolist (symbol functions)
-    (when-let ((unwrapped
-                (edebug--unwrap*-symbol-function symbol)))
+    (when-let ((unwrapped (edebug--unwrap*-symbol-function symbol)))
       (defalias symbol unwrapped)))
   (message "Removed edebug instrumentation from %s"
            (mapconcat #'symbol-name functions ", ")))
--
2.24.0

Regards,

Michael.