GNU bug report logs - #13023
24.3.50; lexical binding does not work within defadvice

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13023 in the body.
You can then email your comments to 13023 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Schmidt <christopher <at> ch.ristopher.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; lexical binding does not work within defadvice
Date: Wed, 28 Nov 2012 16:17:17 +0000 (GMT)

    ;; -*- lexical-binding: t -*-

    (funcall (let ((rms "works"))
               (lambda ()
                 (message "lex-bind %s" rms))))

    (defun asdf (b) (funcall b))

    (defadvice asdf (before rms (b) activate)
      (setf b (let ((abc 1) (b b)) (lambda () (print abc) (funcall b)))))

    (asdf 'ding)

I think this code should work fine.  It doesn't work with trunk and
emacs-24, though:

    Load test.elc? (y or n)  y
    Loading test.elc...
    lex-bind works
    Load error for test.elc:
    (void-variable abc)

        Christopher

Message #8 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Wed, 28 Nov 2012 23:32:36 -0500

>     ;; -*- lexical-binding: t -*-
>     (funcall (let ((rms "works"))
>                (lambda ()
>                  (message "lex-bind %s" rms))))

>     (defun asdf (b) (funcall b))

>     (defadvice asdf (before rms (b) activate)
>       (setf b (let ((abc 1) (b b)) (lambda () (print abc) (funcall b)))))

>     (asdf 'ding)

> I think this code should work fine.  It doesn't work with trunk and
> emacs-24, though:

Indeed, it doesn't work.  And because of the way advice.el works
(building a new function by combining the code chunks from all the
pieces of advice applied to that function) it's not easy to fix.

This is partly related to the issue mentioned recently that macro calls
in pieces of advice are expanded late (typically when the advised
function is defined).

BTW, this does not affect the new `advice-add' feature in Emacs trunk.


        Stefan

Message #11 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Thu, 29 Nov 2012 17:50:02 -0500

    Indeed, it doesn't work.  And because of the way advice.el works
    (building a new function by combining the code chunks from all the
    pieces of advice applied to that function) it's not easy to fix.

Would it be possible to fix this by defining a new primitive construct
for use in the constructed function that runs the advince?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Message #14 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: rms <at> gnu.org
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Thu, 29 Nov 2012 22:45:01 -0500

>     Indeed, it doesn't work.  And because of the way advice.el works
>     (building a new function by combining the code chunks from all the
>     pieces of advice applied to that function) it's not easy to fix.
> Would it be possible to fix this by defining a new primitive construct
> for use in the constructed function that runs the advice?

I don't think there's a need to add any new primitive.  If someone wants
to dig into advice.el to try and fix it, it can be fixed there (as
evidenced by advice-add).
I personally would rather tell people to use advice-add instead.


        Stefan

Message #17 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Fri, 30 Nov 2012 15:11:15 -0500

    I don't think there's a need to add any new primitive.  If someone wants
    to dig into advice.el to try and fix it, it can be fixed there (as
    evidenced by advice-add).
    I personally would rather tell people to use advice-add instead.

That would, in effect, be an incompatible change in interface.
Why make the change?  Is there something wrong with defadvice?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Message #20 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: rms <at> gnu.org
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Fri, 30 Nov 2012 23:23:18 -0500

> Is there something wrong with defadvice?

Yes: complexity.  Compare nadvice.el and advice.el.  Even after you
remove the long comment at the beginning, the difference in code size
speaks for itself.


        Stefan

Message #23 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13023 <at> debbugs.gnu.org, Richard Stallman <rms <at> gnu.org>
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Sun, 2 Dec 2012 01:00:20 +0100

> Even after you
> remove the long comment at the beginning, the difference in code size
> speaks for itself.

Exactly 1,000 lines longer, after removing comments and blank lines:
1,305 vs 305.

(Bored, moi?)

    Juanma

Message #26 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Sat, 01 Dec 2012 23:15:56 -0500

    Yes: complexity.  Compare nadvice.el and advice.el.

The main difference seems to me that part of the arguments of
advice.el get pushed into the function that you need to pass
to advice-add.  So the complexity of using it is no more.
Also, you lose the ability to modify the argument values before
the advised function's body.

The doc string of advice-add is incomplete, so I cannot tell
what all its args mean or what it can really do.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Message #29 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: monnier <at> iro.umontreal.ca, 13023 <at> debbugs.gnu.org
Cc: rms <at> gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Sun, 02 Dec 2012 19:01:40 -0500

Correction:

    Yes: complexity.  Compare nadvice.el and advice.el.

The main difference seems to me that part of the arguments of
advice.el get pushed into the function that you need to pass
to advice-add.  So the complexity of using advice-add is no less.
Also, you lose the ability to modify the argument values before
the advised function's body.

The doc string of advice-add is incomplete, so I cannot tell
what all its args mean or what it can really do.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

Message #32 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Tue, 02 Feb 2016 19:35:35 +0100

On 2012-11-28, at 23:32, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:

>>     ;; -*- lexical-binding: t -*-
>>     (funcall (let ((rms "works"))
>>                (lambda ()
>>                  (message "lex-bind %s" rms))))
>
>>     (defun asdf (b) (funcall b))
>
>>     (defadvice asdf (before rms (b) activate)
>>       (setf b (let ((abc 1) (b b)) (lambda () (print abc) (funcall b)))))
>
>>     (asdf 'ding)
>
>> I think this code should work fine.  It doesn't work with trunk and
>> emacs-24, though:
>
> Indeed, it doesn't work.  And because of the way advice.el works
> (building a new function by combining the code chunks from all the
> pieces of advice applied to that function) it's not easy to fix.
>
> This is partly related to the issue mentioned recently that macro calls
> in pieces of advice are expanded late (typically when the advised
> function is defined).
>
> BTW, this does not affect the new `advice-add' feature in Emacs trunk.

Hi Stefan,
hi all,

does the above mean that this bug should be closed?  AFAIU, new code
should (at least usually) use advice-add, and while the "old" advice
system is not going to be deleted, it is kind of deprecated.

Best,

-- 
Marcin Borkowski
http://mbork.pl/en

Message #35 received at 13023 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: 13023 <at> debbugs.gnu.org
Subject: Re: bug#13023: 24.3.50; lexical binding does not work within defadvice
Date: Tue, 02 Feb 2016 14:42:37 -0500

> does the above mean that this bug should be closed?

To the extent that I don't know of anyone who intends to fix this
problem, I guess we could close it as "won't fix".


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.