diff mbox

Use indentation in gtype.state to show nested structure

Message ID 1367628521-1868-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm May 4, 2013, 12:48 a.m. UTC 
Whilst learning the internals of the GTY code I noticed that gtype.state's
s-expressions are all aligned to the left-hand margin.

The following patch to gengtype-state.c rewrites how they are written out
to introduce indentation, showing the nesting structure of the
s-expressions, which makes the generated gtype.state file somewhat more
human-readable (to this human, at least).

You can see before/after gtype.state files here:
Before the patch:
 http://dmalcolm.fedorapeople.org/gcc/2013-05-03/gtype.state.old.txt
After the patch:
 http://dmalcolm.fedorapeople.org/gcc/2013-05-03/gtype.state.new.txt

Comparison with "diff -b" shows that it only contains whitespace changes,
which read_a_state_token () ignores.

Successfully bootstrapped against gcc-4.7.2-2.fc17.x86_64

---
 gcc/ChangeLog        |  40 +++++++++++
 gcc/gengtype-state.c | 198 +++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 193 insertions(+), 45 deletions(-)

Comments

Laurynas Biveinis May 6, 2013, 4:56 a.m. UTC | #1 
> +2013-05-03  David Malcolm  <dmalcolm@redhat.com>
> +
> +       * gengtype-state.c: (indent_amount): New variable,
> +       (had_recent_newline): likewise
> +       (write_new_line): New function
> +       (write_any_indent): likewise
> +       (write_open_paren): likewise
> +       (write_close_paren): likewise
> +       (write_state_fileloc): Use the new functions above to write
> +       indentation into the gtype.state output file to visually represent
> +       the hierarchical structure of the list structures
> +       (write_state_fields): ditto
> +       (write_state_a_string): ditto
> +       (write_state_string_option): ditto
> +       (write_state_type_option): ditto
> +       (write_state_nested_option): ditto
> +       (write_state_option): ditto
> +       (write_state_options): ditto
> +       (write_state_lang_bitmap): ditto
> +       (write_state_version): ditto
> +       (write_state_scalar_type): ditto
> +       (write_state_string_type): ditto
> +       (write_state_undefined_type): ditto
> +       (write_state_struct_union_type): ditto
> +       (write_state_user_struct_type): ditto
> +       (write_state_lang_struct_type): ditto
> +       (write_state_param_struct_type): ditto
> +       (write_state_pointer_type): ditto
> +       (write_state_array_type): ditto
> +       (write_state_gc_used): ditto
> +       (write_state_type): ditto
> +       (write_state_pair): ditto
> +       (write_state_typedefs): ditto
> +       (write_state_structures): ditto
> +       (write_state_param_structs): ditto
> +       (write_state_variables): ditto
> +       (write_state_variables): ditto
> +       (write_state_files_list): ditto
> +       (write_state_languages): ditto
> +

The patch is OK with some minor formatting errors fixed:

> @@ -714,14 +778,15 @@ write_state_options (options_p opt)
>
>    if (opt == NULL)
>      {
> -      fprintf (state_file, "nil ");
> +       write_any_indent (0);
> +       fprintf (state_file, "nil ");
>        return;
>      }

Indentation error

> @@ -729,16 +794,17 @@ write_state_options (options_p opt)
>  static void
>  write_state_lang_bitmap (lang_bitmap bitmap)
>  {
> -  fprintf (state_file, "%d ", (int) bitmap);
> +   write_any_indent (0);
> +   fprintf (state_file, "%d ", (int) bitmap);
>  }

Likewise

> @@ -889,7 +971,10 @@ write_state_param_struct_type (type_p current)
>        if (current->u.param_struct.param[i] != NULL)
>         write_state_type (current->u.param_struct.param[i]);
>        else
> +        {
> +       write_any_indent (0);
>         fprintf (state_file, "nil ");
> +        }
>      }
>    write_state_fileloc (&current->u.param_struct.line);
>  }

