[RFC] refactoring DEFUN

[RFC] refactoring DEFUN

[Dmitry Antipov]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

Although C preprocessor can't transform the text too much, it's
still possible to use concatenation to avoid silly typing like:

DEFUN ("foo", Ffoo, Sfoo, ...)

and use:

DEFUN ("foo", foo, ...)

instead. The core change is simple, but the obvious rest is ~450K
uncompressed (99.9% was generated by elisp program, BTW).

Dmitry

[-- Attachment #2: refactor_DEFUN_core.patch --]
[-- Type: text/plain, Size: 3840 bytes --]

=== modified file 'lib-src/make-docfile.c'
--- lib-src/make-docfile.c	2013-01-15 21:26:01 +0000
+++ lib-src/make-docfile.c	2013-03-25 11:26:49 +0000
@@ -842,8 +842,18 @@
 		    || c == '\n' || c == '\r'));
 	  input_buffer[i] = '\0';
 
-	  name = xmalloc (i + 1);
-	  memcpy (name, input_buffer, i + 1);
+	  if (defunflag)
+	    {
+	      /* Prepend 'F' to Lisp function name.  */
+	      name = xmalloc (i + 2);
+	      name[0] = 'F';
+	      memcpy (name + 1, input_buffer, i + 1);
+	    }
+	  else
+	    {
+	      name = xmalloc (i + 1);
+	      memcpy (name, input_buffer, i + 1);
+	    }
 
 	  if (!defunflag)
 	    {
@@ -857,7 +867,7 @@
 	 DEFVAR_LISP ("name", addr, doc: /\* doc *\/)  */
 
       if (defunflag)
-	commas = generate_globals ? 4 : 5;
+	commas = generate_globals ? 3 : 4;
       else if (defvarperbufferflag)
 	commas = 3;
       else if (defvarflag)

=== modified file 'src/lisp.h'
--- src/lisp.h	2013-03-24 12:59:45 +0000
+++ src/lisp.h	2013-03-25 11:26:49 +0000
@@ -2029,12 +2029,10 @@
 /* Define a built-in function for calling from Lisp.
  `lname' should be the name to give the function in Lisp,
     as a null-terminated C string.
- `fnname' should be the name of the function in C.
-    By convention, it starts with F.
- `sname' should be the name for the C constant structure
-    that records information on this function for internal use.
-    By convention, it should be the same as `fnname' but with S instead of F.
-    It's too bad that C macros can't compute this from `fnname'.
+ `pattern' is used to generate the name of the function in C,
+    and the C constant structure that records information on this
+    function for internal use.  By convention, the first one is
+    done by prepending F and the second is done by prepending S.
  `minargs' should be a number, the minimum number of arguments allowed.
  `maxargs' should be a number, the maximum number of arguments allowed,
     or else MANY or UNEVALLED.
@@ -2054,22 +2052,22 @@
 /* This version of DEFUN declares a function prototype with the right
    arguments, so we can catch errors with maxargs at compile-time.  */
 #ifdef _MSC_VER
-#define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
-   Lisp_Object fnname DEFUN_ARGS_ ## maxargs ;				\
-   static struct Lisp_Subr alignas (GCALIGNMENT) sname =		\
-   { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS)				\
-       | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) },		\
-      { (Lisp_Object (__cdecl *)(void))fnname },                        \
-       minargs, maxargs, lname, intspec, 0};				\
-   Lisp_Object fnname
+#define DEFUN(lname, pattern, minargs, maxargs, intspec, doc)	\
+  Lisp_Object F ## pattern DEFUN_ARGS_ ## maxargs ;		\
+  static struct Lisp_Subr alignas (GCALIGNMENT) S ## pattern =	\
+  { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS)			\
+      | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) },	\
+    { (Lisp_Object (__cdecl *)(void)) F ## pattern },		\
+    minargs, maxargs, lname, intspec, 0};			\
+  Lisp_Object F ## pattern
 #else  /* not _MSC_VER */
