bug#5863: defadvice in byte compiled file does not work

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

I have this in mumamo.el:

  (defvar mumamo-stop-widen nil)
  (defadvice widen (around
                    mumamo-ad-widen
                    activate
                    compile)
    (unless (and mumamo-multi-major-mode
                 mumamo-stop-widen)
      ad-do-it))

I let bind mumamo-stop-widen to avoid widening for certain situations.
This works sometimes and not other times. It looks like the defadvice
is simply skipped the other times (but I am not sure).

It looks like it depends on byte compilation in some way. Or actually
several ways. I have seen instances where it helps to eval the
defadvice and other cases where it does not. If the function calling
(widen) is byte compiled it does not work, but it works if the
function is evaled. However it looks like both these conditions must
be meat.

Beeing able to do something like this is very essential to get multi
major modes working reliably. Is there any remedy for this problem?

If not, could we please add a way to control if (widen) widens the
buffer (or how it does it, but that seems to complicated to me)?

bug#5863: defadvice in byte compiled file does not work

[Andreas Schwab]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> It looks like the defadvice is simply skipped the other times (but I
> am not sure).

widen has its own byte code.  Advices also never work on direct calls to
builtin functions.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 5:50 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> It looks like the defadvice is simply skipped the other times (but I
>> am not sure).
>
> widen has its own byte code.  Advices also never work on direct calls to
> builtin functions.


Thanks Andreas, but what does it mean that it has its own byte code?
Why is it important here?

Reading (info "(elisp) Advising Primitives") it looks to me that it
should work in those cases I want it to work since the calls to
"(widen)" I want to intercept are in elisp code. That info page says

  "Calls to the primitive from Lisp code
    will take note of the advice."

It looks to me like a regression, but I am not quite sure. I am just
trying to find out but it takes me some minutes.

bug#5863: defadvice in byte compiled file does not work

[Andreas Schwab]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Thanks Andreas, but what does it mean that it has its own byte code?
> Why is it important here?

Because it translates into a direct call to the builtin function.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> Thanks Andreas, but what does it mean that it has its own byte code?
>> Why is it important here?
>
> Because it translates into a direct call to the builtin function.


You mean during byte compilation? That is then a bug since the
documentation says it should work.


I just looked a bit more and it looks like the defadvice does not
result in that `widen' is advised at all after loading mumamo.el. As
far as I understand that is another bug.

Looking into this second bug I can see that `widen' does not get
advised if I do eval-buffer on mumamo.el. However if I go to the
defadvice and directly eval it with C-M-x or C-x C-e widen gets
advised (and things starts working).

bug#5863: defadvice in byte compiled file does not work

[Andreas Schwab]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>
>>> Thanks Andreas, but what does it mean that it has its own byte code?
>>> Why is it important here?
>>
>> Because it translates into a direct call to the builtin function.
>
>
> You mean during byte compilation? That is then a bug since the
> documentation says it should work.

Which documentation says what?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 6:55 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>>
>>>> Thanks Andreas, but what does it mean that it has its own byte code?
>>>> Why is it important here?
>>>
>>> Because it translates into a direct call to the builtin function.
>>
>>
>> You mean during byte compilation? That is then a bug since the
>> documentation says it should work.
>
> Which documentation says what?


The info page I pointed to says that defadvice should work for
primitives called from lisp code. And that is what does not work for
me here.

bug#5863: defadvice in byte compiled file does not work

[Andreas Schwab]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> The info page I pointed to says that defadvice should work for
> primitives called from lisp code.

The primitive is no longer called from lisp code when byte compiled.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> The info page I pointed to says that defadvice should work for
>> primitives called from lisp code.
>
> The primitive is no longer called from lisp code when byte compiled.


I think that is wrong. Can you point me to something that support your
statement?

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 7:06 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>
>>> The info page I pointed to says that defadvice should work for
>>> primitives called from lisp code.
>>
>> The primitive is no longer called from lisp code when byte compiled.
>
>
> I think that is wrong. Can you point me to something that support your
> statement?


Looking a bit at the Emacs sources I can see that at least follow.el,
tramp.el, uniquify.el, ada-mode.el, viper.el do defadvice primitives.

It is a bit ironic that those defadvice "inside Emacs" works and not
those made "outside of Emacs" ... ;-)