Likewise

> @@ -1155,17 +1260,19 @@ write_state_files_list (void)
>        cursrcrelpath = get_file_srcdir_relative_path (curfil);
>        if (cursrcrelpath)
>         {
> -         fprintf (state_file, "(!srcfile %d ", get_lang_bitmap (curfil));
> +         write_open_paren ("srcfile");
> +          fprintf (state_file, "%d ", get_lang_bitmap (curfil));
>           write_state_a_string (cursrcrelpath);
>         }
>        else

Likewise

--
Laurynas
Jeff Law May 6, 2013, 6:22 p.m. UTC | #2 
On 05/03/2013 06:48 PM, David Malcolm wrote:

>
> +static int indent_amount = 0;
> +static int had_recent_newline = 0;
Any clean way to do this without the global state?  I have to ask given 
you're generally working on removing global state.  Seems to take a tiny 
step backwards.


> +
> +static void
> +write_new_line (void)
> +{
> +  /* don't add a newline if we've just had one */
> +  if (!had_recent_newline)
> +    {
> +      fprintf (state_file, "\n");
> +      had_recent_newline = 1;
> +    }
> +}
Need a block comment before WRITE_NEW_LINE.

> +
> +/*
> +  If we've just had a newline, write the indentation amount,
> +  potentially modified.
> +
> +  The modifier exists to support code that writes strings with leading
> +  space (e.g " foo") which might occur within a line, or could be the first
> +  thing on a line.  By passing a modifier of -1, when such a string is the
> +  first thing on a line, the modifier swallows the leading space into the
> +  indentation, and all the fields line up correctly.
> +*/
Comment style is goofy.  Something like this seems better.  Note the 
placement of the start/end comment markers and use of caps when 
describing a variable or parameter name.


/* If we've just had a newline, write the indention amount,
    potentially modified.

    MODIFIER exists to support code that writes strings with leading
    space (e.g. " foo") which might occur within a line, or could be the
    first thing on a line.  When modifier is -1 the leading space into
    the indention is swallowed.  */

Hmm, reading it MODIFIER seems like a particularly bad NAME.  Can you 
come up with a better name for that parameter?



> +static void
> +write_any_indent (int modifier)
> +{
> +  int i;
> +  if (had_recent_newline)
> +    for (i = 0; i < indent_amount + modifier; i++)
> +      fprintf (state_file, " ");
> +  had_recent_newline = 0;
> +}
> +
> +static void
> +write_open_paren (const char *tag)
> +{
> +  write_new_line ();
> +  write_any_indent (0);
> +  fprintf (state_file, "(!%s ", tag);
> +  indent_amount++;
> +}
> +
> +static void
> +write_close_paren (void)
> +{
> +  indent_amount--;
> +  write_any_indent (0);
> +  fprintf (state_file, ")");
> +  write_new_line ();
> +}
Block comments before each function.  I realize they're pretty trivial, 
but we really try to always have that block comment.

It's probably worth noting that these also insert newlines since it's 
not trivial to determine simply by looking at the function name. 
Similarly for the open-paren.  It's actually open-paren-!.  Closing 
paren seems to be closing-paren + newline.  Not sure if it's worth 
renaming though.  Just thought I'd point it out since I had to 
double-check when reviewing the later code.



> +  write_close_paren (); /* (!fields */
We don't generally write comments on the end of lines like this. 
Belongs on the line before and generally should be written as a complete 
sentence.  This occurs often and needs to be fixed.

Please check the indention on your changes themselves. 
write_state_options & write_state_lang_bitmap look like they got mucked 
up, though I didn't look terribly closely.


With those nits fixed, I think this patch will be ready.  Please update 
& repost for final approval.

