This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: patch ping^3: gengtype: improved version for plugin support.


Laurynas Biveinis wrote:
Hello -

Some comments about your patch. Of couse I should mention that I
cannot approve it.


Index: gcc/gengtype.h =================================================================== --- gcc/gengtype.h (revision 151258) +++ gcc/gengtype.h (working copy) @@ -76,6 +76,9 @@ extern void parse_file (const char *name); extern bool hit_error;

+/* flag set when parsing a plugin file */
+extern bool is_plugin_file;
+
 /* Token codes.  */
 enum {
   EOF_TOKEN = 0,

This variable is used only in gengtype.c, so it should belong there as
a static variable.

+ bool inplugin; /* flag set if appearing inside a plugin */

in_plugin perhaps, so that it follows overall naming convention?

-bool hit_error = false;
+bool hit_error = FALSE;

Why this change?

+/* Flag set when parsing a plugin file */
+bool is_plugin_file = FALSE;

Lowercase "false" would be consistent.


+ /* header file should be generated even in plugin mode */ header_file = create_file ("GCC", "gtype-desc.h"); +

+
   base_files = XNEWVEC (outf_p, num_lang_dirs);

Extraneous whitespace lines?

@@ -1763,9 +1817,9 @@

       if (lang_index >= 0)
 	return base_files[lang_index];
-
+

Likewise.

-/* Write out the 'enum' definition for gt_types_enum.  */
+/* Write out only to header_file the 'enum' definition for gt_types_enum.  */

 static void
 write_enum_defn (type_p structures, type_p param_structs)

Where does this enum definition get output to in case of plugins? I
guess it's not to gtype-desc.h in that case?

Also, IIRC not much at the moment uses this definition, so it might be
hard to actually test this change.

@@ -3332,12 +3450,15 @@

   for (v = variables; v; v = v->next)
     {
-      outf_p f = get_output_file_with_visibility (v->line.file);
+      outf_p f = NULL;
       struct flist *fli;
       const char *length = NULL;
       int deletable_p = 0;
       options_p o;
-
+      if (nb_plugin_files > 0 && plugin_output_filename && v->inplugin)
+	f = plugin_output;
+      else
+	f = get_output_file_with_visibility (v->line.file);
       for (o = v->opt; o; o = o->next)
 	if (strcmp (o->name, "length") == 0)
 	  length = o->info;

I think a cleaner abstraction would be to make
get_output_file_with_visibility handle plugins too, with extra
parameters passed.

+/* in plugin mode, the write of functions for structure is delayed to
+   the end; we keep a vector of these */

"...mode, writing of functions...", but my English is not too good to
know for sure.

+static void
+delay_func_for_structure (type_p s, const struct write_types_data* wtd)

A comment stating that it marks function for later output would be helpful here.



--
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 mines, sont seulement les miennes} ***


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]