bug#51026: 29.0.50; Edebug leaves data in symbols plist after instrumentation

[Arthur Miller <arthur.miller@live.com>]

[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Yes, it is true what you say, but only for a handful of symbols, not
>> even all of those handled in that pcase. I think it is emitted only
>> for let, let*, setq and quote, but it is not important.
>
> No, `edebug-form-spec' is added to symbols by the (declare (debug ...))
> form, and should never be removed from any symbols.

Yes, but as I have tested, the previously mentioned pcase-dolist, line 2075 in
edebug.el is what get's the action going when edebug is loaded.

'declare' just marks stuff, it does not really do anything on it's own. 

For some speical forms that property seems to be added via gv.el (let,
quote, etc ?), and for some (defun and defmacro), it is tested and done in
pcase-dolist itself, since they are no longer special forms but macros.

I have tested to remove that property and exec that pcase-dolist, and the
property was back as expected.

I don't know, I can be wrong too; there is a bit of interaction going on between
all this function vars and alists and what "real" function gets executed when
edebug starts, so sorry if I am getting it a bit wrong.

Anyway, since there can be other special forms and stuff added over time, the
static list won't do. The obvious is just to leave that property alone and remove
the rest of leftover stuff. It can be quite a lot of data in some cases, in
'edebug-coverage' vector, as I have seen while tested. Since it will be
referenced in symbol's  plist, it won't get garbage collected either.

Also this patch does just the half the job. It removes leftovers only if
'edebug-remove-instrumentation' is called. If eval-defun is called with C-u
prefix, but with both 'edebug-all-defs' and 'edebug-all-forms' set to nil, it
will "uninstrument" original function, but it does not seem to call
'edebug-remove-instrumentation', so all the leftovers will be left. I haven't
looked at that part since I am using 'edebug-remove-instrumentation' in my own
code.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clean-edebug-props-on-instrumentation-removal.patch --]
[-- Type: text/x-patch, Size: 1537 bytes --]

From 94016b063faf3f69cf03d7ffa3eac1a15bad34c8 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Wed, 6 Oct 2021 14:16:21 +0200
Subject: [PATCH] Clean edebug props on instrumentation removal

* lisp/emacs-lisp/edebug.el (edebug--strip-plist): New function.
(edebug-remove-instrumentation): Added call to 'edebug--strip-plist'.
---
 lisp/emacs-lisp/edebug.el | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index a38c8bd5ca..2489680bcb 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -4529,6 +4529,12 @@ edebug--unwrap*-symbol-function
         (was-macro               `(macro . ,unwrapped))
         (t                       unwrapped))))))
 
+(defun edebug--strip-plist (symbol)
+  "Remove edebug related properties from plist for SYMBOL."
+  (dolist (prop '(edebug edebug-behavior edebug-coverage
+                         edebug-freq-count ghost-edebug))
+      (cl-remprop symbol prop)))
+
 (defun edebug-remove-instrumentation (functions)
   "Remove Edebug instrumentation from FUNCTIONS.
 Interactively, the user is prompted for the function to remove
@@ -4560,6 +4566,7 @@ edebug-remove-instrumentation
   (dolist (symbol functions)
     (when-let ((unwrapped
                 (edebug--unwrap*-symbol-function symbol)))
+      (edebug--strip-plist symbol)
       (defalias symbol unwrapped)))
   (message "Removed edebug instrumentation from %s"
            (mapconcat #'symbol-name functions ", ")))
-- 
2.33.0