GNU bug report logs - #75318
Lisp_Subr header and layout

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Po Lu <luangruo <at> yahoo.com>, 75318 <at> debbugs.gnu.org
Subject: bug#75318: Lisp_Subr header and layout
Date: Thu, 23 Jan 2025 11:59:04 +0000

"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes:

>> Ideally, we'd turn these three into plain DEFUNs, with docstrings
>> indicating they're internal functions.  I'd also like to turn the
>> underscores into ordinary dashes in the Lisp function names if we
>> do that.
>
> +1
> Does anyone know why they're not plain DEFUNs?

The sfntfont.c isn't safe to call from Lisp (it assumes it's passed two
vectors of the right size).

Po Lu, I assume it was originally a normal DEFUN and then modified for
performance reasons?  Are those reasons significant enough to keep the
code as it is, in your opinion, until a better solution than turning it
into a safe DEFUN is found?

(I must confess I don't understand the code at all: the first results
are the ones with the fewest number of fields set? Is the list reversed
somewhere else? I'm not saying this code needs more comments, of
course.  If others can understand it easily, I may just have a blind
spot here.)

About the alloc.c non-DEFUNs:

No idea, couldn't find anything, it looks weird in C-h v
gc-cons-threshold.

Nothing in the commit (97ffa339b6d67cebcbefbdfaa2880214adab639c) or bug
discussion (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37006).

The comment in lisp.h, above Lisp_Subr says:

/* This structure describes a built-in function.
   It is generated by the DEFUN macro only.
   defsubr makes it into a Lisp object.  */

The least we could do is adjust that by expanding the second line.  (In
the nativecomp case, all three lines need fixing.)

The watcher is exposed to Lisp and can be called directly or removed, so
that's not it.

The underscored name is also printed in C-h v gc-cons-threshold, so
hiding it isn't the reason either.

(My drive-by proposal in bug#75632 is to improve display of variable
watchers, for example by starting with:

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 9324cf85454..f4f595aedbe 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1712,7 +1712,9 @@ help-fns--var-watchpoints
     (when watchpoints
       (princ "  Calls these functions when changed: ")
       ;; FIXME: Turn function names into hyperlinks.
-      (princ watchpoints)
+      (dolist (watchpoint watchpoints)
+        (princ (help-fns-function-name watchpoint))
+        (princ "\n  "))
       (terpri))))

And then fixing help-fns-function-name so we can remove the FIXME.)

No matter whether we change help-fns.el, the name is likely to remain in
the output, so the underscores are confusing; and giving it a proper
symbol seems like a good idea to me, not just because we can then add
the symbol rather than the subr to the watchers list, and that prints
nicely.

If there aren't any good reasons, I'll send a patch turning all three
into DEFUNs and we can discuss that further.  Definitely going to push
this particular change until there's consensus, though.

Pip

Previous Next

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