Dan Nicolaescu | 27 Jun 2012 19:12
Picon

coccinelle patch suggestion


Many lisp functions are declared in header files using the EXFUN macro,
like this:
EXFUN (Finteractive_form, 1);

this might have been good when wanting to support both pre-standard C
and standard  C, but we don't support the former anymore.

And EXFUN just adds an unneeded level of obfuscation that doesn't help
code readability.

It seems that with coccinelle it should be easy to create a patch that
replaces all EXFUN usages with the correct declaration, and then EXFUN
can be removed.

Andreas Schwab | 27 Jun 2012 23:03

Re: coccinelle patch suggestion

Dan Nicolaescu <dann <at> gnu.org> writes:

> Many lisp functions are declared in header files using the EXFUN macro,
> like this:
> EXFUN (Finteractive_form, 1);
>
> this might have been good when wanting to support both pre-standard C
> and standard  C, but we don't support the former anymore.

It generates a prototype, which is useful in standard C.

> And EXFUN just adds an unneeded level of obfuscation that doesn't help
> code readability.

It tells you that it's a Lisp function with N arguments.  That's easier
to read than when you need to count them.

Andreas.

--

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

John Wiegley | 28 Jun 2012 02:06
Gravatar

Re: coccinelle patch suggestion

>>>>> Andreas Schwab <schwab <at> linux-m68k.org> writes:

>> And EXFUN just adds an unneeded level of obfuscation that doesn't help code
>> readability.

> It tells you that it's a Lisp function with N arguments.  That's easier to
> read than when you need to count them.

Also, it forms a meaningful parallel with DEFUN.  When I read an EXFUN
declaration, I know that there's a matching DEFUN.  This is not true of
everything other function that has a prototype.

John

Dmitry Antipov | 28 Jun 2012 14:39
Picon
Favicon

Re: coccinelle patch suggestion

On 06/28/2012 04:06 AM, John Wiegley wrote:

>>>>>> Andreas Schwab <schwab <at> linux-m68k.org> writes:
>
>>> And EXFUN just adds an unneeded level of obfuscation that doesn't help code
>>> readability.
>
>> It tells you that it's a Lisp function with N arguments.  That's easier to
>> read than when you need to count them.
>
> Also, it forms a meaningful parallel with DEFUN.  When I read an EXFUN
> declaration, I know that there's a matching DEFUN.  This is not true of
> everything other function that has a prototype.

+10. Of course, it would be a toy task for coccinelle. But, IMHO,
EXFUN makes the things clearer rather than obfuscating them.

Dmitry

Stefan Monnier | 28 Jun 2012 20:05
Picon

Re: coccinelle patch suggestion

> +10. Of course, it would be a toy task for coccinelle.  But, IMHO,
> EXFUN makes the things clearer rather than obfuscating them.

Agreed.  An alternative is to autogenerate all the EXFUNs for all F<foo>
functions into src/global.h, to make it more Lispy.

        Stefan

Tom Tromey | 29 Jun 2012 18:36
Picon
Favicon

Re: coccinelle patch suggestion

>>>>> "Stefan" == Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

Stefan> Agreed.  An alternative is to autogenerate all the EXFUNs for
Stefan> all F<foo> functions into src/global.h, to make it more Lispy.

What do you think of the appended?

It builds for me on x86-64 Fedora 16.
I didn't try any other platforms; there could be hiccups I suppose.

The one wrinkle is that this makes some functions non-static.

Tom

b/lib-src/ChangeLog:
2012-06-29  Tom Tromey  <tromey <at> redhat.com>

	* make-docfile.c (enum global_type) <FUNCTION>: New constant.
	(struct global) <value>: New field.
	(add_global): Add 'value' argument.
	(compare_globals): Sort functions at the end.
	(close_emacs_globals): New function.
	(write_globals): Handle functions.
	(scan_c_file): Call add_global for DEFUN.

b/src/ChangeLog:
2012-06-29  Tom Tromey  <tromey <at> redhat.com>

	* window.c (Fset_window_margins, Fset_window_fringes)
	(Fset_window_scroll_bars, Fset_window_vscroll): No longer static.
(Continue reading)

Stefan Monnier | 2 Jul 2012 20:12
Picon

Re: coccinelle patch suggestion

> What do you think of the appended?

