[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adv
From: |
Stefan Monnier |
Subject: |
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions |
Date: |
Thu, 14 Nov 2019 14:08:24 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
> +(defun edebug--advised-p (symbol)
> + ;; Non-nil when SYMBOL's `symbol-function' is advised. The non-nil
> + ;; return value is the unwrapped base function if it was wrapped,
> + ;; and the symbol t else.
Do you really mean "advised"? It seems that this tests whether the code
is *instrumented* by looking past the (potential) pieces of advice.
I'll assume the above should say "instrumented", but if not, then
disregard the rest because I'm just confused.
> + (pcase (symbol-function symbol)
> + ((and (pred advice--p)
> + (app advice--cd*r orig-f)
> + (let unwrapped (edebug-unwrap* orig-f)))
> + (if (equal unwrapped orig-f) t unwrapped))
> + (`(macro . ,(and (pred advice--p)
> + (app advice--cd*r orig-f)
> + (let unwrapped (edebug-unwrap* orig-f))))
> + (if (equal unwrapped orig-f) t `(macro . ,unwrapped)))))
[ That's pretty ugly. I think I'd move the `app` and the `let` outside
of those patterns and into the code of the corresponding branch. ]
> - (let ((unwrapped (edebug-unwrap* (symbol-function symbol))))
> - (unless (equal unwrapped (symbol-function symbol))
> - (push symbol functions)))))
> + (if-let ((advised (edebug--advised-p symbol)))
> + (unless (eq advised t)
> + (push symbol functions))
> + (let ((unwrapped (edebug-unwrap*
> + (symbol-function symbol))))
> + (unless (equal unwrapped (symbol-function symbol))
> + (push symbol functions))))))
How 'bout using something like
(defun edebug--symbol-function (sym)
(let ((def (symbol-function sym)))
(if (eq 'macro (car-safe def))
(setq def (cdr def)))
(advice--cdr* def)))
and replacing (some) uses of `symbol-function` with it?
> ;; Remove instrumentation.
> (dolist (symbol functions)
> - (setf (symbol-function symbol)
> - (edebug-unwrap* (symbol-function symbol))))
> + (if-let ((advised (edebug--advised-p symbol)))
> + (unless (eq advised t)
> + (funcall (or (get symbol 'defalias-fset-function) #'fset)
> + symbol advised))
> + (setf (symbol-function symbol)
> + (edebug-unwrap* (symbol-function symbol)))))
Yuck!
Can't we just use `defalias` rather than `fset` (and that should
take care of calling `defalias-fset-function` when needed)?
> I'm not sure if I should use `indirect-function' somewhere instead of
> `symbol-function',
That's a question for Edebug's ;-)
In the above I assumed that there's a good reason why it uses
`symbol-function` so I preserved it.
> but I also don't understand (and also didn't yet try to find out) why
> nadvice doesn't use `indirect-function' for its manipulations.
That's because if I have `foo` as an alias for `bar` and I advise `foo`
I don't want it to affect `bar`: if you want to affect both, then you
should advise `bar`.
Stefan
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/13
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Lars Ingebrigtsen, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Lars Ingebrigtsen, 2019/11/15
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/15
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Lars Ingebrigtsen, 2019/11/15
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/16
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions,
Stefan Monnier <=
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Stefan Monnier, 2019/11/14
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/15
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Stefan Monnier, 2019/11/15
- bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/17
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/17
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Stefan Monnier, 2019/11/17
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/21
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/23
bug#38195: 27.0.50; `edebug-remove-instrumentation' doesn't work for adviced functions, Michael Heerdegen, 2019/11/26