Patchwork gengtype plugin improvement last4round -patch 5 [typedopt]

login
register
mail settings
Submitter Laurynas Biveinis
Date Nov. 23, 2010, 5:13 a.m.
Message ID <AANLkTi=G7105bKunA3-R5N3+gB9Ck3SkJC6HRd9iSRzJ@mail.gmail.com>
Download mbox | patch
Permalink /patch/72628/
State New
Headers show

Comments

Laurynas Biveinis - Nov. 23, 2010, 5:13 a.m.
Hello,

+/* 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);

You initialize all four fields immediately, so just XNEW. Likewise in
create_type_option and create_nested_option.

+  o->kind = OPTION_STRING;
+  o->next = next;
+  o->name = name;
+  o->info.string = (const char*) info;

This cast is probably unnecessary.

@@ -823,10 +755,9 @@ create_nested_ptr_option (options_p next, type_p t
[...]
-/* 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

Please drop this chunk.

@@ -1082,14 +1014,11 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUS
   /* 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++)

Please fix indentation.

@@ -1227,12 +1156,10 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUS
       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

Likewise.

@@ -1370,12 +1299,11 @@ process_gc_options (options_p opt, enum gc_used_en
[...]
-
-/* 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])
 {

Please do not reindent the comment.

@@ -2828,6 +2764,8 @@ write_types_process_field (type_p f, const struct

   switch (f->kind)
     {
+    case TYPE_NONE:
+      fatal ("uninitialized type to mangle");

Just gcc_unreachable.

@@ -2948,18 +2886,20 @@ write_func_for_structure (type_p orig_s, type_p 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");

Indentation is wrong (for loop and the last if)

@@ -3697,10 +3639,10 @@ write_root (outf_p f, pair_p v, type_p type, const
 		  {
 		    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)

Indentation is wrong for the line starting with &&.

@@ -3871,10 +3813,10 @@ write_roots (pair_p variables, bool emit_pch)
       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)

Indentation is wrong.

@@ -4001,12 +3943,11 @@ write_roots (pair_p variables, bool emit_pch)
       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))

Indentation is wrong.

@@ -4141,10 +4082,9 @@ note_def_vec (const char *type_name, bool is_scala
[...]
-  /* 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);

Please do not re-indent this comment.

@@ -4461,7 +4401,21 @@ dump_options (int indent, options_p opt)
[...]
+	case OPTION_NONE:
+	  fatal ("corrupted option %p: %s", (void*) o, o->name);
+	}

gcc_unreachable

Patch

Index: gcc/gengtype.h
===================================================================
--- gcc/gengtype.h	(revision 167024)
+++ gcc/gengtype.h	(working copy)
@@ -116,6 +116,250 @@  typedef struct options *options_p;
[...]
+enum typekind {
+  TYPE_NONE=0,		/* Never used, so zero-ed memory is
+			   invalid.  */

"zeroed"

+  TYPE_SCALAR,		/* Scalar types like char.  */
+  TYPE_STRING,		/* The string type.  */
+  TYPE_STRUCT,		/* Type for GTY-ed struct-ures.  */

"structs"

+  TYPE_UNION,		/* Type for GTY-ed discriminated
+			   union-s.  */

"unions"

+  TYPE_POINTER,		/* Pointer type to GTY-ed type.  */

Align the comment

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

"structs", "structs"

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

Align the 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.  */

"discriminating"

+  /* the union below is discriminated by the 'kind' field above.  */

Redundant.

+  union {
+    const char* string;		    /* When OPTION_STRING.  */
+    type_p type;		    /* When OPTION_TYPE.  */
+    struct nested_ptr_data* nested; /* when OPTION_NESTED.  */
+  } info;
+};

Please align the comments.

+/* We have at most ten type parameters in parameterized structures.  */
+#define NUM_PARAM 10

"We can have ... "

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

"structs" and "unions". "lang_struct-s" can become just "lang_struct".

+/* The two and only TYPE_SCALARs.  Their u.scalar_is_char flags are
+   set appropriately early in main.  */

"Their foo flags are set 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:
+      gcc_unreachable ();
+    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;
+    }
+  gcc_unreachable ();
+  return NULL;
+}

This is never used. Presumably some other patch will use it, so please
move it there.

2010/11/22 Basile Starynkevitch <basile@starynkevitch.net>:
> Hello All,
>
> References:
> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01744.html
> and previous references
>
> Attached patch to trunk r167024 did bootstrap.
>
>
> ######## gcc/ChangeLog entry
> 2010-11-22  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

Here and everywhere: present tense.

>        (string_type, scalar_nonchar, scalar_char): Removed static and
>        added state_number fake value.

Fake?

>        (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.

"Check the union discriminator before accessing options"

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

"Use create_string_option and create_type_option"

Thanks,
-- 
Laurynas