[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#3984: Fix for #3984

From: Ryan
Subject: bug#3984: Fix for #3984
Date: Mon, 16 Sep 2013 20:18:23 -0700
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130801 Thunderbird/17.0.8

Ok, I think I figured out how to do it for all advice, but feel free to poke holes in my idea before I get to far implementing it.

We have "ad-is-advised" which we can use to find which stack frames correspond to advised functions. We have "ad-get-orig-definition" which we can use to find the original definition of an advised function. We can tell if there is any active advice by testing if the advised function's definition is equal to the original definition. If the function has active advice, we can search down the stack for the original definition of that function and skip over all the stack frames in between.

Unfortunately, this requires a search through the call stack frames in the wrong order, so the best strategy may be to make a single pass through the call stack, finding all pairs of advised/original function stack frames, and then cache the result in a dynamically bound variable for the duration of the call to "called-interactively-p". This still requires a complete pass through the entire call stack for each call to "called-interactively-p", which defeats the current "lazy-evaluation" strategy of inspecting the frames one by one, although I guess we can stop at the first occurrence of "call-interactively" on the stack. But I doubt that "called-interactively-p" is a performance hotspot anyway, so this may be acceptable.

What do you think?


On 9/13/13 2:02 PM, Stefan Monnier wrote:
Looking at the code in trunk, I see that there is a special hook for
functions to decide which stack frames to skip over when looking for
call-interactively. I still think they should relax the test for
equality to "equal indirect-functions" instead of exactly the symbol
The code does check "equal modulo indirect-functions" in some cases, but
indeed not all.  I don't think that replacing the equality check against
`call-interactively' with a check modulo indirect-functions would solve
your problem, tho (that only helps when calling though an alias of
call-interactively, but here the relevant stack frame will be a call to
the #<subr call-interactively> which is not
equal-modulo-indirect-functions to call-interactively since
call-interactively has been redefined to a different functions by the

You can probably use called-interactively-p-functions to detect the
#<subr call-interactively> and skip the frames between it and the
corresponding call to `call-interactively'.

But if you find a cute patch against the current code which makes it
work for you in a cleanish way, do send it here, to see if it can
be included.

Actually, I just noticed that in trunk, nadvice.el adds a function to
"called-interactively-p-functions" to skip advice-related stack frames, but
this works only for advice on the interactive function, not advice defined
on call-interactively itself.
Indeed.  It doesn't even work for all advices (more specifically it
doesn't work for :around advices, which means it doesn't work for
advices defined via `defadvice' since these all turn into one
big :around "new advice").

Furthermore, from my limited testing it appears that the structure of
the call stack for advised functions has changes significantly in
trunk, making my code obsolete.
Indeed, the implementation of advices has been completely changed.

The whole thing looks like a work in progress right now.
There's no planned change to it, so I consider it "ready modulo
bug-reports".  AFAIK it works "at least as well as before" (it works
better than before in the sense that Edebugging a function with calls
to called-interactively-p should now work correctly).

`called-interactively-p' is a big ugly hack and only works sometimes.
It can break in all kinds of cases (e.g. it currently won't work in
byte-compiled lexical-binding code within a `catch', or
a `condition-case', or the unwind part of an `unwind-protect').  Also,
the functions called (non-interactively, obviously) by your
`call-interactively' advice will probably think that they're called
interactively (hopefully your advice doesn't call many functions, and
hopefully none of them cares whether it's called interactively or not).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]