* bug#12610: unable to use macro in defadvice
@ 2012-10-09 13:02 Le Wang
2012-10-12 2:05 ` Glenn Morris
2020-09-13 16:43 ` Lars Ingebrigtsen
0 siblings, 2 replies; 7+ messages in thread
From: Le Wang @ 2012-10-09 13:02 UTC (permalink / raw)
To: 12610
Relevant stackoverflow question:
http://stackoverflow.com/questions/12767353/using-macros-in-defadvice
I have this simplified macro.
(defadvice kill-buffer (around show-diff-rephrase-question activate compile)
"Prompt when a buffer is about to be killed."
(case (read-char-choice
"(s/k/q)? "
(append "sSKkQq" nil))
((?s ?S)
ad-do-it)
((?k ?K)
ad-do-it)
((?q ?Q) nil))
ad-do-it)
If I compile the file and load it, then I get "Invalid function: (115 83)"
It works if I evaluate it.
--
Le
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-09 13:02 bug#12610: unable to use macro in defadvice Le Wang
@ 2012-10-12 2:05 ` Glenn Morris
2012-10-12 3:46 ` Glenn Morris
2012-10-12 13:35 ` Stefan Monnier
2020-09-13 16:43 ` Lars Ingebrigtsen
1 sibling, 2 replies; 7+ messages in thread
From: Glenn Morris @ 2012-10-12 2:05 UTC (permalink / raw)
To: Le Wang; +Cc: 12610
Interesting. Advice does not macroexpand the advice definition.
I guess it should?
*** lisp/emacs-lisp/advice.el 2012-09-14 13:44:31 +0000
--- lisp/emacs-lisp/advice.el 2012-10-12 02:02:39 +0000
***************
*** 3658,3664 ****
(advice (ad-make-advice
name (memq 'protect flags)
(not (memq 'disable flags))
! `(advice lambda ,arglist ,@body)))
(preactivation (if (memq 'preactivate flags)
(ad-preactivate-advice
function advice class position))))
--- 3658,3664 ----
(advice (ad-make-advice
name (memq 'protect flags)
(not (memq 'disable flags))
! `(advice lambda ,arglist ,@(macroexpand-all body))))
(preactivation (if (memq 'preactivate flags)
(ad-preactivate-advice
function advice class position))))
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-12 2:05 ` Glenn Morris
@ 2012-10-12 3:46 ` Glenn Morris
2012-10-12 13:35 ` Stefan Monnier
1 sibling, 0 replies; 7+ messages in thread
From: Glenn Morris @ 2012-10-12 3:46 UTC (permalink / raw)
To: Le Wang; +Cc: 12610
Actually I guess it would be better for the byte-compiler to do the
expansion.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-12 2:05 ` Glenn Morris
2012-10-12 3:46 ` Glenn Morris
@ 2012-10-12 13:35 ` Stefan Monnier
2012-10-12 17:54 ` Glenn Morris
1 sibling, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2012-10-12 13:35 UTC (permalink / raw)
To: Glenn Morris; +Cc: 12610, Le Wang
> - `(advice lambda ,arglist ,@body)))
> + `(advice lambda ,arglist ,@(macroexpand-all body))))
I'd rather not do such things manually. Instead, we should expose the
code as code rather than hide it as data, so that macro-expansion
can take place without calling it explicitly and so the byte-compiler
gets to see the code, optimize it and emit warnings where needed.
I.e. I think the patch should start more along the lines of the one
below (100% guaranteed untested).
Stefan
=== modified file 'lisp/emacs-lisp/advice.el'
--- lisp/emacs-lisp/advice.el 2012-09-14 13:44:31 +0000
+++ lisp/emacs-lisp/advice.el 2012-10-12 13:25:22 +0000
@@ -3655,13 +3655,13 @@
(t (error "defadvice: Invalid or ambiguous flag: %s"
flag))))))
args))
- (advice (ad-make-advice
- name (memq 'protect flags)
- (not (memq 'disable flags))
- `(advice lambda ,arglist ,@body)))
+ (advice `(ad-make-advice
+ ',name ',(memq 'protect flags)
+ ',(not (memq 'disable flags))
+ (cons 'advice (lambda ,arglist ,@body))))
(preactivation (if (memq 'preactivate flags)
(ad-preactivate-advice
- function advice class position))))
+ function (eval advice) class position))))
;; Now for the things to be done at evaluation time:
(if (memq 'freeze flags)
;; jwz's idea: Freeze the advised definition into a dumpable
@@ -3669,7 +3669,7 @@
(ad-make-freeze-definition function advice class position)
;; the normal case:
`(progn
- (ad-add-advice ',function ',advice ',class ',position)
+ (ad-add-advice ',function ,advice ',class ',position)
,@(if preactivation
`((ad-set-cache
',function
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-12 13:35 ` Stefan Monnier
@ 2012-10-12 17:54 ` Glenn Morris
2012-10-12 18:15 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Morris @ 2012-10-12 17:54 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12610, Le Wang
Stefan Monnier wrote:
> Instead, we should expose the code as code rather than hide it as
> data, so that macro-expansion can take place without calling it
> explicitly and so the byte-compiler gets to see the code, optimize it
> and emit warnings where needed.
Sounds right.
> I.e. I think the patch should start more along the lines of the one
> below (100% guaranteed untested).
It causes warnings like:
In toplevel form:
uniquify.el:479:13:Warning: assignment to free variable `ad-return-value'
uniquify.el:487:41:Warning: reference to free variable `ad-return-value'
In end of data:
uniquify.el:511:1:Warning: the function `ad-get-arg' is not known to be
defined.
More importantly, it doesn't work...
emacs -Q -l uniquify
-> byte-code: Symbol's function definition is void: ad-make-advice
Compiling the initial example from this report gives:
(byte-code "<STUFF>" [ad-add-advice kill-buffer ad-make-advice
show-diff-rephrase-question nil t advice #[nil "<STUFF>" [#:--cl-var--
ad-do-it read-char-choice "(s/k/q)? " append "sSKkQq" nil memql (115 83)
(107 75) (113 81)] 5 "Prompt when a buffer is about to be killed."]
around ad-activate] 8)
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-12 17:54 ` Glenn Morris
@ 2012-10-12 18:15 ` Stefan Monnier
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2012-10-12 18:15 UTC (permalink / raw)
To: Glenn Morris; +Cc: 12610, Le Wang
>> I.e. I think the patch should start more along the lines of the one
>> below (100% guaranteed untested).
[...]
> More importantly, it doesn't work...
That's what I meant by "start".
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#12610: unable to use macro in defadvice
2012-10-09 13:02 bug#12610: unable to use macro in defadvice Le Wang
2012-10-12 2:05 ` Glenn Morris
@ 2020-09-13 16:43 ` Lars Ingebrigtsen
1 sibling, 0 replies; 7+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-13 16:43 UTC (permalink / raw)
To: Le Wang; +Cc: 12610
Le Wang <l26wang@gmail.com> writes:
> I have this simplified macro.
>
> (defadvice kill-buffer (around show-diff-rephrase-question activate compile)
> "Prompt when a buffer is about to be killed."
> (case (read-char-choice
> "(s/k/q)? "
> (append "sSKkQq" nil))
> ((?s ?S)
> ad-do-it)
> ((?k ?K)
> ad-do-it)
> ((?q ?Q) nil))
> ad-do-it)
>
> If I compile the file and load it, then I get "Invalid function: (115 83)"
>
> It works if I evaluate it.
I've modernised the code slightly:
(require 'cl-lib)
(defadvice kill-buffer (around show-diff-rephrase-question activate compile)
"Prompt when a buffer is about to be killed."
(cl-case (read-char-choice
"(s/k/q)? "
(append "sSKkQq" nil))
((?s ?S)
ad-do-it)
((?k ?K)
ad-do-it)
((?q ?Q) nil))
ad-do-it)
I tried byte-compiling it and loading the .elc file, and it still
doesn't bug out.
So I'm guessing this has been fixed over the years? I'm closing this
bug report; if you're still seeing this in recent Emacs versions, please
respond to the debbugs address and we'll reopen.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-13 16:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-09 13:02 bug#12610: unable to use macro in defadvice Le Wang
2012-10-12 2:05 ` Glenn Morris
2012-10-12 3:46 ` Glenn Morris
2012-10-12 13:35 ` Stefan Monnier
2012-10-12 17:54 ` Glenn Morris
2012-10-12 18:15 ` Stefan Monnier
2020-09-13 16:43 ` Lars Ingebrigtsen
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).