* 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 external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.