BTW, is not ECB dependent on this to work too?

bug#5863: defadvice in byte compiled file does not work

[Andreas Schwab]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>
>>> The info page I pointed to says that defadvice should work for
>>> primitives called from lisp code.
>>
>> The primitive is no longer called from lisp code when byte compiled.
>
>
> I think that is wrong. Can you point me to something that support your
> statement?

See Fbyte_code.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Thu, Apr 8, 2010 at 11:52 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>>
>>>> The info page I pointed to says that defadvice should work for
>>>> primitives called from lisp code.
>>>
>>> The primitive is no longer called from lisp code when byte compiled.
>>
>>
>> I think that is wrong. Can you point me to something that support your
>> statement?
>
> See Fbyte_code.


Hm, thanks, yes I can see how it is implemented now and I can see good
reasons for optimizing it this way. However the manual does not say
that byte compilation removes the check during execution if primitives
are adviced or not.

And I am a bit irritated that it doesn't since I trusted the manual.... ;-)

Could we please correct the manual?

And could we as soon as possible please remove the possibility to
defadvice primitives at all? The kind of bugs produced by different
execution paths for evaled and byte compiled code is rather difficult
to nail down.


(I have to do a totally different workaround instead of defadvice
widen. But that is another problem.)

bug#5863: defadvice in byte compiled file does not work

[Lennart Borgman]

On Fri, Apr 9, 2010 at 1:13 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Thu, Apr 8, 2010 at 11:52 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>
>>> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>>> Lennart Borgman <lennart.borgman@gmail.com> writes:
>>>>
>>>>> The info page I pointed to says that defadvice should work for
>>>>> primitives called from lisp code.
>>>>
>>>> The primitive is no longer called from lisp code when byte compiled.
>>>
>>>
>>> I think that is wrong. Can you point me to something that support your
>>> statement?
>>
>> See Fbyte_code.
>
>
> Hm, thanks, yes I can see how it is implemented now and I can see good
> reasons for optimizing it this way. However the manual does not say
> that byte compilation removes the check during execution if primitives
> are adviced or not.
>
> And I am a bit irritated that it doesn't since I trusted the manual.... ;-)
>
> Could we please correct the manual?
>
> And could we as soon as possible please remove the possibility to
> defadvice primitives at all? The kind of bugs produced by different
> execution paths for evaled and byte compiled code is rather difficult
> to nail down.
>
>
> (I have to do a totally different workaround instead of defadvice
> widen. But that is another problem.)


I have done a new workaround. This depends on the bug that primitives
can be defadviced in evaled code. So please do not correct the bug
without giving any possibility to avoid (widen).... ;-)

bug#5863: defadvice in byte compiled file does not work

[Stefan Monnier]

> The info page I pointed to says that defadvice should work for
> primitives called from lisp code.  And that is what does not work for
> me here.

For such uses, there are several categories of functions:
- functions that have their own byte-code (things like widen, car, cdr, ...).
- functions that are implemented in C but don't have their own
  byte-code.
- other functions.

Pieces of advice on the first kind of functions only work for
calls from interpreted Lisp code.
Pieces of advice on the second kind of functions only work for
calls from Lisp code (both interpreted and byte-compiled).


        Stefan

bug#5863: defadvice in byte compiled file does not work

[Drew Adams]

> > The info page I pointed to says that defadvice should work for
> > primitives called from lisp code.  And that is what does 
> > not work for me here.
> 
> For such uses, there are several categories of functions:
> - functions that have their own byte-code (things like widen, 
> car, cdr, ...).
> - functions that are implemented in C but don't have their own
>   byte-code.
> - other functions.
> 
> Pieces of advice on the first kind of functions only work for
> calls from interpreted Lisp code.
> Pieces of advice on the second kind of functions only work for
> calls from Lisp code (both interpreted and byte-compiled).

Please document the behavior in the manual, not just here. Thx.

bug#5863: defadvice in byte compiled file does not work

