GNU bug report logs - #24697
25.1; find-lisp-object-file-name may return wrong locations

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 24697 in the body.
You can then email your comments to 24697 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: Alex <agrambot <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; find-lisp-object-file-name may return wrong locations
Date: Fri, 14 Oct 2016 16:19:23 -0600

#+BEGIN_SRC elisp
(defface tab-width
  '((t :foreground "red"))
  "A face with the same name as a variable.")


(find-lisp-object-file-name 'tab-width 'defface)
#+END_SRC

The above results in the symbol 'C-source'. The result should be nil as
the 'tab-width' face was evaluated in *scratch*.

Putting the above definition into an actual file and using `load-file'
does return the proper source location.

#+BEGIN_SRC elisp
(find-lisp-object-file-name 'mapatoms 1)
#+END_SRC

This should return 'C-source' but instead it returns nil.

#+BEGIN_SRC elisp
(find-lisp-object-file-name 'tab-width 1)
#+END_SRC

This should return nil but instead it returns 'C-source'.

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

From: Alex <agrambot <at> gmail.com>
To: 24697 <at> debbugs.gnu.org
Subject: Re: bug#24697: 25.1;
 find-lisp-object-file-name may return wrong locations
Date: Sat, 17 Jun 2017 16:26:47 -0600

[Message part 1 (text/plain, inline)]

Alex <agrambot <at> gmail.com> writes:

> #+BEGIN_SRC elisp
> (defface tab-width
>   '((t :foreground "red"))
>   "A face with the same name as a variable.")
>
>
> (find-lisp-object-file-name 'tab-width 'defface)
> #+END_SRC
>
>
> The above results in the symbol 'C-source'. The result should be nil as
> the 'tab-width' face was evaluated in *scratch*.
>
> Putting the above definition into an actual file and using `load-file'
> does return the proper source location.
>
> #+BEGIN_SRC elisp
> (find-lisp-object-file-name 'mapatoms 1)
> #+END_SRC
>
>
> This should return 'C-source' but instead it returns nil.
>
> #+BEGIN_SRC elisp
> (find-lisp-object-file-name 'tab-width 1)
> #+END_SRC
>
> This should return nil but instead it returns 'C-source'.

The first and last of these are fixed by the following diff.

[find-lisp-object.diff (text/x-diff, inline)]

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 2c635ffa50..bcf33131fa 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -319,6 +319,7 @@ find-lisp-object-file-name
 	  (help-C-file-name type 'subr)
 	'C-source))
      ((and (not file-name) (symbolp object)
+           (eq type 'defvar)
 	   (integerp (get object 'variable-documentation)))
       ;; A variable defined in C.  The form is from `describe-variable'.
       (if (get-buffer " *DOC*")

[Message part 3 (text/plain, inline)]

The second is a bit odder. For some reason, find-lisp-object-file-name
searches for an internal function definition using TYPE instead of
OBJECT. I would have expected it to use OBJECT like the rest of the
tests do. I see no reason for the current behaviour.

Either the documentation should be changed to clearly indicate the
current behaviour, or the function should be changed so that OBJECTs for
which (subrp (symbol-function OBJECT)) returns t should return
'C-source'.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Alex <agrambot <at> gmail.com>, 24697 <at> debbugs.gnu.org
Subject: Re: bug#24697: 25.1; find-lisp-object-file-name may return wrong
 locations
Date: Sun, 18 Jun 2017 22:51:43 +0300

On 6/18/17 1:26 AM, Alex wrote:

> The first and last of these are fixed by the following diff.
> 
> ...
> 
> 
> The second is a bit odder. For some reason, find-lisp-object-file-name
> searches for an internal function definition using TYPE instead of
> OBJECT. I would have expected it to use OBJECT like the rest of the
> tests do. I see no reason for the current behaviour.

Thanks. Do you think you can write test cases for these problems? There 
are some existing ones in test/lisp/help-fns-tests.el.

> Either the documentation should be changed to clearly indicate the
> current behaviour, or the function should be changed so that OBJECTs for
> which (subrp (symbol-function OBJECT)) returns t should return
> 'C-source'.

With a test case, you might also find it easier to make a choice 
regarding this problem.

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

From: Alex <agrambot <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 24697 <at> debbugs.gnu.org
Subject: Re: bug#24697: 25.1;
 find-lisp-object-file-name may return wrong locations
Date: Sun, 18 Jun 2017 20:59:25 -0600

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 6/18/17 1:26 AM, Alex wrote:
>
>> The first and last of these are fixed by the following diff.
>>
>> ...
>>
>>
>> The second is a bit odder. For some reason, find-lisp-object-file-name
>> searches for an internal function definition using TYPE instead of
>> OBJECT. I would have expected it to use OBJECT like the rest of the
>> tests do. I see no reason for the current behaviour.
>
> Thanks. Do you think you can write test cases for these problems? There are some
> existing ones in test/lisp/help-fns-tests.el.

Sure, I've attached a patch below for the simple cases. As I couldn't
find a satisfactory way to make a temporary face, I just made an
uninterned symbol that find-lisp-object-file-name would treat as an
internal variable.

>> Either the documentation should be changed to clearly indicate the
>> current behaviour, or the function should be changed so that OBJECTs for
>> which (subrp (symbol-function OBJECT)) returns t should return
>> 'C-source'.
>
> With a test case, you might also find it easier to make a choice regarding this
> problem.

I'm not sure. I still don't understand why the design decision was made.
I suppose one benefit is that one can search explicitly for internal
functions rather than lisp functions, but the function could have just
accepted 'subr instead of 'defun to do that.

Perhaps the current use of searching with TYPE should be left in for
backwards compatibility (a Github search shows at least 2 instances of
3rd-party code that makes use of that behaviour).

For instance, here's how you find mapatoms' file:

  (find-lisp-object-file-name 'mapatoms (symbol-function 'mapatoms))

You should just be able to do the following:

  (find-lisp-object-file-name 'mapatoms 'defun)

Or without searching for lisp functions named mapatoms first:

  (find-lisp-object-file-name 'mapatoms 'subr)

What do you think?

[0001-Only-search-for-a-variable-when-instructed.patch (text/x-diff, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Alex <agrambot <at> gmail.com>
Cc: 24697 <at> debbugs.gnu.org
Subject: Re: bug#24697: 25.1; find-lisp-object-file-name may return wrong
 locations
Date: Fri, 29 Sep 2017 00:26:22 +0300

On 6/19/17 5:59 AM, Alex wrote:

>> Thanks. Do you think you can write test cases for these problems? There are some
>> existing ones in test/lisp/help-fns-tests.el.
> 
> Sure, I've attached a patch below for the simple cases. As I couldn't
> find a satisfactory way to make a temporary face, I just made an
> uninterned symbol that find-lisp-object-file-name would treat as an
> internal variable.

Thanks.

Now, the patch looks correct to me, but did you encounter a practical 
problem that prompted you to look into this discrepancy? I'd like to 
know what it was.

>> With a test case, you might also find it easier to make a choice regarding this
>> problem.
> 
> I'm not sure. I still don't understand why the design decision was made.
> I suppose one benefit is that one can search explicitly for internal
> functions rather than lisp functions, but the function could have just
> accepted 'subr instead of 'defun to do that.

There is a FIXME comment with the same question there. So you are not 
alone in wondering.

> Perhaps the current use of searching with TYPE should be left in for
> backwards compatibility (a Github search shows at least 2 instances of
> 3rd-party code that makes use of that behaviour).
> 
> For instance, here's how you find mapatoms' file:
> 
>    (find-lisp-object-file-name 'mapatoms (symbol-function 'mapatoms))
> 
> You should just be able to do the following:
> 
>    (find-lisp-object-file-name 'mapatoms 'defun)
> 
> Or without searching for lisp functions named mapatoms first:
> 
>    (find-lisp-object-file-name 'mapatoms 'subr)
> 
> What do you think?

Maybe you're right, but backward compatibility seems important here as 
well. You're talking about a separate change, right? We could consider 
it for Emacs 27.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Alex <agrambot <at> gmail.com>
Cc: 24697 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#24697: 25.1; find-lisp-object-file-name may return wrong
 locations
Date: Tue, 11 Aug 2020 15:26:53 +0200

Alex <agrambot <at> gmail.com> writes:

>> Thanks. Do you think you can write test cases for these problems?
>> There are some
>> existing ones in test/lisp/help-fns-tests.el.
>
> Sure, I've attached a patch below for the simple cases. As I couldn't
> find a satisfactory way to make a temporary face, I just made an
> uninterned symbol that find-lisp-object-file-name would treat as an
> internal variable.

Thanks; I've applied your patch to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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