jeff
Laurynas Biveinis May 7, 2013, 5:42 a.m. UTC | #3 
2013/5/6 Jeff Law <law@redhat.com>:
> On 05/03/2013 06:48 PM, David Malcolm wrote:
>
>>
>> +static int indent_amount = 0;
>> +static int had_recent_newline = 0;
>
> Any clean way to do this without the global state?  I have to ask given
> you're generally working on removing global state.  Seems to take a tiny
> step backwards.

My 2c is that this global state is confined to gengtype, where the
increased modularity isn't going to pay off much anyway. Thus IMHO
it's not a big deal.
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index cafda6e..9c9ce2a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,43 @@ 
+2013-05-03  David Malcolm  <dmalcolm@redhat.com>
+
+	* gengtype-state.c: (indent_amount): New variable,
+	(had_recent_newline): likewise
+	(write_new_line): New function
+	(write_any_indent): likewise
+	(write_open_paren): likewise
+	(write_close_paren): likewise
+	(write_state_fileloc): Use the new functions above to write
+	indentation into the gtype.state output file to visually represent
+	the hierarchical structure of the list structures
+	(write_state_fields): ditto
+	(write_state_a_string): ditto
+	(write_state_string_option): ditto
+	(write_state_type_option): ditto
+	(write_state_nested_option): ditto
+	(write_state_option): ditto
+	(write_state_options): ditto
+	(write_state_lang_bitmap): ditto
+	(write_state_version): ditto
+	(write_state_scalar_type): ditto
+	(write_state_string_type): ditto
+	(write_state_undefined_type): ditto
+	(write_state_struct_union_type): ditto
+	(write_state_user_struct_type): ditto
+	(write_state_lang_struct_type): ditto
+	(write_state_param_struct_type): ditto
+	(write_state_pointer_type): ditto
+	(write_state_array_type): ditto
+	(write_state_gc_used): ditto
+	(write_state_type): ditto
+	(write_state_pair): ditto
+	(write_state_typedefs): ditto
+	(write_state_structures): ditto
+	(write_state_param_structs): ditto
+	(write_state_variables): ditto
+	(write_state_variables): ditto
+	(write_state_files_list): ditto
+	(write_state_languages): ditto
+
 2013-05-02  David Malcolm  <dmalcolm@redhat.com>
 
 	* testsuite/gcc.dg/plugin/one_time_plugin.c (one_pass_gate): example
diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
index 6092dad..86fa8aa 100644
--- a/gcc/gengtype-state.c
+++ b/gcc/gengtype-state.c
@@ -141,6 +141,57 @@  static long state_bol = 0;	/* offset of beginning of line */
 /* Counter of written types.  */
 static int state_written_type_count = 0;
 
+static int indent_amount = 0;
+static int had_recent_newline = 0;
+
+static void
+write_new_line (void)
+{
+  /* don't add a newline if we've just had one */
+  if (!had_recent_newline)
+    {
+      fprintf (state_file, "\n");
+      had_recent_newline = 1;
+    }
+}
+
+/*
+  If we've just had a newline, write the indentation amount,
+  potentially modified.
+
+  The modifier exists to support code that writes strings with leading
+  space (e.g " foo") which might occur within a line, or could be the first
+  thing on a line.  By passing a modifier of -1, when such a string is the
+  first thing on a line, the modifier swallows the leading space into the
+  indentation, and all the fields line up correctly.
+*/
+static void
+write_any_indent (int modifier)
+{
+  int i;
+  if (had_recent_newline)
+    for (i = 0; i < indent_amount + modifier; i++)
+      fprintf (state_file, " ");
+  had_recent_newline = 0;
+}
+
+static void
+write_open_paren (const char *tag)
+{
+  write_new_line ();
+  write_any_indent (0);
+  fprintf (state_file, "(!%s ", tag);
+  indent_amount++;
+}
+
+static void
+write_close_paren (void)
+{
+  indent_amount--;
+  write_any_indent (0);
+  fprintf (state_file, ")");
+  write_new_line ();
+}
 
 /* Fatal error messages when reading the state.  They are extremely
    unlikely, and only appear when this gengtype-state.c file is buggy,
@@ -565,16 +616,16 @@  write_state_fileloc (struct fileloc *floc)
       srcrelpath = get_file_srcdir_relative_path (floc->file);
       if (srcrelpath != NULL)
 	{
-	  fprintf (state_file, "\n(!srcfileloc ");
+	  write_open_paren ("srcfileloc");
 	  write_state_a_string (srcrelpath);
 	}
       else
 	{
-	  fprintf (state_file, "\n(!fileloc ");
+	  write_open_paren ("fileloc");
 	  write_state_a_string (get_input_file_name (floc->file));
 	}
       fprintf (state_file, " %d", floc->line);
-      fprintf (state_file, ")\n");
+      write_close_paren (); /* "(!srcfileloc" or "(!fileloc" */
     }
   else
     fprintf (state_file, "nil ");