[Stefan Kangas]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The info page I pointed to says that defadvice should work for
>> primitives called from lisp code.  And that is what does not work for
>> me here.
>
> For such uses, there are several categories of functions:
> - functions that have their own byte-code (things like widen, car, cdr, ...).
> - functions that are implemented in C but don't have their own
>   byte-code.
> - other functions.
>
> Pieces of advice on the first kind of functions only work for
> calls from interpreted Lisp code.
> Pieces of advice on the second kind of functions only work for
> calls from Lisp code (both interpreted and byte-compiled).

BTW, it seems like advising primitives will work once we get native-comp
merged:

  The result of this procedure is that each newly activated function will
  use the trampoline in place of the original primitive and the
  trampoline will execute the call going through funcall making the
  advice effective!

  This works so well that in-fact now is even possible to advice
  effectively what wasn't effective in byte-code (ex the + function). But
  hey, don't try this a home!

  https://akrl.sdf.org/gccemacs.html#org3b7398e

I'm not sure how this would affect the status of this bug, if at all.
Perhaps we could expand the description of why advising primitives is a
bad idea in the Info node `(elisp) Advising Named Functions':

  It is possible to advise a primitive (*note What Is a Function::),
  but one should typically _not_ do so, for two reasons.  Firstly, some
  primitives are used by the advice mechanism, and advising them could
  cause an infinite recursion.  Secondly, many primitives are called
  directly from C, and such calls ignore advice; hence, one ends up in a
  confusing situation where some calls (occurring from Lisp code) obey the
  advice and other calls (from C code) do not.

We could perhaps simply say that advice on primitives with their own
bytecode will be ineffective if the code is run byte-compiled.

bug#5863: defadvice in byte compiled file does not work

[Stefan Monnier]

> BTW, it seems like advising primitives will work once we get native-comp
> merged:
>
>   The result of this procedure is that each newly activated function will
>   use the trampoline in place of the original primitive and the
>   trampoline will execute the call going through funcall making the
>   advice effective!
>
>   This works so well that in-fact now is even possible to advice
>   effectively what wasn't effective in byte-code (ex the + function). But
>   hey, don't try this a home!
>
>   https://akrl.sdf.org/gccemacs.html#org3b7398e

AFAIK this still won't cover all cases: we'd additionally have to
arrange for C calls to F<fun> to go through the trampoline.  Maybe we
can do that cheaply, I don't know.

In any case, I think the story is still sufficiently complex that we're
better off saying that "it may work or not depending on your luck".


        Stefan

bug#5863: defadvice in byte compiled file does not work

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> BTW, it seems like advising primitives will work once we get native-comp
>> merged:
>>
>>   The result of this procedure is that each newly activated function will
>>   use the trampoline in place of the original primitive and the
>>   trampoline will execute the call going through funcall making the
>>   advice effective!
>>
>>   This works so well that in-fact now is even possible to advice
>>   effectively what wasn't effective in byte-code (ex the + function). But
>>   hey, don't try this a home!
>>
>>   https://akrl.sdf.org/gccemacs.html#org3b7398e
>
> AFAIK this still won't cover all cases: we'd additionally have to
> arrange for C calls to F<fun> to go through the trampoline.  Maybe we
> can do that cheaply, I don't know.

I think there's no way we'll be able to fix the C to C case without some
very measurable performance impact.

For essentially the same reason the nativecomp trick does not work for
the (few) functions we opencode.

Ex on optimized compiled code redefining or advising `1+' will not take
effect.  So yeah works better but is still not perfect.

> In any case, I think the story is still sufficiently complex that we're
> better off saying that "it may work or not depending on your luck".

thus agree.

  Andrea

bug#5863: defadvice in byte compiled file does not work

[Stefan Kangas]

Andrea Corallo <akrl@sdf.org> writes:

>> In any case, I think the story is still sufficiently complex that we're
>> better off saying that "it may work or not depending on your luck".
>
> thus agree.

Agreed.  Sound like this should be closed as wontfix?

bug#5863: defadvice in byte compiled file does not work

[Stefan Kangas]

tags 5863 + wontfix
close 5863
thanks

Stefan Kangas <stefan@marxist.se> writes:

> >> In any case, I think the story is still sufficiently complex that we're
> >> better off saying that "it may work or not depending on your luck".
> >
> > thus agree.
>
> Agreed.  Sound like this should be closed as wontfix?

No further comments within a week, so let's assume yes.  I'm therefore
closing this bug now.