Problems with advice...

Problems with advice...

[David Kastrup]

I have the problem that preactivated advice no longer gets
preactivated.  However, the same problem occurs now with a current
compilation of Emacs-21.3!  So I suspect that it might be
compiler-related (gcc-4.0).  It worked at one point of time in the
past.

Whatever.  In the search for the problem I have also come across the
following code snippet in advice.el:

		    ((and orig-subr-p
			  orig-interactive-p
			  (not interactive-form)
			  (not advised-interactive-form))
		     ;; Check whether we were called interactively
		     ;; in order to do proper prompting:
		     `(if (called-interactively-p)
			  (call-interactively ',origname)
			,(ad-make-mapped-call orig-arglist
					      advised-arglist
					      origname)))
		    ;; And now for normal functions and non-interactive subrs
	            ;; (or subrs whose interactive behavior was advised):
		    (t (ad-make-mapped-call
			advised-arglist orig-arglist origname)))))

This looks so wrong that I want somebody with more of a clue to take a
look at it: ad-make-mapped-call is called in two branches of a cond,
and the order of its first two arguments is interchanged in those two
calls!

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Problems with advice...

[David Kastrup]

David Kastrup <dak@gnu.org> writes:

> I have the problem that preactivated advice no longer gets
> preactivated.  However, the same problem occurs now with a current
> compilation of Emacs-21.3!  So I suspect that it might be
> compiler-related (gcc-4.0).  It worked at one point of time in the
> past.

Update on that: seemingly related to loading a completely unrelated
package (that does a "require" on the file containing the function to
be advised), so not a compiler error after all.  I have to debug this
further.

In the mean time, the following finding still looks bad:

> Whatever.  In the search for the problem I have also come across the
> following code snippet in advice.el:
>
> 		    ((and orig-subr-p
> 			  orig-interactive-p
> 			  (not interactive-form)
> 			  (not advised-interactive-form))
> 		     ;; Check whether we were called interactively
> 		     ;; in order to do proper prompting:
> 		     `(if (called-interactively-p)
> 			  (call-interactively ',origname)
> 			,(ad-make-mapped-call orig-arglist
> 					      advised-arglist
> 					      origname)))
> 		    ;; And now for normal functions and non-interactive subrs
> 	            ;; (or subrs whose interactive behavior was advised):
> 		    (t (ad-make-mapped-call
> 			advised-arglist orig-arglist origname)))))
>
> This looks so wrong that I want somebody with more of a clue to take a
> look at it: ad-make-mapped-call is called in two branches of a cond,
> and the order of its first two arguments is interchanged in those two
> calls!

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Problems with advice...

[Stefan Monnier]

> This looks so wrong that I want somebody with more of a clue to take a
> look at it: ad-make-mapped-call is called in two branches of a cond,
> and the order of its first two arguments is interchanged in those two
> calls!

I think the patch below is the right one,


        Stefan


--- orig/lisp/emacs-lisp/advice.el
+++ mod/lisp/emacs-lisp/advice.el
@@ -3108,8 +3108,8 @@
 		     ;; in order to do proper prompting:
 		     `(if (called-interactively-p)
 			  (call-interactively ',origname)
-			,(ad-make-mapped-call orig-arglist
-					      advised-arglist
+			,(ad-make-mapped-call advised-arglist
+					      orig-arglist
 					      origname)))
 		    ;; And now for normal functions and non-interactive subrs
 	            ;; (or subrs whose interactive behavior was advised):