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

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
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)?





Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
Lennart Borgman <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 5:50 PM, Andreas Schwab <[hidden email]> wrote:
> Lennart Borgman <[hidden email]> 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.




Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
Lennart Borgman <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <[hidden email]> wrote:
> Lennart Borgman <[hidden email]> 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).




Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
Lennart Borgman <[hidden email]> writes:

> On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <[hidden email]> wrote:
>> Lennart Borgman <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 6:55 PM, Andreas Schwab <[hidden email]> wrote:

> Lennart Borgman <[hidden email]> writes:
>
>> On Thu, Apr 8, 2010 at 6:06 PM, Andreas Schwab <[hidden email]> wrote:
>>> Lennart Borgman <[hidden email]> 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.




Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
Lennart Borgman <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <[hidden email]> wrote:
> Lennart Borgman <[hidden email]> 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?




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 7:06 PM, Lennart Borgman
<[hidden email]> wrote:

> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <[hidden email]> wrote:
>> Lennart Borgman <[hidden email]> 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?




Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
In reply to this post by Lennart Borgman (gmail)
Lennart Borgman <[hidden email]> writes:

> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <[hidden email]> wrote:
>> Lennart Borgman <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Thu, Apr 8, 2010 at 11:52 PM, Andreas Schwab <[hidden email]> wrote:

> Lennart Borgman <[hidden email]> writes:
>
>> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <[hidden email]> wrote:
>>> Lennart Borgman <[hidden email]> 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.)




Reply | Threaded
Open this post in threaded view
|

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

Lennart Borgman (gmail)
On Fri, Apr 9, 2010 at 1:13 AM, Lennart Borgman
<[hidden email]> wrote:

> On Thu, Apr 8, 2010 at 11:52 PM, Andreas Schwab <[hidden email]> wrote:
>> Lennart Borgman <[hidden email]> writes:
>>
>>> On Thu, Apr 8, 2010 at 7:04 PM, Andreas Schwab <[hidden email]> wrote:
>>>> Lennart Borgman <[hidden email]> 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).... ;-)




Reply | Threaded
Open this post in threaded view
|

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

Stefan Monnier
In reply to this post by Lennart Borgman (gmail)
> 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




Reply | Threaded
Open this post in threaded view
|

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.





Reply | Threaded
Open this post in threaded view
|

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

Stefan Kangas
In reply to this post by Stefan Monnier
Stefan Monnier <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

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

Stefan Kangas
Andrea Corallo <[hidden email]> 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?



Reply | Threaded
Open this post in threaded view
|

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

Stefan Kangas
tags 5863 + wontfix
close 5863
thanks

Stefan Kangas <[hidden email]> 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.