Patchwork gengtype improvements for plugins, thirdround! patch 2/7 [verbosity]

login
register
mail settings
Submitter Basile Starynkevitch
Date Sept. 21, 2010, 7:08 p.m.
Message ID <20100921210829.5f2ea8b0.basile@starynkevitch.net>
Download mbox | patch
Permalink /patch/65370/
State New
Headers show

Comments

Basile Starynkevitch - Sept. 21, 2010, 7:08 p.m.
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-09/msg01702.html

The second piece [verbosity] of our serie of patches adds verbosity to
gengtype. The gengtype user (either a plugin developer or a true GCC
hacker) can pass the -v program flag. This flag can even be passed
more than once, to increase the verbosity level. Verbose messages
explain succintly to the user what gengtype is actually doing.  We
feel this functionality is useful for plugin developpers (who might
not understand what gengtype is doing today, since it is essentially
silent) and even for GCC hackers.

We did took into account Laurynas comments from
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00210.html

In addition of verbose messages, we added a tiny but useful feature to
gengtype. When given a backup directory thru the -B program option and
when it is generating a file gt-foo.h, gengtype renames the old
version into the backup directory so that the user could compare the
old version with the current one.  Verbosity & backing up of old files
go together (in verbose mode, the renaming is told to the
user). Backing up is no longer by default.  Per Laurynas' wish, it
should be enabled by its program option.

Some existing functions has been slightly improved, for instance to
count the number of GTY-ed variables.

The verbosity_level variable needs to be global. It will be used in
file gengtype-state.c.

The attached patch is relative to the previous patch 1 [declprog] sent
in http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01702.html This
relative patch was obtained with
  diff -u -p  -N $(svn stat | awk '/[AM]/{print $2}') \
      --from-file ../thirdround_01_declprog/ > \
        $HOME/Gengtype_Thirdround/patch2_verbosity-relto01.diff

I am also attaching the gzipped cumulated patches against trunk 164437 
as file cumulatedpatch2_verbosity__gengtypethird_r164437.diff.gz

######################################
gcc/ChangeLog entry [of the relative patch]
2010-09-20  Jeremie Salvucci  <jeremie.salvucci@free.fr>
	    Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c (verbosity_level, backup_dir): Added variables.
	(set_gc_used): Count variables for verbosity.
	(close_output_files): Backing up files, counting written ones
	verbosily.
	(write_types): Count emitted functions for verbosity. Added
        debug messages.
	(write_enum_defn): Count structures for verbosity. Added debug
	messages.
	(gengtype_long_options): Added "verbose".
	(print_usage): Ditto.
	(main): Verbose display of parsed files.

	* gengtype.h (verbosity_level): Added declaration.

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

Ok for trunk?
Laurynas Biveinis - Sept. 22, 2010, 2:07 a.m.
2010/9/21 Basile Starynkevitch <basile@starynkevitch.net>:
> In addition of verbose messages, we added a tiny but useful feature to
> gengtype. When given a backup directory thru the -B program option and
> when it is generating a file gt-foo.h, gengtype renames the old
> version into the backup directory so that the user could compare the
> old version with the current one.  Verbosity & backing up of old files
> go together (in verbose mode, the renaming is told to the
> user). Backing up is no longer by default.  Per Laurynas' wish, it
> should be enabled by its program option.

Although I agree that if backups go in, they should have their own
switch, my wish was to remove the backup feature altogether. Merits of
backups aside, since now backup is independent from verbosity, it
should be submitted in a separate patch.