@@ -586,10 +637,11 @@  write_state_fields (pair_p fields)
 {
   int nbfields = pair_list_length (fields);
   int nbpairs = 0;
-  fprintf (state_file, "\n(!fields %d ", nbfields);
+  write_open_paren ("fields");
+  fprintf (state_file, "%d ", nbfields);
   nbpairs = write_state_pair_list (fields);
   gcc_assert (nbpairs == nbfields);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!fields */
 }
 
 /* Write a null-terminated string in our lexical convention, very
@@ -599,6 +651,8 @@  write_state_a_string (const char *s)
 {
   char c;
 
+  write_any_indent (-1);
+
   fputs (" \"", state_file);
   for (; *s != 0; s++)
     {
@@ -646,6 +700,7 @@  write_state_a_string (const char *s)
 static void
 write_state_string_option (options_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "string ");
   if (current->info.string != NULL)
     write_state_a_string (current->info.string);
@@ -656,6 +711,7 @@  write_state_string_option (options_p current)
 static void
 write_state_type_option (options_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "type ");
   write_state_type (current->info.type);
 }
@@ -663,24 +719,32 @@  write_state_type_option (options_p current)
 static void
 write_state_nested_option (options_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "nested ");
   write_state_type (current->info.nested->type);
   if (current->info.nested->convert_from != NULL)
     write_state_a_string (current->info.nested->convert_from);
   else
-    fprintf (state_file, " nil ");
+    {
+      write_any_indent (-1);
+      fprintf (state_file, " nil ");
+    }
 
   if (current->info.nested->convert_to != NULL)
     write_state_a_string (current->info.nested->convert_to);
   else
-    fprintf (state_file, " nil ");
+    {
+      write_any_indent (-1);
+      fprintf (state_file, " nil ");
+    }
 }
 
 static void
 write_state_option (options_p current)
 {
-  fprintf (state_file, "\n(!option ");
+  write_open_paren ("option");
 
+  write_any_indent (0);
   if (current->name != NULL)
     fprintf (state_file, "%s ", current->name);
   else
@@ -701,7 +765,7 @@  write_state_option (options_p current)
       fatal ("Option tag unknown");
     }
 
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!option */
 }
 
 
@@ -714,14 +778,15 @@  write_state_options (options_p opt)
 
   if (opt == NULL)
     {
-      fprintf (state_file, "nil ");
+       write_any_indent (0);
+       fprintf (state_file, "nil ");
       return;
     }
 
-  fprintf (state_file, "\n(!options ");
+  write_open_paren ("options");
   for (current = opt; current != NULL; current = current->next)
       write_state_option (current);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!options */
 }
 
 
@@ -729,16 +794,17 @@  write_state_options (options_p opt)
 static void
 write_state_lang_bitmap (lang_bitmap bitmap)
 {
-  fprintf (state_file, "%d ", (int) bitmap);
+   write_any_indent (0);
+   fprintf (state_file, "%d ", (int) bitmap);
 }
 
 /* Write version information.  */
 static void
 write_state_version (const char *version)
 {
-  fprintf (state_file, "\n(!version ");
+  write_open_paren ("version");
   write_state_a_string (version);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!version */
 }
 
 /* Common routine to write the common content of all types.  */
