From: Michael Heerdegen <michael_heerdegen@web.de>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi@gnus.org>, 38195@debbugs.gnu.org
Subject: bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions
Date: Thu, 21 Nov 2019 12:49:26 +0100 [thread overview]
Message-ID: <877e3t78l5.fsf@web.de> (raw)
In-Reply-To: <jwvzhgu7apl.fsf-monnier+emacs@gnu.org> (Stefan Monnier's message of "Sun, 17 Nov 2019 11:04:34 -0500")
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> I don't think you need the `advice--p` here since `advice--cd*r` uses the
> `*` meaning of regexps: "*zero* or more".
Ok, removed.
> > + ;; `defalias' takes care of any advises so we can just strip them
>
> Actually, you *have* to strip them (otherwise you'd end up copying them).
Sure, I made that comment clearer.
New patch:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-edebug-instrumentation-removing-from-advised-fun.patch --]
[-- Type: text/x-diff, Size: 2752 bytes --]
From aab2cd47da230993e374d378c434989c98ce68ed Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Thu, 14 Nov 2019 17:47:51 +0100
Subject: [PATCH] Fix edebug instrumentation removing from advised functions
* lisp/emacs-lisp/edebug.el (edebug-remove-instrumentation): Handle
advised functions correctly.
---
lisp/emacs-lisp/edebug.el | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 5d52704410..d68ed966f8 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -4571,6 +4571,21 @@ edebug-unload-function
;; Continue standard unloading.
nil)
+(defun edebug--unwrap*-symbol-function (symbol)
+ ;; Try to unwrap SYMBOL's `symbol-function'. The result is suitable
+ ;; to be fbound back to SYMBOL with `defalias'. When no unwrapping
+ ;; could be done return nil.
+ (pcase (symbol-function symbol)
+ ((or (and `(macro . ,f) (let was-macro t))
+ (and f (let was-macro nil)))
+ ;; `defalias' takes care of advises so we must strip them
+ (let* ((orig-f (advice--cd*r f))
+ (unwrapped (edebug-unwrap* orig-f)))
+ (cond
+ ((equal unwrapped orig-f) nil)
+ (was-macro `(macro . ,unwrapped))
+ (t unwrapped))))))
+
(defun edebug-remove-instrumentation (functions)
"Remove Edebug instrumentation from FUNCTIONS.
Interactively, the user is prompted for the function to remove
@@ -4582,10 +4597,10 @@ edebug-remove-instrumentation
(lambda (symbol)
(when (and (get symbol 'edebug)
(or (functionp symbol)
- (macrop symbol)))
- (let ((unwrapped (edebug-unwrap* (symbol-function symbol))))
- (unless (equal unwrapped (symbol-function symbol))
- (push symbol functions)))))
+ (macrop symbol))
+ (edebug--unwrap*-symbol-function
+ symbol))
+ (push symbol functions)))
obarray)
(unless functions
(error "Found no functions to remove instrumentation from"))
@@ -4599,8 +4614,9 @@ edebug-remove-instrumentation
functions)))))
;; Remove instrumentation.
(dolist (symbol functions)
- (setf (symbol-function symbol)
- (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
[-- Attachment #3: Type: text/plain, Size: 354 bytes --]
Ok to install?
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. Could be controlled via prefix argument.
Regards,
Michael.
next prev parent reply other threads:[~2019-11-21 11:49 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-13 13:55 bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions Michael Heerdegen
2019-11-14 5:20 ` Lars Ingebrigtsen
2019-11-14 16:51 ` Michael Heerdegen
2019-11-14 22:39 ` Michael Heerdegen
2019-11-15 7:57 ` Lars Ingebrigtsen
2019-11-15 12:39 ` Michael Heerdegen
2019-11-16 4:28 ` Lars Ingebrigtsen
2019-11-16 12:25 ` Michael Heerdegen
2019-11-14 16:55 ` Michael Heerdegen
2019-11-14 19:08 ` Stefan Monnier
2019-11-14 20:27 ` Michael Heerdegen
2019-11-14 21:33 ` Stefan Monnier
2019-11-15 13:54 ` Michael Heerdegen
2019-11-15 17:30 ` Stefan Monnier
2019-11-17 12:35 ` Michael Heerdegen
2019-11-17 12:55 ` Michael Heerdegen
2019-11-17 16:04 ` Stefan Monnier
2019-11-21 11:49 ` Michael Heerdegen [this message]
2019-11-23 13:32 ` Michael Heerdegen
2019-11-26 21:01 ` Michael Heerdegen
2019-11-27 12:17 ` Lars Ingebrigtsen
2020-09-20 10:54 ` Lars Ingebrigtsen
2019-11-14 21:15 ` Michael Heerdegen
2019-11-15 8:03 ` Lars Ingebrigtsen
2019-11-15 12:11 ` Michael Heerdegen
2019-11-15 12:15 ` Lars Ingebrigtsen
2019-11-15 12:34 ` Michael Heerdegen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877e3t78l5.fsf@web.de \
--to=michael_heerdegen@web.de \
--cc=38195@debbugs.gnu.org \
--cc=larsi@gnus.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).