diff mbox

gengtype improvements for plugins, thirdround! patch 5/7 [typedopt]

Message ID 20100921212431.f07dad12.basile@starynkevitch.net
State New
Headers show

Commit Message

Basile Starynkevitch Sept. 21, 2010, 7:24 p.m. UTC 
Hello All,

[join work by Basile Starynkevitch & Jeremie Salvucci]

References: 
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02060.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00616.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00663.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02063.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02065.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00983.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01079.html
This fifth patch chunk is an improved version of
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html taking into
account Laurynas comments in
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00420.html 
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01079.html

I hope that adding comments don't decrease the probability of
having this work accepted into GCC 4.6 end of stage 1.

In addition to the improved comments, I made some minor cleanups
w.r.t. http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html (such
as using OPTION_STRING instead of INFO_STRING for the kind of a string
option, etc.).

I also changed the indentation: old gengtype.c had severals 
  struct FOOBAR
  {
while it seems that the preferred indentation style (at least when
looking into gimple.h or tree.h) is instead
  struct FOOBAR {
which I suppose is the politically correct way of GCC coding.

In addition of moving lots of typing definitions from gengtype.c to
gengtype.h I also (and this is already the case in the previous
version http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html of
this patch), and obviously, made the 'struct options' a typed &
discriminated data, because to persist the gengtype state every union
should be a sum type discriminated conveniently.

The attached patch relative to the "thirdround" patch 4 [filesrules]
obtained with

diff -u -p  $(svn stat . |awk '/[AM]/{print $2}') \
     --from-file ../thirdround_04_filesrules/ > \
     $HOME/Gengtype_Thirdround/patch5_typedopt-relto04.diff


In am also attaching for convenience the cumulated patches against
trunk 164437 as a gzip file

######## gcc/ChangeLog entry relative to patch 5/N thirdround! [filesrules]
2010-09-21  Jeremie Salvucci  <jeremie.salvucci@free.fr>
	    Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c
	(enum typekind, struct options, struct nested_ptr_data)
	(struct pair, NUM_PARAM, enum gc_used_enum, struct type, UNION_P)
	(UNION_OR_STRUCT_P): Moved into gengtype.h

	(string_type, scalar_nonchar, scalar_char): Removed static and
	added state_number fake value.
	(typedefs, structures, param_structs, variables): Removed static.
	(create_option): Removed function.
	(create_string_option, create_type_option, created_nested_option):
	New.
	(created_nested_ptr_option, create_optional_field_)
	(adjust_field_rtx_def, adjust_field_tree_exp): Updated accordingly.
	(adjust_field_type, process_gc_options, output_mangled_typename)
	(walk_type, write_func_for_structure, write_types, write_roots)
	(note_def_vec, dump_options): Ditto.  Access to options is
	protected by tests on their kind.

	* gengtype.h: Added a lot of comments.  Moved many definitions
	from gengtype.h and updated them.
	(typedef, structures, param_structs, variables): Declared extern.
	(enum typekind): Moved from gengtype.c and added TYPE_NONE.
	(enum option_kind): New.
	(struct options): Added discriminating kind, and info union.
	(struct nested_ptr_data): Moved from gengtype.c.
	(create_string_option, create_type_option, create_nested_option):
	New.
	(struct pair, enum_gc_used_enum, NUM_PARAM): Moved
	from gengtype.c and commented more.
	(struct_type): Ditto, added state_number.
	(string_type, scalar_nonchar, scalar_char): Moved from gengtype.c.
	(type_lineloc): New function.
	(UNION_P, UNION_OR_STRUCT_P): Moved from gengtype.c and commented
	more.
	(struct outf): Indented better.

	* gengtype-parse.c (str_optvalue_opt, type_optvalue, option):
	Updated for our discriminated option.

################################################################

Comments and/or Ok for trunk are welcome!

Cheers.

Comments

Basile Starynkevitch Sept. 21, 2010, 7:29 p.m. UTC | #1 
Hello All,

[join work by Basile Starynkevitch & Jeremie Salvucci]

References: 
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02060.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00616.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00663.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02063.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02065.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00983.html
  http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02069.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01150.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01153.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01713.html

The 6th part [wstate] of our patch series (thirdround) is a slightly
improved version of
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01150.html by adding
comments notably taking into account Laurynas remarks in
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01151.html &
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01153.html

diff -u -p -N $(svn stat . |awk '/[AM]/{print $2}') \
     --from-file ../thirdround_05_typedopt/ > \
     $HOME/Gengtype_Thirdround/patch6_wstate-relto05.diff

#################### gcc/ChangeLog entry relative to patch piece 5 [typedopt]
2010-09-21  Jeremie Salvucci  <jeremie.salvucci@free.fr>
	    Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype-state.c: Added new file.

	* gengtype.c:
	(type_count): New static variable.
	(new_structure, find_structure, find_param_structure)
	(create_pointer, create_array): Use type_count for initializing
	state_number field of types.
	(main): Initialize state_number in predefined types.  Call
	read_state and write_state appropriately.  Show the
	type_count when verbose.

	* gengtype.h: Updated comment about per-language directories.
	(read_state, write_state): New declarations.

	* Makefile.in (MOSTLYCLEANFILES): Added gtype.state.
	(GENGTYPE_FLAGS): New variable.
	(s-gtype): Runs gengtype twice, once to write the gtype.state,
	once to read it.
	(build/gengtype-state.o): New target.
	(build/gengtype): Use it.
	(mostlyclean): Remove gtype.state
################################################################



Also, how should the gengtype program be installed? Perhaps it should
be named gcc-gengtype?  I still need help on these minor issues, in
particular as ways to patch gcc/Makefile.in.... There has been some
discussions & suggestions, but I was not able to imagine a
gcc/Makefile.in patch from them.  I confess that I don't understand all
of gcc/Makefile.in in particular the installations tricks. 

I am as usual attaching the cumulated patch w.r.t. trunk 164437.

Ok for trunk?

Cheers.
Laurynas Biveinis Sept. 22, 2010, 2:46 a.m. UTC | #2 
2010/9/21 Basile Starynkevitch <basile@starynkevitch.net>:

@@ -1082,14 +1018,11 @@ adjust_field_rtx_def (type_p t, options_
   /* Create a type to represent the various forms of SYMBOL_REF_DATA.  */
   {
     pair_p sym_flds;
-
-    sym_flds = create_field (NULL, tree_tp, "rt_tree");
-    sym_flds->opt = create_option (nodot, "default", "");
-
-    sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
-    sym_flds->opt = create_option (nodot, "tag", "1");
-
-    symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
+     sym_flds = create_field (NULL, tree_tp, "rt_tree");
+    sym_flds->opt = create_string_option (nodot, "default", "");
+     sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
+    sym_flds->opt = create_string_option (nodot, "tag", "1");
+     symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
 				     &lexer_line, sym_flds, NULL);
   }
   for (i = 0; i < NUM_RTX_CODE; i++)

Indentation is wrong.

@@ -1228,12 +1161,10 @@ adjust_field_rtx_def (type_p t, options_
       ftag = xstrdup (rtx_name[i]);
       for (nmindex = 0; nmindex < strlen (ftag); nmindex++)
 	ftag[nmindex] = TOUPPER (ftag[nmindex]);
-
-      flds = create_field (flds, substruct, "");
-      flds->opt = create_option (nodot, "tag", ftag);
+       flds = create_field (flds, substruct, "");
+      flds->opt = create_string_option (nodot, "tag", ftag);
     }
-
-  return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
+   return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
 }

 /* Handle `special("tree_exp")'.  This is a special case for

Indentation is wrong.

 }
-
-/* Set the gc_used field of T to LEVEL, and handle the types it references.  */
-
+ /* Set the gc_used field of T to LEVEL, and handle the types it
references.  */
 static void
 set_gc_used_type (type_p t, enum gc_used_enum level, type_p param[NUM_PARAM])
 {

Comment should start on the first column.

-  d->needs_cast_p = false;
+   d->needs_cast_p = false;

Please do not break indentation in the code you don't touch :)

-	/* Some things may also be defined in the structure's options.  */
+ 	/* Some things may also be defined in the structure's options.  */

Please drop.

-	    d->reorder_fn = NULL;
+ 	    d->reorder_fn = NULL;

Likewise.

 		use_param_p = 1;
-
 	    if (skip_p)

Likewise.

@@ -2956,18 +2893,20 @@ write_func_for_structure (type_p orig_s,

   memset (&d, 0, sizeof (d));
   d.of = get_output_file_for_structure (s, param);
-
-  for (opt = s->u.s.opt; opt; opt = opt->next)
-    if (strcmp (opt->name, "chain_next") == 0)
-      chain_next = opt->info;
-    else if (strcmp (opt->name, "chain_prev") == 0)
-      chain_prev = opt->info;
-    else if (strcmp (opt->name, "chain_circular") == 0)
-      chain_circular = opt->info;
-    else if (strcmp (opt->name, "mark_hook") == 0)
-      mark_hook_name = opt->info;
-
-  if (chain_prev != NULL && chain_next == NULL)
+   for (opt = s->u.s.opt; opt; opt = opt->next)
[...continues...]

Indentation is wrong.

+       for (o = v->opt; o; o = o->next)
+	if (strcmp (o->name, "length") == 0
+		       && o->kind == OPTION_STRING)
+	  length = o->info.string;

Indentation is wrong.

+	else if (strcmp (o->name, "if_marked") == 0
+		       && o->kind == OPTION_STRING)
+	  if_marked = o->info.string;
+       if (if_marked == NULL)

And wrong.

-
-  /* We assemble the field list in reverse order.  */
+   /* We assemble the field list in reverse order.  */

Again, you are not changing anything here except that you break indentation.

+	case OPTION_NONE:
+	  fatal ("corrupted option %p: %s", (void*) o, o->name);

Insert default label too between these two lines.

+  TYPE_UNION,		/* Type for GTY-ed discriminated
+			   union-s.  */
+  TYPE_POINTER,		/* Pointer type to GTY-ed type.  */
+  TYPE_ARRAY,		/* Array of GTY-ed types.  */

Align TYPE_POINTER comment.

+  OPTION_TYPE,			/* A type-valued option.  */
+  OPTION_NESTED			/* Option data for 'nested_ptr'.  */

Align OPTION_NESTED comment.

+/* A way to pass data through to the output end.  */
+struct options {
+  struct options *next;		/* next option of the same pair.  */
+  const char *name;		/* GTY option name.  */
+  enum option_kind kind;		/* disciminating option kind.  */
+  /* the union below is discriminated by the 'kind' field above.  */
+  union {
+    const char* string;		    /* When OPTION_STRING.  */
+    type_p type;		    /* When OPTION_TYPE.  */
+    struct nested_ptr_data* nested; /* when OPTION_NESTED.  */
+  } info;
+};

Align all the comments.

+  /* State number used when writing & reading the persistent state.  A
+     type with a positive number has already been written.  For ease
+     of debugging, newly allocated types have a unique negative
+     number.  */
+  int state_number;

Should be a part of patch 7 in the series but I'm inclined to let this one slip.

+    /* TYPE__NONE is impossible.  */

TYPE_NONE

+    case TYPE_NONE:
+      fatal ("type_lineloc on bad zeroed type %p", (const void*)ty);

Move to the end of switch, add default label too.

-/* absdecl: type '*'*
+ /* absdecl: type '*'*

Column 1.

-/* Nested pointer data: '(' type '*'* ',' string_seq ',' string_seq ')' */
+ /* Nested pointer data: '(' type '*'* ',' string_seq ',' string_seq ')' */

Likewise.


> 2010-09-21  Jeremie Salvucci  <jeremie.salvucci@free.fr>
>            Basile Starynkevitch  <basile@starynkevitch.net>
>
>        * gengtype.c
>        (enum typekind, struct options, struct nested_ptr_data)
>        (struct pair, NUM_PARAM, enum gc_used_enum, struct type, UNION_P)
>        (UNION_OR_STRUCT_P): Moved into gengtype.h
>
>        (string_type, scalar_nonchar, scalar_char): Removed static and
>        added state_number fake value.
>        (typedefs, structures, param_structs, variables): Removed static.
>        (create_option): Removed function.
>        (create_string_option, create_type_option, created_nested_option):
>        New.
>        (created_nested_ptr_option, create_optional_field_)
>        (adjust_field_rtx_def, adjust_field_tree_exp): Updated accordingly.
>        (adjust_field_type, process_gc_options, output_mangled_typename)
>        (walk_type, write_func_for_structure, write_types, write_roots)
>        (note_def_vec, dump_options): Ditto.  Access to options is
>        protected by tests on their kind.
>
>        * gengtype.h: Added a lot of comments.  Moved many definitions
>        from gengtype.h and updated them.
>        (typedef, structures, param_structs, variables): Declared extern.
>        (enum typekind): Moved from gengtype.c and added TYPE_NONE.
>        (enum option_kind): New.
>        (struct options): Added discriminating kind, and info union.
>        (struct nested_ptr_data): Moved from gengtype.c.
>        (create_string_option, create_type_option, create_nested_option):
>        New.
>        (struct pair, enum_gc_used_enum, NUM_PARAM): Moved
>        from gengtype.c and commented more.
>        (struct_type): Ditto, added state_number.
>        (string_type, scalar_nonchar, scalar_char): Moved from gengtype.c.
>        (type_lineloc): New function.
>        (UNION_P, UNION_OR_STRUCT_P): Moved from gengtype.c and commented
>        more.
>        (struct outf): Indented better.
>
>        * gengtype-parse.c (str_optvalue_opt, type_optvalue, option):
>        Updated for our discriminated option.
>
> ################################################################
>
> Comments and/or Ok for trunk are welcome!
>
> Cheers.
>
> --
> 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} ***
>
Laurynas Biveinis Sept. 22, 2010, 3:02 a.m. UTC | #3 
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?

+/* Write the gc_used information.  */
+static void
+write_state_gc_used (enum gc_used_enum gus)
+{
+  switch (gus)
+    {
+    case GC_UNUSED:
+      fprintf (state_file, " gc_unused");
+      break;
+    case GC_USED:
+      fprintf (state_file, " gc_used");
+      break;
+    case GC_MAYBE_POINTED_TO:
+      fprintf (state_file, " gc_maybe_pointed_to");
+      break;
+    case GC_POINTED_TO:
+      fprintf (state_file, " gc_pointed_to");
+      break;
+    }
+}

Please add default: ggc_unreachable() or something in that spirit.

+void
+write_state (const char *state_path)
+{
+  long statelen = 0;
+  time_t now = 0;
+  char *temp_state_path = NULL;
+  char tempsuffix[40];
+  time (&now);
+
+  /* We write a unique temporary file which is renamed when complete
+   * only.  So even if gengtype is interrupted, the written state file
+   * won't be partially written, since the temporary file is not yet
+   * renamed in that case....  */

case.  */

+  /* The writing of structures probably wrote a lot, so we flush to
+     disk and check for errors, e.g. to catch disk full situations a
+     bit earlier.  */
+  if (fflush (state_file) || ferror (state_file))
+    fatal ("output error when writing state file %s [%s]",
+	   temp_state_path, xstrerror (errno));

IMHO just drop this. IMHO the benefit of early abort is too
insignificant and the less code the better.

+/*****************************************************************
+ * End of writing routines!  The corresponding reading routines follow.
+ *****************************************************************/

I'm against ASCII art of stars in general in comments, and this one
has too few stars to match the text.

@@ -634,11 +644,12 @@ find_structure (const char *name, int is
     if (strcmp (name, s->u.s.tag) == 0
 	&& UNION_P (s) == isunion)
       return s;
-
-  s = XCNEW (struct type);
+   s = XCNEW (struct type);

Broke indentation.

@@ -703,8 +718,10 @@ create_array (type_p t, const char *len)
 {
   type_p v;

-  v = XCNEW (struct type);
+   v = XCNEW (struct type);

Likewise.

@@ -4787,8 +4806,12 @@ main (int argc, char **argv)
   static struct fileloc pos = { NULL, 0 };
   outf_p output_header;

-  /* Mandatory common initializations.  */
+   /* Mandatory common initializations.  */

Likewise.

+  /* 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.

+      /* We definitely don't want to write a state file if some error
+	 occurred while reading input files processed by gengtype,
+	 because we only want to write sane state files!  */

Redundant.

> 2010-09-21  Jeremie Salvucci  <jeremie.salvucci@free.fr>
>            Basile Starynkevitch  <basile@starynkevitch.net>
>
>        * gengtype-state.c: Added new file.
>
>        * gengtype.c:
>        (type_count): New static variable.
>        (new_structure, find_structure, find_param_structure)
>        (create_pointer, create_array): Use type_count for initializing
>        state_number field of types.
>        (main): Initialize state_number in predefined types.  Call
>        read_state and write_state appropriately.  Show the
>        type_count when verbose.
>
>        * gengtype.h: Updated comment about per-language directories.
>        (read_state, write_state): New declarations.
>
>        * Makefile.in (MOSTLYCLEANFILES): Added gtype.state.
>        (GENGTYPE_FLAGS): New variable.
>        (s-gtype): Runs gengtype twice, once to write the gtype.state,
>        once to read it.
>        (build/gengtype-state.o): New target.
>        (build/gengtype): Use it.
>        (mostlyclean): Remove gtype.state
Basile Starynkevitch Sept. 23, 2010, 2:07 p.m. UTC | #4 
On Wed, Sep 22, 2010 at 05:46:29AM +0300, Laurynas Biveinis wrote:


A big and warm thanks to Laurynas for your review and for taking time
to read our patch. 

However, I believe that gengtype is misindented from the start, and
this explains most of my indentation issues. Please read
http://gcc.gnu.org/ml/gcc/2010-09/msg00448.html for details.


> 2010/9/21 Basile Starynkevitch <basile@starynkevitch.net>:
> 
> @@ -1082,14 +1018,11 @@ adjust_field_rtx_def (type_p t, options_
>    /* Create a type to represent the various forms of SYMBOL_REF_DATA.  */
>    {
>      pair_p sym_flds;
> -
> -    sym_flds = create_field (NULL, tree_tp, "rt_tree");
> -    sym_flds->opt = create_option (nodot, "default", "");
> -
> -    sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
> -    sym_flds->opt = create_option (nodot, "tag", "1");
> -
> -    symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
> +     sym_flds = create_field (NULL, tree_tp, "rt_tree");
> +    sym_flds->opt = create_string_option (nodot, "default", "");
> +     sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
> +    sym_flds->opt = create_string_option (nodot, "tag", "1");
> +     symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
>  				     &lexer_line, sym_flds, NULL);
>    }
>    for (i = 0; i < NUM_RTX_CODE; i++)
> 
> Indentation is wrong.
> 
> @@ -1228,12 +1161,10 @@ adjust_field_rtx_def (type_p t, options_
>        ftag = xstrdup (rtx_name[i]);
>        for (nmindex = 0; nmindex < strlen (ftag); nmindex++)
>  	ftag[nmindex] = TOUPPER (ftag[nmindex]);
> -
> -      flds = create_field (flds, substruct, "");
> -      flds->opt = create_option (nodot, "tag", ftag);
> +       flds = create_field (flds, substruct, "");
> +      flds->opt = create_string_option (nodot, "tag", ftag);
>      }
> -
> -  return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
> +   return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
>  }
> 
>  /* Handle `special("tree_exp")'.  This is a special case for
> 
> Indentation is wrong.
> 
>  }
> -
> -/* Set the gc_used field of T to LEVEL, and handle the types it references.  */
> -
> + /* Set the gc_used field of T to LEVEL, and handle the types it
> references.  */
>  static void
>  set_gc_used_type (type_p t, enum gc_used_enum level, type_p param[NUM_PARAM])
>  {
> 
> Comment should start on the first column.
> 
> -  d->needs_cast_p = false;
> +   d->needs_cast_p = false;
> 
> Please do not break indentation in the code you don't touch :)
> 
>    memset (&d, 0, sizeof (d));
>    d.of = get_output_file_for_structure (s, param);
> -
> -  for (opt = s->u.s.opt; opt; opt = opt->next)
> -    if (strcmp (opt->name, "chain_next") == 0)
> -      chain_next = opt->info;
> -    else if (strcmp (opt->name, "chain_prev") == 0)
> -      chain_prev = opt->info;
> -    else if (strcmp (opt->name, "chain_circular") == 0)
> -      chain_circular = opt->info;
> -    else if (strcmp (opt->name, "mark_hook") == 0)
> -      mark_hook_name = opt->info;
> -
> -  if (chain_prev != NULL && chain_next == NULL)
> +   for (opt = s->u.s.opt; opt; opt = opt->next)
> [...continues...]
> 
> Indentation is wrong.
> 
> +       for (o = v->opt; o; o = o->next)
> +	if (strcmp (o->name, "length") == 0
> +		       && o->kind == OPTION_STRING)
> +	  length = o->info.string;
> 
> Indentation is wrong.
> 
> +	else if (strcmp (o->name, "if_marked") == 0
> +		       && o->kind == OPTION_STRING)
> +	  if_marked = o->info.string;
> +       if (if_marked == NULL)
> 
> And wrong.
> 
> -
> -  /* We assemble the field list in reverse order.  */
> +   /* We assemble the field list in reverse order.  */
> 
> Again, you are not changing anything here except that you break indentation.
> 
> +	case OPTION_NONE:
> +	  fatal ("corrupted option %p: %s", (void*) o, o->name);
> 
> Insert default label too between these two lines.
> 
> +  TYPE_UNION,		/* Type for GTY-ed discriminated
> +			   union-s.  */
> +  TYPE_POINTER,		/* Pointer type to GTY-ed type.  */
> +  TYPE_ARRAY,		/* Array of GTY-ed types.  */
> 
> Align TYPE_POINTER comment.
> 
> +  OPTION_TYPE,			/* A type-valued option.  */
> +  OPTION_NESTED			/* Option data for 'nested_ptr'.  */
> 
> Align OPTION_NESTED comment.
> 
> +/* A way to pass data through to the output end.  */
> +struct options {
> +  struct options *next;		/* next option of the same pair.  */
> +  const char *name;		/* GTY option name.  */
> +  enum option_kind kind;		/* disciminating option kind.  */
> +  /* the union below is discriminated by the 'kind' field above.  */
> +  union {
> +    const char* string;		    /* When OPTION_STRING.  */
> +    type_p type;		    /* When OPTION_TYPE.  */
> +    struct nested_ptr_data* nested; /* when OPTION_NESTED.  */
> +  } info;
> +};
> 
> Align all the comments.


Regarding indentation issues in gengtype, please read
http://gcc.gnu.org/ml/gcc/2010-09/msg00448.html

I thanks warmly Laurynas for reading & commenting my patches, but I
need *specific* and *detailed* help on indentation, as detailed in the
message referenced above.  The best case for me is to be able to
submit a preliminary indentation patch, [option 1 of email
http://gcc.gnu.org/ml/gcc/2010-09/msg00448.html] which would only
change the indentation of gengtype.c & gengtype.h.  But I need
approval on the idea of submitting an indentation-only patch (this is
against GCC habits).

Only once indentation issues are settled would I be able to work on
the bulk of Laurynas comments. I put many hours of indentation efforts
in vain in these patches, and I strongly believe that it is because
the original (ie current trunk) gengtype.[ch] is badly indented, so I
don't know if I should perpetuate the indentation errors in gengtype
or correct them. (I did a mix of both).



Cheers.
Diego Novillo Sept. 23, 2010, 2:14 p.m. UTC | #5 
On Thu, Sep 23, 2010 at 10:07, Basile Starynkevitch
<basile@starynkevitch.net> wrote:

> However, I believe that gengtype is misindented from the start, and
> this explains most of my indentation issues. Please read
> http://gcc.gnu.org/ml/gcc/2010-09/msg00448.html for details.

If gengtype is misindented, please submit a separate patch fixing
indentation issues.  Don't mix the two things in one patch.  Your
changes are sufficiently big already.


Diego.
Richard Biener Sept. 23, 2010, 2:16 p.m. UTC | #6 
On Thu, Sep 23, 2010 at 4:14 PM, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Sep 23, 2010 at 10:07, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
>
>> However, I believe that gengtype is misindented from the start, and
>> this explains most of my indentation issues. Please read
>> http://gcc.gnu.org/ml/gcc/2010-09/msg00448.html for details.
>
> If gengtype is misindented, please submit a separate patch fixing
> indentation issues.  Don't mix the two things in one patch.  Your
> changes are sufficiently big already.

And note that using GNU indent is not going to produce something
that is free of indentation issues.

Usual practice is to fix indentation issues in the code snippets you
are changing anyway.

Richard.
diff mbox

Patch

--- ../thirdround_04_filesrules//gengtype.c	2010-09-21 13:53:03.000000000 +0200
+++ gcc/gengtype.c	2010-09-21 16:16:19.000000000 +0200
@@ -31,98 +31,6 @@ 
 
 /* Data types, macros, etc. used only in this file.  */
 
-/* Kinds of types we can understand.  */
-enum typekind {
-  TYPE_SCALAR,
-  TYPE_STRING,
-  TYPE_STRUCT,
-  TYPE_UNION,
-  TYPE_POINTER,
-  TYPE_ARRAY,
-  TYPE_LANG_STRUCT,
-  TYPE_PARAM_STRUCT
-};
-
-
-/* A way to pass data through to the output end.  */
-struct options
-{
-  struct options *next;
-  const char *name;
-  const char *info;
-};
-
-/* Option data for the 'nested_ptr' option.  */
-struct nested_ptr_data
-{
-  type_p type;
-  const char *convert_to;
-  const char *convert_from;
-};
-
-/* A name and a type.  */
-struct pair
-{
-  pair_p next;
-  const char *name;
-  type_p type;
-  struct fileloc line;
-  options_p opt;
-};
-
-#define NUM_PARAM 10
-
-/* A description of a type.  */
-enum gc_used_enum
-  {
-    GC_UNUSED = 0,
-    GC_USED,
-    /* Used for structures whose definitions we haven't seen so far when
-       we encounter a pointer to it that is annotated with ``maybe_undef''.
-       If after reading in everything we don't have source file
-       information for it, we assume that it never has been defined. */
-    GC_MAYBE_POINTED_TO,
-    GC_POINTED_TO
-  };
-
-struct type
-{
-  enum typekind kind;
-  type_p next;
-  type_p pointer_to;
-  enum gc_used_enum gc_used;
-  union {
-    type_p p;
-    struct {
-      const char *tag;
-      struct fileloc line;
-      pair_p fields;
-      options_p opt;
-      lang_bitmap bitmap;
-      type_p lang_struct;
-    } s;
-    bool scalar_is_char;
-    struct {
-      type_p p;
-      const char *len;
-    } a;
-    struct {
-      type_p stru;
-      type_p param[NUM_PARAM];
-      struct fileloc line;
-    } param_struct;
-  } u;
-};
-
-#define UNION_P(x)					\
- ((x)->kind == TYPE_UNION || 				\
-  ((x)->kind == TYPE_LANG_STRUCT 			\
-   && (x)->u.s.lang_struct->kind == TYPE_UNION))
-#define UNION_OR_STRUCT_P(x)			\
- ((x)->kind == TYPE_UNION 			\
-  || (x)->kind == TYPE_STRUCT 			\
-  || (x)->kind == TYPE_LANG_STRUCT)
-
 
 
 
@@ -547,26 +455,26 @@  read_input_list (const char *listname)
 
 /* The one and only TYPE_STRING.  */
 
-static struct type string_type = {
-  TYPE_STRING, 0, 0, GC_USED, {0}
+struct type string_type = {
+  TYPE_STRING, 0, 0, 0, GC_USED, {0}
 };
 
 /* The two and only TYPE_SCALARs.  Their u.scalar_is_char flags are
    set to appropriate values at the beginning of main.  */
 
-static struct type scalar_nonchar = {
-  TYPE_SCALAR, 0, 0, GC_USED, {0}
+struct type scalar_nonchar = {
+  TYPE_SCALAR, 0, 0, 0, GC_USED, {0}
 };
-static struct type scalar_char = {
-  TYPE_SCALAR, 0, 0, GC_USED, {0}
+struct type scalar_char = {
+  TYPE_SCALAR, 0, 0, 0, GC_USED, {0}
 };
 
 /* Lists of various things.  */
 
-static pair_p typedefs;
-static type_p structures;
-static type_p param_structs;
-static pair_p variables;
+pair_p typedefs;
+type_p structures;
+type_p param_structs;
+pair_p variables;
 
 static type_p find_param_structure
   (type_p t, type_p param[NUM_PARAM]);
@@ -802,16 +710,46 @@  create_array (type_p t, const char *len)
   return v;
 }
 
-/* Return an options structure with name NAME and info INFO.  NEXT is the
-   next option in the chain.  */
+/* Return a string options structure with name NAME and info INFO.
+   NEXT is the next option in the chain.  */
+options_p
+create_string_option (options_p next, const char *name, const char *info)
+{
+  options_p o = XCNEW (struct options);
+  o->kind = OPTION_STRING;
+  o->next = next;
+  o->name = name;
+  o->info.string = (const char*) info;
+  return o;
+}
 
+/* Create a type options structure with name NAME and info INFO.  NEXT
+   is the next option in the chain.  */
 options_p
-create_option (options_p next, const char *name, const void *info)
+create_type_option (options_p next, const char* name, type_p info)
 {
-  options_p o = XNEW (struct options);
+  options_p o;
+  o = XCNEW (struct options);
   o->next = next;
   o->name = name;
-  o->info = (const char*) info;
+  o->kind = OPTION_TYPE;
+  o->info.type = info;
+  return o;
+}
+
+
+/* Create a nested pointer options structure with name NAME and info
+   INFO.  NEXT is the next option in the chain.  */
+options_p
+create_nested_option (options_p next, const char* name,
+		      struct nested_ptr_data* info)
+{
+  options_p o;
+  o = XCNEW (struct options);
+  o->next = next;
+  o->name = name;
+  o->kind = OPTION_NESTED;
+  o->info.nested = info;
   return o;
 }
 
@@ -825,10 +763,9 @@  create_nested_ptr_option (options_p next
   d->type = adjust_field_type (t, 0);
   d->convert_to = to;
   d->convert_from = from;
-  return create_option (next, "nested_ptr", d);
+  return create_nested_option (next, "nested_ptr", d);
 }
-
-/* Add a variable named S of type T with options O defined at POS,
+ /* Add a variable named S of type T with options O defined at POS,
    to `variables'.  */
 
 void
@@ -892,15 +829,14 @@  create_optional_field_ (pair_p next, typ
      The field has a tag of "1".  This allows us to make the presence
      of a field of type TYPE depend on some boolean "desc" being true.  */
   union_fields = create_field (NULL, type, "");
-  union_fields->opt = create_option (union_fields->opt, "dot", "");
-  union_fields->opt = create_option (union_fields->opt, "tag", "1");
+  union_fields->opt = create_string_option (union_fields->opt, "dot", "");
+  union_fields->opt = create_string_option (union_fields->opt, "tag", "1");
   union_type = new_structure (xasprintf ("%s_%d", "fake_union", id++), 1,
 			      &lexer_line, union_fields, NULL);
-
-  /* Create the field and give it the new fake union type.  Add a "desc"
+   /* Create the field and give it the new fake union type.  Add a "desc"
      tag that specifies the condition under which the field is valid.  */
   return create_field_all (next, union_type, name,
-			   create_option (0, "desc", cond),
+			   create_string_option (0, "desc", cond),
 			   this_file, line);
 }
 #define create_optional_field(next,type,name,cond)	\
@@ -1032,7 +968,7 @@  adjust_field_rtx_def (type_p t, options_
       return &string_type;
     }
 
-  nodot = create_option (NULL, "dot", "");
+  nodot = create_string_option (NULL, "dot", "");
 
   rtx_tp = create_pointer (find_structure ("rtx_def", 0));
   rtvec_tp = create_pointer (find_structure ("rtvec_def", 0));
@@ -1072,9 +1008,9 @@  adjust_field_rtx_def (type_p t, options_
 	/* NOTE_INSN_MAX is used as the default field for line
 	   number notes.  */
 	if (c == NOTE_INSN_MAX)
-	  note_flds->opt = create_option (nodot, "default", "");
+	  note_flds->opt = create_string_option (nodot, "default", "");
 	else
-	  note_flds->opt = create_option (nodot, "tag", note_insn_name[c]);
+	  note_flds->opt = create_string_option (nodot, "tag", note_insn_name[c]);
       }
     note_union_tp = new_structure ("rtx_def_note_subunion", 1,
 				   &lexer_line, note_flds, NULL);
@@ -1082,14 +1018,11 @@  adjust_field_rtx_def (type_p t, options_
   /* Create a type to represent the various forms of SYMBOL_REF_DATA.  */
   {
     pair_p sym_flds;
-
-    sym_flds = create_field (NULL, tree_tp, "rt_tree");
-    sym_flds->opt = create_option (nodot, "default", "");
-
-    sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
-    sym_flds->opt = create_option (nodot, "tag", "1");
-
-    symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
+     sym_flds = create_field (NULL, tree_tp, "rt_tree");
+    sym_flds->opt = create_string_option (nodot, "default", "");
+     sym_flds = create_field (sym_flds, constant_tp, "rt_constant");
+    sym_flds->opt = create_string_option (nodot, "tag", "1");
+     symbol_union_tp = new_structure ("rtx_def_symbol_subunion", 1,
 				     &lexer_line, sym_flds, NULL);
   }
   for (i = 0; i < NUM_RTX_CODE; i++)
@@ -1206,11 +1139,11 @@  adjust_field_rtx_def (type_p t, options_
 					       subname));
 	  subfields->opt = nodot;
 	  if (t == note_union_tp)
-	    subfields->opt = create_option (subfields->opt, "desc",
-					    "NOTE_KIND (&%0)");
+	    subfields->opt = create_string_option (subfields->opt, "desc",
+						   "NOTE_KIND (&%0)");
 	  if (t == symbol_union_tp)
-	    subfields->opt = create_option (subfields->opt, "desc",
-					    "CONSTANT_POOL_ADDRESS_P (&%0)");
+	    subfields->opt = create_string_option (subfields->opt, "desc",
+						   "CONSTANT_POOL_ADDRESS_P (&%0)");
 	}
 
       if (i == SYMBOL_REF)
@@ -1228,12 +1161,10 @@  adjust_field_rtx_def (type_p t, options_
       ftag = xstrdup (rtx_name[i]);
       for (nmindex = 0; nmindex < strlen (ftag); nmindex++)
 	ftag[nmindex] = TOUPPER (ftag[nmindex]);
-
-      flds = create_field (flds, substruct, "");
-      flds->opt = create_option (nodot, "tag", ftag);
+       flds = create_field (flds, substruct, "");
+      flds->opt = create_string_option (nodot, "tag", ftag);
     }
-
-  return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
+   return new_structure ("rtx_def_subunion", 1, &lexer_line, flds, nodot);
 }
 
 /* Handle `special("tree_exp")'.  This is a special case for
@@ -1255,12 +1186,12 @@  adjust_field_tree_exp (type_p t, options
       return &string_type;
     }
 
-  nodot = create_option (NULL, "dot", "");
+  nodot = create_string_option (NULL, "dot", "");
 
   flds = create_field (NULL, t, "");
-  flds->opt = create_option (nodot, "length",
-			     "TREE_OPERAND_LENGTH ((tree) &%0)");
-  flds->opt = create_option (flds->opt, "default", "");
+  flds->opt = create_string_option (nodot, "length",
+				    "TREE_OPERAND_LENGTH ((tree) &%0)");
+  flds->opt = create_string_option (flds->opt, "default", "");
 
   return new_structure ("tree_exp_subunion", 1, &lexer_line, flds, nodot);
 }
@@ -1290,10 +1221,11 @@  adjust_field_type (type_p t, options_p o
   for (; opt; opt = opt->next)
     if (strcmp (opt->name, "length") == 0)
       length_p = 1;
-    else if (strcmp (opt->name, "param_is") == 0
-	     || (strncmp (opt->name, "param", 5) == 0
-		 && ISDIGIT (opt->name[5])
-		 && strcmp (opt->name + 6, "_is") == 0))
+    else if ((strcmp (opt->name, "param_is") == 0
+	      || (strncmp (opt->name, "param", 5) == 0
+		  && ISDIGIT (opt->name[5])
+		  && strcmp (opt->name + 6, "_is") == 0))
+	     && opt->kind == OPTION_TYPE)
       {
 	int num = ISDIGIT (opt->name[5]) ? opt->name[5] - '0' : 0;
 
@@ -1310,13 +1242,14 @@  adjust_field_type (type_p t, options_p o
 	if (params[num] != NULL)
 	  error_at_line (&lexer_line, "duplicate `%s' option", opt->name);
 	if (! ISDIGIT (opt->name[5]))
-	  params[num] = create_pointer (CONST_CAST2(type_p, const char *, opt->info));
+	  params[num] = create_pointer (opt->info.type);
 	else
-	  params[num] = CONST_CAST2 (type_p, const char *, opt->info);
+	  params[num] = opt->info.type;
       }
-    else if (strcmp (opt->name, "special") == 0)
+    else if (strcmp (opt->name, "special") == 0
+	     && opt->kind == OPTION_STRING)
       {
-	const char *special_name = opt->info;
+	const char *special_name = opt->info.string;
 	if (strcmp (special_name, "tree_exp") == 0)
 	  t = adjust_field_tree_exp (t, opt);
 	else if (strcmp (special_name, "rtx_def") == 0)
@@ -1360,8 +1293,9 @@  process_gc_options (options_p opt, enum
 {
   options_p o;
   for (o = opt; o; o = o->next)
-    if (strcmp (o->name, "ptr_alias") == 0 && level == GC_POINTED_TO)
-      set_gc_used_type (CONST_CAST2 (type_p, const char *, o->info),
+    if (strcmp (o->name, "ptr_alias") == 0 && level == GC_POINTED_TO
+	&& o->kind == OPTION_TYPE)
+      set_gc_used_type (o->info.type,
 			GC_POINTED_TO, NULL);
     else if (strcmp (o->name, "maybe_undef") == 0)
       *maybe_undef = 1;
@@ -1371,12 +1305,11 @@  process_gc_options (options_p opt, enum
       *length = 1;
     else if (strcmp (o->name, "skip") == 0)
       *skip = 1;
-    else if (strcmp (o->name, "nested_ptr") == 0)
-      *nested_ptr = ((const struct nested_ptr_data *) o->info)->type;
+    else if (strcmp (o->name, "nested_ptr") == 0
+	     && o->kind == OPTION_NESTED)
+      *nested_ptr = ((const struct nested_ptr_data *) o->info.nested)->type;
 }
-
-/* Set the gc_used field of T to LEVEL, and handle the types it references.  */
-
+ /* Set the gc_used field of T to LEVEL, and handle the types it references.  */
 static void
 set_gc_used_type (type_p t, enum gc_used_enum level, type_p param[NUM_PARAM])
 {
@@ -2314,6 +2247,8 @@  output_mangled_typename (outf_p of, cons
     oprintf (of, "Z");
   else switch (t->kind)
     {
+    case TYPE_NONE:
+      fatal ("uninitialized type to mangle");
     case TYPE_POINTER:
       oprintf (of, "P");
       output_mangled_typename (of, t->u.p);
@@ -2405,10 +2340,10 @@  walk_type (type_p t, struct walk_type_da
   options_p oo;
   const struct nested_ptr_data *nested_ptr_d = NULL;
 
-  d->needs_cast_p = false;
+   d->needs_cast_p = false;
   for (oo = d->opt; oo; oo = oo->next)
-    if (strcmp (oo->name, "length") == 0)
-      length = oo->info;
+    if (strcmp (oo->name, "length") == 0 && oo->kind == OPTION_STRING)
+      length = oo->info.string;
     else if (strcmp (oo->name, "maybe_undef") == 0)
       maybe_undef_p = 1;
     else if (strncmp (oo->name, "use_param", 9) == 0
@@ -2416,12 +2351,12 @@  walk_type (type_p t, struct walk_type_da
       use_param_num = oo->name[9] == '\0' ? 0 : oo->name[9] - '0';
     else if (strcmp (oo->name, "use_params") == 0)
       use_params_p = 1;
-    else if (strcmp (oo->name, "desc") == 0)
-      desc = oo->info;
+    else if (strcmp (oo->name, "desc") == 0 && oo->kind == OPTION_STRING)
+      desc = oo->info.string;
     else if (strcmp (oo->name, "mark_hook") == 0)
       ;
-    else if (strcmp (oo->name, "nested_ptr") == 0)
-      nested_ptr_d = (const struct nested_ptr_data *) oo->info;
+    else if (strcmp (oo->name, "nested_ptr") == 0 && oo->kind == OPTION_NESTED)
+      nested_ptr_d = (const struct nested_ptr_data *) oo->info.nested;
     else if (strcmp (oo->name, "dot") == 0)
       ;
     else if (strcmp (oo->name, "tag") == 0)
@@ -2674,12 +2609,12 @@  walk_type (type_p t, struct walk_type_da
 	    error_at_line (&t->u.s.line, "one structure defined here");
 	  }
 
-	/* Some things may also be defined in the structure's options.  */
+ 	/* Some things may also be defined in the structure's options.  */
 	for (o = t->u.s.opt; o; o = o->next)
-	  if (! desc && strcmp (o->name, "desc") == 0)
-	    desc = o->info;
-
-	d->prev_val[2] = oldval;
+	  if (! desc && strcmp (o->name, "desc") == 0
+	      && o->kind == OPTION_STRING)
+	    desc = o->info.string;
+ 	d->prev_val[2] = oldval;
 	d->prev_val[1] = oldprevval2;
 	if (union_p)
 	  {
@@ -2705,22 +2640,24 @@  walk_type (type_p t, struct walk_type_da
 	    int use_param_p = 0;
 	    char *newval;
 
-	    d->reorder_fn = NULL;
+ 	    d->reorder_fn = NULL;
 	    for (oo = f->opt; oo; oo = oo->next)
-	      if (strcmp (oo->name, "dot") == 0)
-		dot = oo->info;
-	      else if (strcmp (oo->name, "tag") == 0)
-		tagid = oo->info;
+	      if (strcmp (oo->name, "dot") == 0
+		  && oo->kind == OPTION_STRING)
+		dot = oo->info.string;
+	      else if (strcmp (oo->name, "tag") == 0
+		       && oo->kind == OPTION_STRING)
+		tagid = oo->info.string;
 	      else if (strcmp (oo->name, "skip") == 0)
 		skip_p = 1;
 	      else if (strcmp (oo->name, "default") == 0)
 		default_p = 1;
-	      else if (strcmp (oo->name, "reorder") == 0)
-		d->reorder_fn = oo->info;
+	      else if (strcmp (oo->name, "reorder") == 0
+		       && oo->kind == OPTION_STRING)
+		d->reorder_fn = oo->info.string;
 	      else if (strncmp (oo->name, "use_param", 9) == 0
 		       && (oo->name[9] == '\0' || ISDIGIT (oo->name[9])))
 		use_param_p = 1;
-
 	    if (skip_p)
 	      continue;
 
@@ -2956,18 +2893,20 @@  write_func_for_structure (type_p orig_s,
 
   memset (&d, 0, sizeof (d));
   d.of = get_output_file_for_structure (s, param);
-
-  for (opt = s->u.s.opt; opt; opt = opt->next)
-    if (strcmp (opt->name, "chain_next") == 0)
-      chain_next = opt->info;
-    else if (strcmp (opt->name, "chain_prev") == 0)
-      chain_prev = opt->info;
-    else if (strcmp (opt->name, "chain_circular") == 0)
-      chain_circular = opt->info;
-    else if (strcmp (opt->name, "mark_hook") == 0)
-      mark_hook_name = opt->info;
-
-  if (chain_prev != NULL && chain_next == NULL)
+   for (opt = s->u.s.opt; opt; opt = opt->next)
+    if (strcmp (opt->name, "chain_next") == 0
+	&& opt->kind == OPTION_STRING)
+      chain_next = opt->info.string;
+    else if (strcmp (opt->name, "chain_prev") == 0
+	&& opt->kind == OPTION_STRING)
+      chain_prev = opt->info.string;
+    else if (strcmp (opt->name, "chain_circular") == 0
+	&& opt->kind == OPTION_STRING)
+      chain_circular = opt->info.string;
+    else if (strcmp (opt->name, "mark_hook") == 0
+	&& opt->kind == OPTION_STRING)
+      mark_hook_name = opt->info.string;
+   if (chain_prev != NULL && chain_next == NULL)
     error_at_line (&s->u.s.line, "chain_prev without chain_next");
   if (chain_circular != NULL && chain_next != NULL)
     error_at_line (&s->u.s.line, "chain_circular with chain_next");
@@ -3135,11 +3074,11 @@  write_types (outf_p output_header, type_
 		 s->u.s.tag);
 	oprintf (output_header,
 		 "  } while (0)\n");
-
-	for (opt = s->u.s.opt; opt; opt = opt->next)
-	  if (strcmp (opt->name, "ptr_alias") == 0)
+ 	for (opt = s->u.s.opt; opt; opt = opt->next)
+	  if (strcmp (opt->name, "ptr_alias") == 0
+	      && opt->kind == OPTION_TYPE)
 	    {
-	      const_type_p const t = (const_type_p) opt->info;
+	      const_type_p const t = (const_type_p) opt->info.type;
 	      if (t->kind == TYPE_STRUCT
 		  || t->kind == TYPE_UNION
 		  || t->kind == TYPE_LANG_STRUCT)
@@ -3362,11 +3301,11 @@  write_local (outf_p output_header, type_
 
 	if (s->u.s.line.file == NULL)
 	  continue;
-
-	for (opt = s->u.s.opt; opt; opt = opt->next)
-	  if (strcmp (opt->name, "ptr_alias") == 0)
+ 	for (opt = s->u.s.opt; opt; opt = opt->next)
+	  if (strcmp (opt->name, "ptr_alias") == 0
+	      && opt->kind == OPTION_TYPE)
 	    {
-	      const_type_p const t = (const_type_p) opt->info;
+	      const_type_p const t = (const_type_p) opt->info.type;
 	      if (t->kind == TYPE_STRUCT
 		  || t->kind == TYPE_UNION
 		  || t->kind == TYPE_LANG_STRUCT)
@@ -3694,8 +3633,9 @@  write_root (outf_p f, pair_p v, type_p t
 	    for (o = fld->opt; o; o = o->next)
 	      if (strcmp (o->name, "skip") == 0)
 		skip_p = 1;
-	      else if (strcmp (o->name, "desc") == 0)
-		desc = o->info;
+	      else if (strcmp (o->name, "desc") == 0
+		       && o->kind == OPTION_STRING)
+		desc = o->info.string;
 	      else if (strcmp (o->name, "param_is") == 0)
 		;
 	      else
@@ -3714,10 +3654,10 @@  write_root (outf_p f, pair_p v, type_p t
 		  {
 		    const char *tag = NULL;
 		    options_p oo;
-
-		    for (oo = ufld->opt; oo; oo = oo->next)
-		      if (strcmp (oo->name, "tag") == 0)
-			tag = oo->info;
+ 		    for (oo = ufld->opt; oo; oo = oo->next)
+		      if (strcmp (oo->name, "tag") == 0
+		       && oo->kind == OPTION_STRING)
+			tag = oo->info.string;
 		    if (tag == NULL || strcmp (tag, desc) != 0)
 		      continue;
 		    if (validf != NULL)
@@ -3891,10 +3831,10 @@  write_roots (pair_p variables, bool emit
       const char *length = NULL;
       int deletable_p = 0;
       options_p o;
-
-      for (o = v->opt; o; o = o->next)
-	if (strcmp (o->name, "length") == 0)
-	  length = o->info;
+       for (o = v->opt; o; o = o->next)
+	if (strcmp (o->name, "length") == 0
+		       && o->kind == OPTION_STRING)
+	  length = o->info.string;
 	else if (strcmp (o->name, "deletable") == 0)
 	  deletable_p = 1;
 	else if (strcmp (o->name, "param_is") == 0)
@@ -4019,12 +3959,11 @@  write_roots (pair_p variables, bool emit
       for (o = v->opt; o; o = o->next)
 	if (strcmp (o->name, "length") == 0)
 	  length_p = 1;
-	else if (strcmp (o->name, "if_marked") == 0)
-	  if_marked = o->info;
-
-      if (if_marked == NULL)
+	else if (strcmp (o->name, "if_marked") == 0
+		       && o->kind == OPTION_STRING)
+	  if_marked = o->info.string;
+       if (if_marked == NULL)
 	continue;
-
       if (v->type->kind != TYPE_POINTER
 	  || v->type->u.p->kind != TYPE_PARAM_STRUCT
 	  || v->type->u.p->u.param_struct.stru != find_structure ("htab", 0))
@@ -4156,10 +4095,9 @@  note_def_vec (const char *type_name, boo
   else
     {
       t = resolve_typedef (type_name, pos);
-      o = create_option (0, "length", "%h.num");
+      o = create_string_option (0, "length", "%h.num");
     }
-
-  /* We assemble the field list in reverse order.  */
+   /* We assemble the field list in reverse order.  */
   fields = create_field_at (0, create_array (t, "1"), "vec", o, pos);
   fields = create_field_at (fields, len_ty, "alloc", 0, pos);
   fields = create_field_at (fields, len_ty, "num", 0, pos);
@@ -4447,8 +4385,22 @@  dump_options (int indent, options_p opt)
   o = opt;
   while (o)
     {
-       printf ("%s:%s ", o->name, o->info);
-       o = o->next;
+      switch (o->kind)
+	{
+	case OPTION_STRING:
+	  printf ("%s:string %s ", o->name, o->info.string);
+	  break;
+	case OPTION_TYPE:
+	  printf ("%s:type ", o->name);
+	  dump_type (indent+1, o->info.type);
+	  break;
+	case OPTION_NESTED:
+	  printf ("%s:nested ", o->name);
+	  break;
+	case OPTION_NONE:
+	  fatal ("corrupted option %p: %s", (void*) o, o->name);
+	}
+      o = o->next;
     }
   printf ("\n");
 }
--- ../thirdround_04_filesrules//gengtype.h	2010-09-21 10:27:50.000000000 +0200
+++ gcc/gengtype.h	2010-09-21 16:11:46.000000000 +0200
@@ -135,12 +135,256 @@  extern size_t num_lang_dirs;
 
 
 
-/* Data types handed around within, but opaque to, the lexer and parser.  */
+/* Common types, for structures defined below.  */
 typedef struct pair *pair_p;
 typedef struct type *type_p;
 typedef const struct type *const_type_p;
 typedef struct options *options_p;
 
+/* Various things, organized as linked lists, needed both in
+   gengtype.c & in gengtype-state.c files.  */
+extern pair_p typedefs;
+extern type_p structures;
+extern type_p param_structs;
+extern pair_p variables;
+
+
+
+/* Discrimating kind of types we can understand.  */
+
+enum typekind {
+  TYPE_NONE=0,		/* Never used, so zero-ed memory is
+			   invalid.  */
+  TYPE_SCALAR,		/* Scalar types like char.  */
+  TYPE_STRING,		/* The string type.  */
+  TYPE_STRUCT,		/* Type for GTY-ed struct-ures.  */
+  TYPE_UNION,		/* Type for GTY-ed discriminated
+			   union-s.  */
+  TYPE_POINTER,		/* Pointer type to GTY-ed type.  */
+  TYPE_ARRAY,		/* Array of GTY-ed types.  */
+  TYPE_LANG_STRUCT,	/* GCC front-end language specific
+			   struct-s.  Various languages may have
+			   homonymous but different
+			   struct-s.  */
+  TYPE_PARAM_STRUCT	/* Type for parametrized struct-ures,
+			   e.g. hash_t hash-tables, ...  See
+			   (param_is, use_param, param1_is,
+			   param2_is,... use_param1,
+			   use_param_2,... use_params) GTY
+			   options.  */
+};
+
+/* Discriminating kind for options.  */
+enum option_kind {
+  OPTION_NONE=0,		/* Never used, so zero-ed memory is
+				   invalid.  */
+  OPTION_STRING,		/* A string-valued option.  Most options
+				   are strings.  */
+  OPTION_TYPE,			/* A type-valued option.  */
+  OPTION_NESTED			/* Option data for 'nested_ptr'.  */
+};
+
+
+/* A way to pass data through to the output end.  */
+struct options {
+  struct options *next;		/* next option of the same pair.  */
+  const char *name;		/* GTY option name.  */
+  enum option_kind kind;		/* disciminating option kind.  */
+  /* the union below is discriminated by the 'kind' field above.  */
+  union {
+    const char* string;		    /* When OPTION_STRING.  */
+    type_p type;		    /* When OPTION_TYPE.  */
+    struct nested_ptr_data* nested; /* when OPTION_NESTED.  */
+  } info;
+};
+
+
+/* Option data for the 'nested_ptr' option.  */
+struct nested_ptr_data {
+  type_p type;
+  const char *convert_to;
+  const char *convert_from;
+};
+
+/* Some functions to create various options structures with name NAME
+   and info INFO.  NEXT is the next option in the chain.  */
+
+/* Create a string option.  */
+options_p create_string_option (options_p next, const char* name,
+                                const char* info);
+
+/* Create a type option.  */
+options_p create_type_option (options_p next, const char* name,
+                              type_p info);
+
+/* Create a nested option.  */
+options_p create_nested_option (options_p next, const char* name,
+				struct nested_ptr_data* info);
+
+/* Create a nested pointer option.  */
+options_p create_nested_ptr_option (options_p, type_p t,
+			 	     const char *from, const char *to);
+
+/* A name and a type.  */
+struct pair {
+  pair_p next;			/* The next pair in the linked list.  */
+  const char *name;		/* The defined name.  */
+  type_p type;			/* Its GTY-ed type.  */
+  struct fileloc line;		/* The file location.  */
+  options_p opt;		/* GTY options, as a linked list.  */
+};
+
+/* Usage information for GTY-ed types.  Gengtype has to care only of
+   used GTY-ed types.  Types are initially unused, and their usage is
+   computed by set_gc_used_type and set_gc_used functions.  */
+
+enum gc_used_enum {
+
+  /* We need that zero-ed types are initially unused.  */
+  GC_UNUSED=0,
+
+  /* The GTY-ed type is used, e.g by a GTY-ed variable or a field
+     inside a GTY-ed used type.  */
+  GC_USED,
+
+  /* For GTY-ed structures whose definitions we haven't seen so far
+     when we encounter a pointer to it that is annotated with
+     ``maybe_undef''.  If after reading in everything we don't have
+     source file information for it, we assume that it never has been
+     defined.  */
+  GC_MAYBE_POINTED_TO,
+
+  /* For known GTY-ed structures which are pointed to by GTY-ed
+     variables or fields.  */
+  GC_POINTED_TO
+};
+
+/* We have at most ten type parameters in parameterized structures.  */
+#define NUM_PARAM 10
+
+/* Our type structure describes all types handled by gengtype.  */
+struct type {
+  /* Discriminating kind, cannot be TYPE_NONE.  */
+  enum typekind kind;
+
+  /* For top-level struct-s or union-s, the 'next' field link the
+     global list 'structures' or 'param_structs'; for lang_struct-s,
+     their homonymous struct-s are linked using this next field.  The
+     homonymous list starts at the s.lang_struct field of the
+     lang_struct.  See the new_structure function for details.  This is
+     tricky!  */
+  type_p next;
+
+  /* State number used when writing & reading the persistent state.  A
+     type with a positive number has already been written.  For ease
+     of debugging, newly allocated types have a unique negative
+     number.  */
+  int state_number;
+
+  /* Each GTY-ed type which is pointed to by some GTY-ed type knows
+     the GTY pointer type pointing to it.  See create_pointer
+     function.  */
+  type_p pointer_to;
+
+  /* Type usage information, computed by set_gc_used_type and
+     set_gc_used functions.  */
+  enum gc_used_enum gc_used;
+
+  /* The following union is discriminated by the 'kind' field above.  */
+  union {
+    /* TYPE__NONE is impossible.  */
+
+    /* when TYPE_POINTER:  */
+    type_p p;
+
+    /* when TYPE_STRUCT or TYPE_UNION or TYPE_LANG_STRUCT, we have an
+       aggregate type containing fields: */
+    struct {
+      const char *tag;		/* the aggragate tag, if any.  */
+      struct fileloc line;	/* the source location.  */
+      pair_p fields;		/* the linked list of fields.  */
+      options_p opt;		/* the GTY options if any.  */
+      lang_bitmap bitmap;	/* the set of front-end languages
+				   using that GTY-ed aggregate.  */
+      /* For TYPE_LANG_STRUCT, the lang_struct field gives the first
+	 element of a linked list of homonymous struct or union types.
+	 Within this list, each homonymous type has as its lang_struct
+	 field the original TYPE_LANG_STRUCT type.  This is a dirty
+	 trick, see the new_structure function for details.  */
+      type_p lang_struct;
+    } s;
+
+    /* when TYPE_SCALAR: */
+    bool scalar_is_char;
+
+    /* when TYPE_ARRAY: */
+    struct {
+      type_p p;		/* The array component type.  */
+      const char *len;		/* The string if any giving its length.  */
+    } a;
+
+    /* When TYPE_PARAM_STRUCT for (param_is, use_param, param1_is,
+       param2_is, ... use_param1, use_param_2, ... use_params) GTY
+       options.  */
+    struct {
+      type_p stru;		/* The generic GTY-ed type.  */
+      type_p param[NUM_PARAM];	/* The actual parameter types.  */
+      struct fileloc line;	/* The source location.  */
+    } param_struct;
+
+  } u;
+};
+
+/* The one and only TYPE_STRING.  */
+extern struct type string_type;
+
+/* The two and only TYPE_SCALARs.  Their u.scalar_is_char flags are
+   set appropriately early in main.  */
+extern struct type scalar_nonchar;
+extern struct type scalar_char;
+
+/* Gives the file location of a type, if any.  */
+static inline struct fileloc*
+type_lineloc (const_type_p ty)
+{
+  if (!ty)
+    return NULL;
+  switch (ty->kind)
+    {
+    case TYPE_NONE:
+      fatal ("type_lineloc on bad zeroed type %p", (const void*)ty);
+    case TYPE_STRUCT:
+    case TYPE_UNION:
+    case TYPE_LANG_STRUCT:
+      return CONST_CAST (struct fileloc*, &ty->u.s.line);
+    case TYPE_PARAM_STRUCT:
+      return CONST_CAST (struct fileloc*, &ty->u.param_struct.line);
+    case TYPE_SCALAR:
+    case TYPE_STRING:
+    case TYPE_POINTER:
+    case TYPE_ARRAY:
+      return NULL;
+    }
+  fatal ("type_lineloc on corrupted type %p kd#%d",
+	 (const void*)ty, (int) ty->kind);
+}
+
+/* Test if a type is a union, either a plain one or a language
+   specific one.  */
+#define UNION_P(x)					\
+    ((x)->kind == TYPE_UNION ||			\
+     ((x)->kind == TYPE_LANG_STRUCT 			\
+      && (x)->u.s.lang_struct->kind == TYPE_UNION))
+
+/* Test if a type is a union or a structure, perhaps a language
+   specific one.  */
+#define UNION_OR_STRUCT_P(x)			\
+    ((x)->kind == TYPE_UNION 			\
+     || (x)->kind == TYPE_STRUCT		\
+     || (x)->kind == TYPE_LANG_STRUCT)
+
+
+
 /* Variables used to communicate between the lexer and the parser.  */
 extern int lexer_toplevel_done;
 extern struct fileloc lexer_line;
--- ../thirdround_04_filesrules//gengtype-parse.c	2010-09-21 07:36:06.000000000 +0200
+++ gcc/gengtype-parse.c	2010-09-21 16:04:11.000000000 +0200
@@ -343,10 +343,9 @@  str_optvalue_opt (options_p prev)
       value = string_seq ();
       require (')');
     }
-  return create_option (prev, name, value);
+  return create_string_option (prev, name, value);
 }
-
-/* absdecl: type '*'*
+ /* absdecl: type '*'*
    -- a vague approximation to what the C standard calls an abstract
    declarator.  The only kinds that are actually used are those that
    are just a bare type and those that have trailing pointer-stars.
@@ -381,10 +380,9 @@  type_optvalue (options_p prev, const cha
   require ('(');
   ty = absdecl ();
   require (')');
-  return create_option (prev, name, ty);
+  return create_type_option (prev, name, ty);
 }
-
-/* Nested pointer data: '(' type '*'* ',' string_seq ',' string_seq ')' */
+ /* Nested pointer data: '(' type '*'* ',' string_seq ',' string_seq ')' */
 static options_p
 nestedptr_optvalue (options_p prev)
 {
@@ -431,7 +429,7 @@  option (options_p prev)
       parse_error ("expected an option keyword, have %s",
 		   print_cur_token ());
       advance ();
-      return create_option (prev, "", "");
+      return create_string_option (prev, "", "");
     }
 }