I think it's going in the right direction.  I do wonder/worry about
handling dependencies: for the current globals.h, there's no real
problem since if the vars in it can be pretty much only missing or
extraneous but can't be incorrect (they *very* rarely change type), but
it's slightly less rare to change the type of a DEFUN'd function
(i.e. adding/removing arguments).

        Stefan

> It builds for me on x86-64 Fedora 16.
> I didn't try any other platforms; there could be hiccups I suppose.

> The one wrinkle is that this makes some functions non-static.

> Tom

> b/lib-src/ChangeLog:
> 2012-06-29  Tom Tromey  <tromey <at> redhat.com>

> 	* make-docfile.c (enum global_type) <FUNCTION>: New constant.
> 	(struct global) <value>: New field.
> 	(add_global): Add 'value' argument.
> 	(compare_globals): Sort functions at the end.
> 	(close_emacs_globals): New function.
> 	(write_globals): Handle functions.
> 	(scan_c_file): Call add_global for DEFUN.

> b/src/ChangeLog:
(Continue reading)

Tom Tromey | 2 Jul 2012 21:05
Picon
Favicon

Re: coccinelle patch suggestion

Tom> What do you think of the appended?

Stefan> I think it's going in the right direction.  I do wonder/worry about
Stefan> handling dependencies: for the current globals.h, there's no real
Stefan> problem since if the vars in it can be pretty much only missing or
Stefan> extraneous but can't be incorrect (they *very* rarely change type), but
Stefan> it's slightly less rare to change the type of a DEFUN'd function
Stefan> (i.e. adding/removing arguments).

I think dependencies should already work fine.

gl-stamp is rebuilt when a source file changes:

    gl-stamp: $(libsrc)/make-docfile$(EXEEXT) $(GLOBAL_SOURCES)

Then this is used to rebuild global.h if the contents change:

    globals.h: gl-stamp;  <at> true

Is there a particular scenario you are concerned with?
I could give it a try.

Tom

Stefan Monnier | 3 Jul 2012 15:24
Picon

Re: coccinelle patch suggestion

> Is there a particular scenario you are concerned with?

I guess it's OK then: all files are recompiled whenever globals.h
changes, which is a bit wasteful, but is not the end of the world and
is correct.

I guess you can install that patch, then,

        Stefan

Samuel Bronson | 2 Jul 2012 17:40
Picon
Gravatar

Re: coccinelle patch suggestion


On Jun 28, 2012, at 2:05 PM, Stefan Monnier wrote:

>> +10. Of course, it would be a toy task for coccinelle.  But, IMHO,
>> EXFUN makes the things clearer rather than obfuscating them.
>
> Agreed.  An alternative is to autogenerate all the EXFUNs for all  
> F<foo>
> functions into src/global.h, to make it more Lispy.

But wouldn't that trigger full rebuilds?  I think it would probably be  
better to put them in headers which are associated with, say, the  
relevant subsystem...

Tom Tromey | 2 Jul 2012 19:11
Picon
Favicon

Re: coccinelle patch suggestion

>>>>> "Samuel" == Samuel Bronson <naesten <at> gmail.com> writes:

Stefan> Agreed.  An alternative is to autogenerate all the EXFUNs for all
Stefan> F<foo>
Stefan> functions into src/global.h, to make it more Lispy.

Samuel> But wouldn't that trigger full rebuilds?  I think it would probably be
Samuel> better to put them in headers which are associated with, say, the
Samuel> relevant subsystem...

You already get full rebuilds if you change one, since most are declared
in lisp.h; but changing one is rare anyway.

Tom

Stefan Monnier | 27 Jun 2012 23:24
Picon

Re: coccinelle patch suggestion

> And EXFUN just adds an unneeded level of obfuscation that doesn't help
> code readability.

I'm not sure the "plain" form is better, because of its length
(especially if there are 3 args or more).

        Stefan

Dan Nicolaescu | 27 Jun 2012 23:45
Picon

Re: coccinelle patch suggestion

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>> And EXFUN just adds an unneeded level of obfuscation that doesn't help
>> code readability.
>
> I'm not sure the "plain" form is better, because of its length
> (especially if there are 3 args or more).

The only thing that matter is that the number of arguments matters
between the declaration an the function.  
Why would anybody count the number of arguments?
There's no way to make mistakes in calling with the wrong number of
arguments, the compiler won't let you....

Adding strange constructs like EXFUN has not value...


Gmane