@@ -748,6 +814,7 @@  static void write_state_common_type_content (type_p current);
 static void
 write_state_scalar_type (type_p current)
 {
+  write_any_indent (0);
   if (current == &scalar_nonchar)
     fprintf (state_file, "scalar_nonchar ");
   else if (current == &scalar_char)
@@ -764,6 +831,7 @@  write_state_string_type (type_p current)
 {
   if (current == &string_type)
     {
+      write_any_indent (0);
       fprintf (state_file, "string ");
       write_state_common_type_content (current);
     }
@@ -777,13 +845,17 @@  write_state_undefined_type (type_p current)
 {
   DBGPRINTF ("undefined type @ %p #%d '%s'", (void *) current,
 	     current->state_number, current->u.s.tag);
+  write_any_indent (0);
   fprintf (state_file, "undefined ");
   gcc_assert (current->gc_used == GC_UNUSED);
   write_state_common_type_content (current);
   if (current->u.s.tag != NULL)
     write_state_a_string (current->u.s.tag);
   else
-    fprintf (state_file, "nil");
+    {
+      write_any_indent (0);
+      fprintf (state_file, "nil");
+    }
 
   write_state_fileloc (type_lineloc (current));
 }
@@ -795,12 +867,16 @@  write_state_struct_union_type (type_p current, const char *kindstr)
 {
   DBGPRINTF ("%s type @ %p #%d '%s'", kindstr, (void *) current,
 	     current->state_number, current->u.s.tag);
+  write_any_indent (0);
   fprintf (state_file, "%s ", kindstr);
   write_state_common_type_content (current);
   if (current->u.s.tag != NULL)
     write_state_a_string (current->u.s.tag);
   else
-    fprintf (state_file, "nil");
+    {
+      write_any_indent (0);
+      fprintf (state_file, "nil");
+    }
 
   write_state_fileloc (type_lineloc (current));
   write_state_fields (current->u.s.fields);
@@ -823,12 +899,16 @@  write_state_user_struct_type (type_p current)
 {
   DBGPRINTF ("user_struct type @ %p #%d '%s'", (void *) current,
 	     current->state_number, current->u.s.tag);
+  write_any_indent (0);
   fprintf (state_file, "user_struct ");
   write_state_common_type_content (current);
   if (current->u.s.tag != NULL)
     write_state_a_string (current->u.s.tag);
   else
-    fprintf (state_file, "nil");
+    {
+      write_any_indent (0);
+      fprintf (state_file, "nil");
+    }
   write_state_fileloc (type_lineloc (current));
   write_state_fields (current->u.s.fields);
 }
@@ -869,10 +949,11 @@  write_state_lang_struct_type (type_p current)
 	homoname = hty->u.s.tag;
       gcc_assert (strcmp (homoname, hty->u.s.tag) == 0);
     }
-  fprintf (state_file, "(!homotypes %d\n", nbhomontype);
+  write_open_paren ("homotypes");
+  fprintf (state_file, "%d", nbhomontype);
   for (hty = current->u.s.lang_struct; hty != NULL; hty = hty->next)
     write_state_type (hty);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!homotypes */
 }
 
 /* Write a parametrized structure GTY type.  */
@@ -881,6 +962,7 @@  write_state_param_struct_type (type_p current)
 {
   int i;
 
+  write_any_indent (0);
   fprintf (state_file, "param_struct ");
   write_state_common_type_content (current);
   write_state_type (current->u.param_struct.stru);
@@ -889,7 +971,10 @@  write_state_param_struct_type (type_p current)
       if (current->u.param_struct.param[i] != NULL)
 	write_state_type (current->u.param_struct.param[i]);
       else
+        {
+	write_any_indent (0);
 	fprintf (state_file, "nil ");
+        }
     }
   write_state_fileloc (&current->u.param_struct.line);
 }
