gengtype plugin improvement last4round -patch 6 [wstate]

Basile Starynkevitch basile@starynkevitch.net
Thu Nov 25 11:19:00 GMT 2010


Hello All,

References:
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01716.html
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01745.html
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg02350.html
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg02321.html
and all the messages referenced by above messages.

Patch for trunk rev 167136 adding the write & reread of state files for
gengtype to be usable by plugin developers.

I am attaching the diff file gengtype-wstate-r167136.diff and the
gcc/ChangeLog entry file gengtype-wstate-r167136.ChangeLog

Some comments about Laurynas remarks in
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01745.html

> 2010/9/21 Basile Starynkevitch <basile@starynkevitch.net>:
> 
> +/* Fatal message while reading state.  */
> +#define fatal_reading_state(Tok,Msg)  do {		\
> ...
> 
> +#define fatal_reading_state_printf(Tok,Fmt,...) do {	\
> 
> Would you mind doing functions instead of function-like macros here?

I did made fatal_reading_state an inline function to please Laurynas 
(but I don't think that brings much).

However I cannot make the variadic fatal_reading_state_printf a
function, because I see no va_arg argument accepting function in
errors.h, so I have to leave fatal_reading_state_printf as a macro. I
am not sure that having  the binary fatal_reading_state a function and
the variary fatal_reading_state_printf staying a macro is prettier, but
I try to do what Laurynas wants.

> +/* Write the gc_used information.  */
> +static void
> +write_state_gc_used (enum gc_used_enum gus)
> Please add default: ggc_unreachable() or something in that spirit.

Done. I corrected various indentations & typos. However, Laurynas asked
> +  /* We write the state here.  It could eventually happen that the
> +     state file is written after some plugin files have been parsed,
> +     perhaps to enlarge the state file for other plugins needs.  But
> +     this is an uncommon scenario.  */
> 
> Probably not the best place to discuss what may or may not happen in
> the future. More like wiki material.

I respectfully disagree with Laurynas comments.  We have to explain why
write_state is called precisely at this place in main (intuitively, Jeremie 
& me placed it elsewhere, and that was wrong). So I shortened the comment, 
but did say something.

The uncommon scenario (this word does not appear any more in the patch) is:
A plugin foo.so is so common and widely used that it adds new GTY-ed stuff 
and redistribute a gtype-foo.state file augmented with the GTY-ed types 
defined by that plugin. Other plugins foobar.so and foodee.so are based 
upon foo.so (that is, they require the foo.so plugin to be loaded before 
them) and use gtype-foo.state as the state file. Laurynas, could you accept 
the small comment that remains?  I strongly think we have to say something!

I am bootstrapping it right now on x86_64-unknown-linux-gnu (i.e. 
Debian/Sid/AMD64) with lto & c+++ & plugins enabled.

Ok for trunk if it bootstraps? With what changes?

Cheers

PS. Before a documentation patch, we still need a minor Makefile.in patch 
to install gengtype appropriately. Alexander Oliva suggested me at the 
GCC summit to install gengtype, which becomes a user visible utility, 
in $(DESTDIR)$(libexecsubdir)/ where cc1 is already sitting.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gengtype-wstate-r167136.diff
Type: text/x-diff
Size: 77931 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20101125/d706be8d/attachment.bin>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: gengtype-wstate-r167136.ChangeLog
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20101125/d706be8d/attachment.ksh>


More information about the Gcc-patches mailing list