bug#3984: Fix for #3984

[Ryan]

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?

-Ryan


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
> > > call-interactively.
> 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
> advice).
>
> 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).
>
>
>          Stefan