@@ -898,6 +983,7 @@  write_state_param_struct_type (type_p current)
 static void
 write_state_pointer_type (type_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "pointer ");
   write_state_common_type_content (current);
   write_state_type (current->u.p);
@@ -907,13 +993,18 @@  write_state_pointer_type (type_p current)
 static void
 write_state_array_type (type_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "array ");
   write_state_common_type_content (current);
   if (current->u.a.len != NULL)
     write_state_a_string (current->u.a.len);
   else
-    fprintf (state_file, " nil");
+    {
+      write_any_indent (-1);
+      fprintf (state_file, " nil");
+    }
 
+  write_any_indent (-1);
   fprintf (state_file, " ");
   write_state_type (current->u.a.p);
 }
@@ -922,6 +1013,7 @@  write_state_array_type (type_p current)
 static void
 write_state_gc_used (enum gc_used_enum gus)
 {
+  write_any_indent (-1);
   switch (gus)
     {
     case GC_UNUSED:
@@ -946,6 +1038,7 @@  write_state_gc_used (enum gc_used_enum gus)
 static void
 write_state_common_type_content (type_p current)
 {
+  write_any_indent (0);
   fprintf (state_file, "%d ", current->state_number);
   /* We do not write the next type, because list of types are
      explicitly written.  However, lang_struct are special in that
@@ -961,16 +1054,20 @@  write_state_common_type_content (type_p current)
 static void
 write_state_type (type_p current)
 {
+  write_any_indent (0);
   if (current == NULL)
     {
       fprintf (state_file, "nil ");
       return;
     }
 
-  fprintf (state_file, "\n(!type ");
+  write_open_paren ("type");
 
   if (current->state_number > 0)
-    fprintf (state_file, "already_seen %d", current->state_number);
+    {
+      write_any_indent (0);
+      fprintf (state_file, "already_seen %d", current->state_number);
+    }
   else
     {
       state_written_type_count++;
@@ -1014,7 +1111,7 @@  write_state_type (type_p current)
 	}
     }
 
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!type */
 }
 
 
@@ -1024,11 +1121,12 @@  write_state_pair (pair_p current)
 {
   if (current == NULL)
     {
+      write_any_indent (0);
       fprintf (state_file, "nil)");
       return;
     }
 
-  fprintf (state_file, "\n(!pair ");
+  write_open_paren ("pair");
 
   if (current->name != NULL)
     write_state_a_string (current->name);
@@ -1038,8 +1136,7 @@  write_state_pair (pair_p current)
   write_state_type (current->type);
   write_state_fileloc (&(current->line));
   write_state_options (current->opt);
-
-  fprintf (state_file, ")");
+  write_close_paren (); /* (!pair */
 }
 
 /* Write a pair list and return the number of pairs written.  */
@@ -1069,10 +1166,11 @@  write_state_typedefs (void)
 {
   int nbtypedefs = pair_list_length (typedefs);
   int nbpairs = 0;
-  fprintf (state_file, "\n(!typedefs %d\n", nbtypedefs);
+  write_open_paren ("typedefs");
+  fprintf (state_file, "%d", nbtypedefs);
   nbpairs = write_state_pair_list (typedefs);
   gcc_assert (nbpairs == nbtypedefs);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!typedefs */
   if (verbosity_level >= 2)
     printf ("%s wrote %d typedefs\n", progname, nbtypedefs);
 }
@@ -1087,12 +1185,16 @@  write_state_structures (void)
   for (current = structures; current != NULL; current = current->next)
     nbstruct++;
 
-  fprintf (state_file, "\n(!structures %d\n", nbstruct);
+  write_open_paren ("structures");
+  fprintf (state_file, "%d", nbstruct);
 
   for (current = structures; current != NULL; current = current->next)
-    write_state_type (current);
+    {
+      write_new_line ();
+      write_state_type (current);
+    }
 
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!structures */
   if (verbosity_level >= 2)
     printf ("%s wrote %d structures in state\n", progname, nbstruct);
 }
@@ -1107,12 +1209,13 @@  write_state_param_structs (void)
   for (current = param_structs; current != NULL; current = current->next)
     nbparamstruct++;
 
-  fprintf (state_file, "\n(!param_structs %d\n", nbparamstruct);
+  write_open_paren ("param_structs");
+  fprintf (state_file, "%d", nbparamstruct);
 
   for (current = param_structs; current != NULL; current = current->next)
     write_state_type (current);
 
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!param_structs */
 }
 
 /* Write our variables.  */