-#define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
-   Lisp_Object fnname DEFUN_ARGS_ ## maxargs ;				\
-   static struct Lisp_Subr alignas (GCALIGNMENT) sname =		\
-     { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
-       { .a ## maxargs = fnname },					\
-       minargs, maxargs, lname, intspec, 0};				\
-   Lisp_Object fnname
+#define DEFUN(lname, pattern, minargs, maxargs, intspec, doc)	\
+  Lisp_Object F ## pattern DEFUN_ARGS_ ## maxargs ;		\
+  static struct Lisp_Subr alignas (GCALIGNMENT) S ## pattern =	\
+  { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },			\
+    { .a ## maxargs = F ## pattern },				\
+    minargs, maxargs, lname, intspec, 0};			\
+  Lisp_Object F ## pattern
 #endif
 
 /* Note that the weird token-substitution semantics of ANSI C makes


[-- Attachment #3: refactor_DEFUN_rest.patch.xz --]
[-- Type: application/x-xz, Size: 86264 bytes --]

Re: [RFC] refactoring DEFUN

[Eli Zaretskii]

> Date: Mon, 25 Mar 2013 15:56:29 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> 
> Although C preprocessor can't transform the text too much, it's
> still possible to use concatenation to avoid silly typing like:
> 
> DEFUN ("foo", Ffoo, Sfoo, ...)
> 
> and use:
> 
> DEFUN ("foo", foo, ...)
> 
> instead.

You can get "foo" from foo using the # stringizing operator.

Re: [RFC] refactoring DEFUN

[Eli Zaretskii]

> > DEFUN ("foo", Ffoo, Sfoo, ...)
> > 
> > and use:
> > 
> > DEFUN ("foo", foo, ...)
> > 
> > instead.
> 
> You can get "foo" from foo using the # stringizing operator.

... but I guess "foo-bar" vs foo_bar gets in the way.

Re: refactoring DEFUN

[Andy Moreton]

On Mon 25 Mar 2013, Eli Zaretskii wrote:

>> Date: Mon, 25 Mar 2013 15:56:29 +0400
>> From: Dmitry Antipov <dmantipov@yandex.ru>
>> 
>> Although C preprocessor can't transform the text too much, it's
>> still possible to use concatenation to avoid silly typing like:
>> 
>> DEFUN ("foo", Ffoo, Sfoo, ...)
>> 
>> and use:
>> 
>> DEFUN ("foo", foo, ...)
>> 
>> instead.
>
> You can get "foo" from foo using the # stringizing operator.

True, but you cannot get "foo-bar" from foo_bar that way :-)

    AndyM

Re: refactoring DEFUN

[Andy Moreton]

On Mon 25 Mar 2013, Dmitry Antipov wrote:

> Although C preprocessor can't transform the text too much, it's
> still possible to use concatenation to avoid silly typing like:
>
> DEFUN ("foo", Ffoo, Sfoo, ...)
>
> and use:
>
> DEFUN ("foo", foo, ...)
>
> instead. The core change is simple, but the obvious rest is ~450K
> uncompressed (99.9% was generated by elisp program, BTW).

It would help to show the elisp program you used to generate the patch,
as that would make it easier to check that the results of the transform
are correct.

Did you find any cases where the names did not match up ?

    AndyM

Re: refactoring DEFUN

[Dmitry Antipov]

On 03/25/2013 04:55 PM, Andy Moreton wrote:

> It would help to show the elisp program you used to generate the patch,
> as that would make it easier to check that the results of the transform
> are correct.

This is just about mastering the monster regexp:

(defun defun-replace ()
   (set-buffer (find-file-noselect (car (last command-line-args))))
   (while (re-search-forward
"DEFUN \(\\(\".+\"\\),[ \n\t]+F\\(\[A-Za-z0-9_\]+\\),[ \n\t]+S\\(\[A-Za-z0-9_\]+\\),[ \n\t]+\\([^,]+, [^,]+, [^,]+,\\)" nil t)
     (replace-match "DEFUN \(\\1, \\3, \\4"))
   (save-buffer))

And then do it in batch mode:

for f in src/*.[cm]; do emacs -Q -batch -l ../misc/defun-replace.el -f defun-replace $f; done

The regexp above can't handle complex interactive specs like in Frename_buffer.
However defun-replace doesn't change such a functions at all, so the whole
procedure yields to a few compilation errors which may be fixed manually.
Finally, too long lines may be found with something like:

grep -nH DEFUN *.[cm] | awk 'length > 70'

and fixed manually too.

The whole change passes full bootstrap and basic editing tests.

> Did you find any cases where the names did not match up ?

I believe that I've fixed the last mismatch in r112124 :-).

Dmitry

Re: [RFC] refactoring DEFUN

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On 3/25/2013 4:56 AM, Dmitry Antipov wrote:
> Although C preprocessor can't transform the text too much, it's
> still possible to use concatenation to avoid silly typing like:
> 
> DEFUN ("foo", Ffoo, Sfoo, ...)
> 

I really don't mind the way it is now. Adding a subr isn't a particularly common
endeavor.

> and use:
> 
> DEFUN ("foo", foo, ...)
> 
> instead. The core change is simple, but the obvious rest is ~450K
> uncompressed (99.9% was generated by elisp program, BTW).

I'd prefer to keep it the old way --- it preserves greppability. The new way
involves a lot more magic, and I don't think it buys us much maintainability
because changing buildin function names is rare.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [RFC] refactoring DEFUN

[Dmitry Antipov]

On 03/25/2013 05:18 PM, Daniel Colascione wrote:

> I'd prefer to keep it the old way --- it preserves greppability. The new way
> involves a lot more magic, and I don't think it buys us much maintainability
> because changing buildin function names is rare.

I believe that any tool which is worth using should not have issues with this
change. For example, build TAGS with:

find lib lib-src lwlib oldXMenu src -name '*.[chm]' | xargs ctags -e \
     --regex-c='/[ \t]*DEFUN \(".+", ([A-Za-z_]+)/F\1/' \
     --regex-c='/[ \t]DEFVAR_[A-Z]+ \(".+", (V[A-Za-z_]+)/\1/'

and you should be able to do M-x find-tag for both Fcons and cons
(ctags is Exuberant ctags).

Dmitry

Re: [RFC] refactoring DEFUN

[Alan Mackenzie]

Hello Dmitry.

On Mon, Mar 25, 2013 at 03:56:29PM +0400, Dmitry Antipov wrote:
> Although C preprocessor can't transform the text too much, it's
> still possible to use concatenation to avoid silly typing like:

> DEFUN ("foo", Ffoo, Sfoo, ...)

> and use:

> DEFUN ("foo", foo, ...)

> instead. The core change is simple, but the obvious rest is ~450K
> uncompressed (99.9% was generated by elisp program, BTW).

I'm against this change.  We don't seem to need it.  I think it will
cause more work than it will save, given how seldomly we write new
DEFUNs.

I think it is best to keep Ffoo (at least) explicit.  It would be strange
indeed if grep finds uses of a DEFUN, but not the DEFUN itself.  This
would be suboptimal.

This explicit Ffoo is used by (amongst others?) c-defun-name, a component
of `add-change-log-entry-other-window', C-x 4 a, which is used
frequently.

After this change, c-defun-name would need to be significantly
complicated in order to parse both the traditional and new versions of
DEFUN.

I am against this change.

> Dmitry

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [RFC] refactoring DEFUN

[Stefan Monnier]

> DEFUN ("foo", Ffoo, Sfoo, ...)
> and use:
> DEFUN ("foo", foo, ...)

Please don't bother.  It might look a tiny little bit nicer, but the
rest of the code still has to use Ffoo and Sfoo, so it just makes the
code that much more impenetrable, since grepping for Ffoo won't show you
the place where it's defined.

IOW I don't think it's an overall win.


        Stefan