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

[Laurynas Biveinis <laurynas.biveinis@gmail.com>]

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