@@ -1121,10 +1224,11 @@  write_state_variables (void)
 {
   int nbvars = pair_list_length (variables);
   int nbpairs = 0;
-  fprintf (state_file, "\n(!variables %d\n", nbvars);
+  write_open_paren ("variables");
+  fprintf (state_file, "%d", nbvars);
   nbpairs = write_state_pair_list (variables);
   gcc_assert (nbpairs == nbvars);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!variables */
   if (verbosity_level >= 2)
     printf ("%s wrote %d variables.\n", progname, nbvars);
 }
@@ -1134,9 +1238,9 @@  write_state_variables (void)
 static void
 write_state_srcdir (void)
 {
-  fprintf (state_file, "\n(!srcdir ");
+  write_open_paren ("srcdir");
   write_state_a_string (srcdir);
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!srcdir */
 }
 
 /* Count and write the list of our files.  */
@@ -1145,7 +1249,8 @@  write_state_files_list (void)
 {
   int i = 0;
   /* Write the list of files with their lang_bitmap.  */
-  fprintf (state_file, "\n(!fileslist %d\n", (int) num_gt_files);
+  write_open_paren ("fileslist");
+  fprintf (state_file, "%d", (int) num_gt_files);
   for (i = 0; i < (int) num_gt_files; i++)
     {
       const char *cursrcrelpath = NULL;
@@ -1155,17 +1260,19 @@  write_state_files_list (void)
       cursrcrelpath = get_file_srcdir_relative_path (curfil);
       if (cursrcrelpath)
 	{
-	  fprintf (state_file, "(!srcfile %d ", get_lang_bitmap (curfil));
+	  write_open_paren ("srcfile");
+          fprintf (state_file, "%d ", get_lang_bitmap (curfil));
 	  write_state_a_string (cursrcrelpath);
 	}
       else
 	{
-	  fprintf (state_file, "(!file %d ", get_lang_bitmap (curfil));
+	  write_open_paren ("file");
+	  fprintf (state_file, "%d ", get_lang_bitmap (curfil));
 	  write_state_a_string (get_input_file_name (curfil));
 	}
-      fprintf (state_file, ")\n");
+      write_close_paren (); /* (!srcfile or (!file */
     }
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!fileslist */
 }
 
 /* Write the list of GCC front-end languages.  */
@@ -1173,7 +1280,8 @@  static void
 write_state_languages (void)
 {
   int i = 0;
-  fprintf (state_file, "\n(!languages %d", (int) num_lang_dirs);
+  write_open_paren ("languages");
+  fprintf (state_file, "%d", (int) num_lang_dirs);
   for (i = 0; i < (int) num_lang_dirs; i++)
     {
       /* Languages names are identifiers, we expect only letters or
@@ -1181,7 +1289,7 @@  write_state_languages (void)
          valid language name, but cp is valid.  */
       fprintf (state_file, " %s", lang_dir_names[i]);
     }
-  fprintf (state_file, ")\n");
+  write_close_paren (); /* (!languages */
 }
 
 /* Write the trailer.  */