@@ -1511,8 +1520,15 @@ static void
 set_gc_used (pair_p variables)
 {
   pair_p p;
+  int nbvars = 0;
   for (p = variables; p; p = p->next)
+    {
+      DBGPRINTF ("set_gc_used p %p '%s' nbvars %d", (void*) p,
p->name, nbvars);
     set_gc_used_type (p->type, GC_USED, NULL);

Indentation is wrong.

+      if (!is_file_equal (of))
+	{

Indentation is wrong.

@@ -2894,11 +2941,26 @@ write_types (outf_p output_header, type_
 	  {
 	    type_p ss;
 	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
+	      {
+		nbfun++;
+		DBGPRINTF ("writing func #%d lang_struct ss @ %p '%s'",
+			   nbfun, (void*) ss, ss->u.s.tag);
 	      write_func_for_structure (s, ss, NULL, wtd);
 	  }
+	  }
 	else
+	  {
+	    nbfun++;
+	    DBGPRINTF ("writing func #%d struct s @ %p '%s'",
+		       nbfun, (void*) s, s->u.s.tag);
 	  write_func_for_structure (s, s, NULL, wtd);
       }
+      }

Indentation is wrong.

+	  DBGPRINTF ("write_enum_defn s %p #%d is unionorstruct tagged %s",

union or struct. Also indentation is wrong in the code surrounding this line.

> 2010-09-20  Jeremie Salvucci  <jeremie.salvucci@free.fr>
>            Basile Starynkevitch  <basile@starynkevitch.net>
>
>        * gengtype.c (verbosity_level, backup_dir): Added variables.
>        (set_gc_used): Count variables for verbosity.
>        (close_output_files): Backing up files, counting written ones
>        verbosily.
>        (write_types): Count emitted functions for verbosity. Added
>        debug messages.
>        (write_enum_defn): Count structures for verbosity. Added debug
>        messages.
>        (gengtype_long_options): Added "verbose".
>        (print_usage): Ditto.
>        (main): Verbose display of parsed files.
>
>        * gengtype.h (verbosity_level): Added declaration.

Patch

--- ../thirdround_01_declprog//gengtype.c	2010-09-20 21:36:24.000000000 +0200
+++ gcc/gengtype.c	2010-09-20 22:35:07.000000000 +0200
@@ -158,6 +158,15 @@  const char *write_state_filename;
 int do_dump;
 int do_debug;
 
+/* For verbose messages to the user.  */
+int verbosity_level;
+
+/* The backup directory should be in the same file-system as the
+   generated files, otherwise the rename(2) system call would fail.
+   If NULL, no backup is made when overwriting a generated file.  */
+static const char* backup_dir;
+
+
 static outf_p create_file (const char *, const char *);
 
 static const char * get_file_basename (const char *);
@@ -1511,8 +1520,15 @@  static void
 set_gc_used (pair_p variables)
 {
   pair_p p;
+  int nbvars = 0;
   for (p = variables; p; p = p->next)
+    {
+      DBGPRINTF ("set_gc_used p %p '%s' nbvars %d", (void*) p, p->name, nbvars);
     set_gc_used_type (p->type, GC_USED, NULL);
+      nbvars++;
+    };
+  if (verbosity_level >= 2)
+    printf ("%s used %d GTY-ed variables\n", progname, nbvars);
 }
 
 /* File mapping routines.  For each input file, there is one output .c file
@@ -1901,26 +1917,56 @@  static void
 close_output_files (void)
 {
   outf_p of;
+  int nbwrittenfiles = 0;
 
   for (of = output_files; of; of = of->next)
     {
 
-      if (!is_file_equal(of))
-      {
-        FILE *newfile = fopen (of->name, "w");
-        if (newfile == NULL)
-          fatal ("opening output file %s: %s", of->name, xstrerror (errno));
-        if (fwrite (of->buf, 1, of->bufused, newfile) != of->bufused)
-          fatal ("writing output file %s: %s", of->name, xstrerror (errno));
-        if (fclose (newfile) != 0)
-          fatal ("closing output file %s: %s", of->name, xstrerror (errno));
-      }
+      if (!is_file_equal (of))
+	{
+	  FILE *newfile = NULL;
+	  char *backupname = NULL;
+	  /* Backup the old version of the output file gt-FOO.c as
+	     BACKUPDIR/gt-FOO.c~ if we have a backup directory.  */
+	  if (backup_dir)
+	    {
+	      backupname = concat (backup_dir, "/",
+				   lbasename (of->name), "~", NULL);
+	      if (!access (of->name, F_OK) && rename (of->name, backupname))
+		fatal ("failed to backup %s as %s: %s",
+		       of->name, backupname, xstrerror (errno));
+	    }
+	  newfile = fopen (of->name, "w");
+	  if (newfile == NULL)
+	    fatal ("opening output file %s: %s", of->name, xstrerror (errno));
+	  if (fwrite (of->buf, 1, of->bufused, newfile) != of->bufused)
+	    fatal ("writing output file %s: %s", of->name, xstrerror (errno));
+	  if (fclose (newfile) != 0)
+	    fatal ("closing output file %s: %s", of->name, xstrerror (errno));
+	  nbwrittenfiles++;
+	  if (verbosity_level >= 2 && backupname)
+	    printf ("%s wrote #%-3d %s backed-up in %s\n",
+		    progname, nbwrittenfiles, of->name, backupname);
+	  else if (verbosity_level >= 1)
+	    printf ("%s wrote #%-3d %s\n", progname, nbwrittenfiles, of->name);
+	  if (backupname)
+	    free (backupname);
+	}
+      else
+	{
+	  /* Output file remains unchanged! */
+	  if (verbosity_level >= 2)
+	    printf ("%s kept %s\n", progname, of->name);
+	}
       free(of->buf);
       of->buf = NULL;
       of->bufused = of->buflength = 0;
     }
+  if (verbosity_level >= 1)
+    printf ("%s wrote %d files.\n", progname, nbwrittenfiles);
 }
 
+
 struct flist {
   struct flist *next;
   int started_p;
@@ -2800,6 +2846,7 @@  write_types (outf_p output_header, type_
 	     const struct write_types_data *wtd)
 {
   type_p s;
+  int nbfun = 0; /* Count the emitted functions.  */
 
   oprintf (output_header, "\n/* %s*/\n", wtd->comment);
   /* We first emit the macros and the declarations. Functions' code is
@@ -2894,11 +2941,26 @@  write_types (outf_p output_header, type_
 	  {
 	    type_p ss;
 	    for (ss = s->u.s.lang_struct; ss; ss = ss->next)
+	      {
+		nbfun++;
+		DBGPRINTF ("writing func #%d lang_struct ss @ %p '%s'",
+			   nbfun, (void*) ss, ss->u.s.tag);
 	      write_func_for_structure (s, ss, NULL, wtd);
 	  }
+	  }
 	else
+	  {
+	    nbfun++;
+	    DBGPRINTF ("writing func #%d struct s @ %p '%s'",
+		       nbfun, (void*) s, s->u.s.tag);
 	  write_func_for_structure (s, s, NULL, wtd);
       }
+      }
+    else
+      DBGPRINTF ("ignored s @ %p  '%s' gc_used#%d",
+		 (void*)s,  s->u.s.tag,
+		 (int) s->gc_used);
+
   for (s = param_structs; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO)
       {
@@ -2910,11 +2972,27 @@  write_types (outf_p output_header, type_
 	  {
 	    type_p ss;
 	    for (ss = stru->u.s.lang_struct; ss; ss = ss->next)
+	      {
+		nbfun++;
+		DBGPRINTF ("writing func #%d param lang_struct ss @ %p '%s'",
+			   nbfun, (void*) ss,  ss->u.s.tag);
 	      write_func_for_structure (s, ss, param, wtd);
 	  }
+	  }
 	else
+	  {
+	    nbfun++;
+	    DBGPRINTF ("writing func #%d param struct s @ %p stru @ %p '%s'",
+		       nbfun, (void*) s,
+		       (void*) stru,  stru->u.s.tag);
 	  write_func_for_structure (s, stru, param, wtd);
       }
+      }
+    else
+      DBGPRINTF ("ignored s @ %p", (void*)s);
+  if (verbosity_level >= 2)
+    printf ("%s emitted %d routines for %s\n",
+	    progname, nbfun, wtd->comment);
 }
 
 static const struct write_types_data ggc_wtd =
@@ -3108,27 +3186,44 @@  static void
 write_enum_defn (type_p structures, type_p param_structs)
 {
   type_p s;
+  int nbstruct = 0;
+  int nbparamstruct = 0;
 
   if (!header_file)
     return;
   oprintf (header_file, "\n/* Enumeration of types known.  */\n");
   oprintf (header_file, "enum gt_types_enum {\n");
+
   for (s = structures; s; s = s->next)
     if (USED_BY_TYPED_GC_P (s))
       {
+	DBGPRINTF ("write_enum_defn s @ %p nbstruct %d",
+		  (void*) s, nbstruct);
+	if (UNION_OR_STRUCT_P (s))
+	  DBGPRINTF ("write_enum_defn s %p #%d is unionorstruct tagged %s",
+		     (void*) s, nbstruct, s->u.s.tag);
 	oprintf (header_file, " gt_ggc_e_");
 	output_mangled_typename (header_file, s);
 	oprintf (header_file, ",\n");
+	nbstruct++;
       }
+
   for (s = param_structs; s; s = s->next)
     if (s->gc_used == GC_POINTED_TO)
       {
+	DBGPRINTF ("write_enum_defn s %p nbparamstruct %d",
+		   (void*) s, nbparamstruct);
 	oprintf (header_file, " gt_e_");
 	output_mangled_typename (header_file, s);
 	oprintf (header_file, ",\n");
+	nbparamstruct++;
       }
+
   oprintf (header_file, " gt_types_enum_last\n");
   oprintf (header_file, "};\n");
+  if (verbosity_level >= 2)
+    printf ("%s handled %d GTY-ed structures & %d parameterized structures.\n",
+	    progname, nbstruct, nbparamstruct);
 }
 
 /* Might T contain any non-pointer elements?  */
@@ -4273,17 +4368,19 @@  dump_everything (void)
 
 /* Option specification for getopt_long.  */
 static const struct option gengtype_long_options[] = {
-  { "help",      no_argument, NULL, 'h' },
-  { "version",   no_argument, NULL, 'V' },
-  { "dump",      no_argument, NULL, 'd' },
-  { "debug",     no_argument, NULL, 'D' },
-  { "plugin",    required_argument, NULL, 'P' },
-  { "srcdir",    required_argument, NULL, 'S' },
-  { "inputs",    required_argument, NULL, 'I' },
+  { "help",          no_argument, 	NULL, 'h' },
+  { "version",       no_argument, 	NULL, 'V' },
+  { "verbose",       no_argument, 	NULL, 'v' },
+  { "dump",          no_argument, 	NULL, 'd' },
+  { "debug",         no_argument, 	NULL, 'D' },
+  { "plugin",        required_argument, NULL, 'P' },
+  { "srcdir",        required_argument, NULL, 'S' },
+  { "backupdir",     required_argument, NULL, 'B' },
+  { "inputs",        required_argument, NULL, 'I' },
   { "read-state",    required_argument, NULL, 'r' },
-  { "write-state",    required_argument, NULL, 'w' },
+  { "write-state",   required_argument, NULL, 'w' },
   /* Terminating NULL placeholder.  */
-  { NULL,        no_argument, NULL, 0   },
+  { NULL,            no_argument,       NULL,  0  },
 };
 
 
@@ -4298,12 +4395,16 @@  print_usage (void)
             progname);
     printf ("\t -V | --version "
 	    " \t# Give version information.\n");
+    printf ("\t -v | --verbose "
+	    " \t# Increase verbosity.  Can be given several times.\n");
     printf ("\t -d | --dump "
 	    " \t# Dump state for debugging.\n");
     printf ("\t -P | --plugin <output-file> <plugin-src> ... "
             " \t# Generate for plugin.\n");
     printf ("\t -S | --srcdir <GCC-directory> "
             " \t# Specify the GCC source directory.\n");
+    printf ("\t -B | --backupdir <directory> "
+            " \t# Specify the backup directory for updated files.\n");
     printf ("\t -I | --inputs <input-list> "
             " \t# Specify the file with source files list.\n");
     printf ("\t -w | --write-state <state-file> "
@@ -4324,7 +4425,7 @@  static void
 parse_program_options (int argc, char**argv)
 {
     int opt = -1;
-    while ((opt = getopt_long (argc, argv, "hVdP:S:I:w:r:D",
+  while ((opt = getopt_long (argc, argv, "hvVdP:S:B:I:w:r:D",
 			       gengtype_long_options, NULL)) >= 0)
     {
 	switch (opt)
@@ -4332,6 +4433,9 @@  parse_program_options (int argc, char**a
 	case 'h': /* --help */
 	    print_usage ();
 	    break;
+	case 'v': /* --verbose */
+	  verbosity_level++;
+	  break;
 	case 'V': /* --version */
 	    print_version ();
 	    break;
@@ -4354,6 +4458,13 @@  parse_program_options (int argc, char**a
 		fatal ("missing source directory");
 	    srcdir_len = strlen (srcdir);
 	    break;
+	case 'B': /* --backupdir */
+	    if (optarg)
+		srcdir = optarg;
+	    else
+		fatal ("missing backup directory");
+	    srcdir_len = strlen (srcdir);
+	    break;
 	case 'I': /* --inputs */
 	    if (optarg)
 		inputlist = optarg;
@@ -4452,6 +4563,8 @@  main (int argc, char **argv)
       }
       DBGPRINT_COUNT_TYPE ("structures after parsing", structures);
       DBGPRINT_COUNT_TYPE ("param_structs after parsing", param_structs);
+      if (verbosity_level >= 1)
+	printf ("%s parsed %d files\n", progname, (int) num_gt_files);
 
     }
   else
--- ../thirdround_01_declprog//gengtype.h	2010-09-20 21:12:46.000000000 +0200
+++ gcc/gengtype.h	2010-09-20 22:03:42.000000000 +0200
@@ -154,6 +154,9 @@  enum {
   FIRST_TOKEN_WITH_VALUE = PARAM_IS
 };
 
+/* Level for verbose messages.  Gengtype tells something to its user
+   when it is positive, and tells more if it is greater.  */
+extern int verbosity_level;
 
 /* For debugging purposes of gengtype itself!  */
 extern int do_dump;