GNU bug report logs - #19324
25.0.50; add-function and nil

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 19324 in the body.
You can then email your comments to 19324 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: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; add-function and nil
Date: Tue, 09 Dec 2014 12:55:22 +0800

1. Goto a buffer (e.g. text-mode) with eldoc-documentation-function nil
2. (add-function :after-until (local 'eldoc-documentation-function) #'ignore)
3. Move around to trigger eldoc error: (void-function nil)

Looks like the orig function is unconditionally called without checking
it's a function object.

Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Wed, 10 Dec 2014 14:54:42 -0500

> 1. Goto a buffer (e.g. text-mode) with eldoc-documentation-function nil
> 2. (add-function :after-until (local 'eldoc-documentation-function) #'ignore)
> 3. Move around to trigger eldoc error: (void-function nil)
> Looks like the orig function is unconditionally called without checking
> it's a function object.

Indeed.  But I don't think advice.el should handle this case.
IOW the way I see it there are 2 ways to fix this problem:
- Change the default value so it's always a function.
- Make sure all the functions you add use either :override or :around
  and check that the orig is not nil before calling it.

Obviously, the first option is much better, which is why I've been
changing various foo-function variables so that their default value is
not nil.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Thu, 11 Dec 2014 08:57:33 +0800

On 2014-12-10 14:54 -0500, Stefan Monnier wrote:
> - Make sure all the functions you add use either :override or :around
>   and check that the orig is not nil before calling it.

Yes, I had expected this to work and obviously not so filed this bug.
Otherwise it is not a bad workaround.

Go to a buffer in text mode where the default
eldoc-documentation-function is nil and eval

  (add-function :around (local 'eldoc-documentation-function)
                        (lambda (orig) (or (and orig (funcall orig)) (ignore))))

Move around to get the error.

Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Wed, 10 Dec 2014 21:48:51 -0500

> Yes, I had expected this to work and obviously not so filed this bug.
> Otherwise it is not a bad workaround.

> Go to a buffer in text mode where the default
> eldoc-documentation-function is nil and eval

>   (add-function :around (local 'eldoc-documentation-function)
>                         (lambda (orig) (or (and orig (funcall orig)) (ignore))))

> Move around to get the error.

Ah, right, damn: the (local <foo>) causes the insertion of a "proxy
function" which runs the global value, but that doesn't check if
it's nil.
IOW it only works for :override.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Thu, 11 Dec 2014 11:54:50 +0800

On 2014-12-10 21:48 -0500, Stefan Monnier wrote:
> Ah, right, damn: the (local <foo>) causes the insertion of a "proxy
> function" which runs the global value, but that doesn't check if it's
> nil. IOW it only works for :override.

Sounds like a bug ;)

Leo

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Thu, 11 Dec 2014 12:01:31 -0500

>> Ah, right, damn: the (local <foo>) causes the insertion of a "proxy
>> function" which runs the global value, but that doesn't check if it's
>> nil. IOW it only works for :override.
> Sounds like a bug ;)

Indeed, I fixed the eldoc-documentation-function not to be nil.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Fri, 12 Dec 2014 01:46:27 +0800

On 2014-12-11 12:01 -0500, Stefan Monnier wrote:
> Indeed, I fixed the eldoc-documentation-function not to be nil.
>
>
>         Stefan

But :around are still broken. It promises to pass on ORIG but instead
passes a proxy. so the advice has no (public) way to tell missing
function. Do you have a fix for this? Otherwise we have to check and set
a dummy value to be safe, for example¹.

Leo

Footnotes: 
¹  https://github.com/leoliu/ggtags/commit/115194cc27dd578b986aaaf0dc390bb57e9838b2

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 19324 <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Thu, 11 Dec 2014 14:11:25 -0500

> But :around are still broken.  It promises to pass on ORIG but instead
> passes a proxy.  So the advice has no (public) way to tell missing
> function.  Do you have a fix for this?

No I don't have a fix for this.
`add-function' is designed to work on places that only hold functions,
so any other value (such as nil) will create problems.

IOW a variable that can hold "either nil or a function" is not something
that add-function supports.

I guess we could treat nil as an alias for `ignore' in the "proxy
function", which would fix this particular issue.  See patch below.
But I don't intend to handle all the cases in which a "nil function" can
show up.  Many/most uses of `foo-function' actually give a special
meaning to nil which is different from `ignore'.
So I'm not sure we should cater to this particular case.

> Otherwise we have to check and set a dummy value to be safe, for
> example¹.

That looks like a good workaround.


        Stefan


diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index a81d3e4..7e1c236 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -234,7 +234,7 @@ different, but `function-equal' will hopefully ignore those differences.")
   (if (local-variable-p var) (symbol-value var)
     (setq advice--buffer-local-function-sample
           ;; This function acts like the t special value in buffer-local hooks.
-          (lambda (&rest args) (apply (default-value var) args)))))
+          (lambda (&rest args) (apply (or (default-value var) #'ignore) args)))))
 
 (eval-and-compile
   (defun advice--normalize-place (place)

Message #31 received at 19324-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 19324-done <at> debbugs.gnu.org
Subject: Re: bug#19324: 25.0.50; add-function and nil
Date: Fri, 12 Dec 2014 08:44:11 +0800

On 2014-12-11 14:11 -0500, Stefan Monnier wrote:
> No I don't have a fix for this.
> `add-function' is designed to work on places that only hold functions,
> so any other value (such as nil) will create problems.
>
> IOW a variable that can hold "either nil or a function" is not something
> that add-function supports.
>
> I guess we could treat nil as an alias for `ignore' in the "proxy
> function", which would fix this particular issue.  See patch below.
> But I don't intend to handle all the cases in which a "nil function" can
> show up.  Many/most uses of `foo-function' actually give a special
> meaning to nil which is different from `ignore'.
> So I'm not sure we should cater to this particular case.

Thanks, Stefan, for the explanation. In that case maybe the workaround
isn't needed because we still cannot tell if ORIG is missing. I consider
the bug closed.

Leo

Previous Next

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