We are in trouble

[Akim Demaille]

Well, I had a look at the way Bison works currently, and we are in
troubles if we really want to clean up the memory management.

The main problem is still that the architecture is a pure nightmare:
you never know when you're done with the reading of files, you can
never be sure something will not be used later and so on.

Ideally, there is just no reason to consume memory before outputting
files such as .vcg and .output, and yes, it means that we have too
many obstacks currently.  Unfortunately some people complain, and it
is certainly related to this.

Given the amount of work done on Bison Next Generation, I am tempted
to completely drop any hope of cleaning up 1.30, it's a waste of our
time since we will certainly introduce zillions of conflicts.

Therefore, let's fix what can be done _without_ massaging Bison too
furiously.  Let's release 1.30, and focus on making Bison NG viable as
soon as possible.  This means for instance that we can delay C++, but
settle the right grounds now.  We must focus on making it usable.

FYI, here is a patch I tried to make to avoid consuming memory when
--verbose is not used.  Unfortunately it introduces problems I feel
too tired to be able to solve.  The test suite won't pass with it.

Thoughts?

Index: ChangeLog
from  Akim Demaille  <address@hidden>
        * src/conflicts.c (initialize_conflicts): Rename as...
        (solve_conflicts): this.
        * src/files.c (compute_output_file_names, close_files): New.
        (output_files): Adjust.
        * src/main.c (main): Adjust.

Index: src/conflicts.c
===================================================================
RCS file: /cvsroot/bison/bison/src/conflicts.c,v
retrieving revision 1.22.2.1
diff -u -u -r1.22.2.1 conflicts.c
--- src/conflicts.c 2001/09/22 18:13:24 1.22.2.1
+++ src/conflicts.c 2001/09/22 19:07:45
@@ -30,12 +30,12 @@
 #include "LR0.h"
 
 int any_conflicts = 0;
-errs **err_table;
+errs **err_table = NULL;
 int expected_conflicts;
-static char *conflicts;
+static char *conflicts = NULL;
 
-static unsigned *shiftset;
-static unsigned *lookaheadset;
+static unsigned *shiftset = NULL;
+static unsigned *lookaheadset = NULL;
 static int src_total;
 static int rrc_total;
 static int src_count;
@@ -264,7 +264,7 @@
 }
 
 void
-initialize_conflicts (void)
+solve_conflicts (void)
 {
   int i;
 
@@ -458,7 +458,7 @@
 `---------------------------------------------*/
 
 void
-print_conflicts (void)
+print_conflicts (FILE *out)
 {
   int i;
 
@@ -478,9 +478,8 @@
 
          if (verbose_flag)
            {
-             obstack_fgrow1 (&output_obstack, _("State %d contains"), i);
-             obstack_sgrow (&output_obstack,
-                            conflict_report (src_count, rrc_count));
+             fprintf (out, _("State %d contains"), i);
+             fputs (conflict_report (src_count, rrc_count), out);
            }
        }
     }
Index: src/conflicts.h
===================================================================
RCS file: /cvsroot/bison/bison/src/conflicts.h,v
retrieving revision 1.3.2.1
diff -u -u -r1.3.2.1 conflicts.h
--- src/conflicts.h 2001/09/22 18:13:24 1.3.2.1
+++ src/conflicts.h 2001/09/22 19:07:45
@@ -22,8 +22,8 @@
 # define CONFLICTS_H_
 # include "state.h"
 
-void initialize_conflicts PARAMS ((void));
-void print_conflicts PARAMS ((void));
+void solve_conflicts PARAMS ((void));
+void print_conflicts PARAMS ((FILE *out));
 void print_reductions PARAMS ((int));
 void free_conflicts PARAMS ((void));
 
Index: src/files.c
===================================================================
RCS file: /cvsroot/bison/bison/src/files.c,v
retrieving revision 1.51.2.2
diff -u -u -r1.51.2.2 files.c
--- src/files.c 2001/09/17 23:00:01 1.51.2.2
+++ src/files.c 2001/09/22 19:07:45
@@ -39,6 +39,7 @@
 char *spec_outfile = NULL;     /* for -o. */
 char *spec_file_prefix = NULL; /* for -b. */
 char *spec_name_prefix = NULL; /* for -p. */
+char *spec_verbose_file = NULL;   /* for --verbose. */
 char *spec_graph_file = NULL;   /* for -g. */
 char *spec_defines_file = NULL; /* for --defines. */
 
@@ -387,6 +388,32 @@
   }
 }
 
+/*-------------------------------------------------------.
+| Close the open files, compute the output files names.  |
+`-------------------------------------------------------*/
+
+void
+compute_output_file_names (void)
+{
+  compute_base_names ();
+
+  /* It the defines filename if not given, we create it.  */
+  if (!spec_defines_file)
+    spec_defines_file = stringappend (base_name, header_extension);
+
+  /* It the graph filename if not given, we create it.  */
+  if (!spec_graph_file)
+    spec_graph_file = stringappend (short_base_name, ".vcg");
+
+  spec_verbose_file = stringappend (short_base_name, EXT_OUTPUT);
+
+  attrsfile = stringappend (short_base_name, EXT_STYPE_H);
+#ifndef MSDOS
+  stringappend (attrsfile, header_extension);
+#endif /* MSDOS */
+
+}
+
 /*-----------------------------------------------------------------.
 | Open the input file.  Look for the skeletons.  Find the names of |
 | the output files.  Prepare the obstacks.                         |
@@ -409,30 +436,23 @@
 
 
 
-/*-----------------------------------------------------.
-| Close the open files, produce all the output files.  |
-`-----------------------------------------------------*/
+/*-----------------------.
+| Close the open file..  |
+`-----------------------*/
 
 void
-output_files (void)
+close_files (void)
 {
   xfclose (finput);
+}
 
-  compute_base_names ();
+/*---------------------------.
+| Produce the output files.  |
+`---------------------------*/
 
-  /* It the defines filename if not given, we create it.  */
-  if (!spec_defines_file)
-    spec_defines_file = stringappend (base_name, header_extension);
-  
-  /* It the graph filename if not given, we create it.  */
-  if (!spec_graph_file)
-    spec_graph_file = stringappend (short_base_name, ".vcg");
-  
-  attrsfile = stringappend (short_base_name, EXT_STYPE_H);
-#ifndef MSDOS
-  stringappend (attrsfile, header_extension);
-#endif /* MSDOS */
-
+void
+output_files (void)
+{
   /* Output the main file.  */
   if (spec_outfile)
     obstack_save (&table_obstack, spec_outfile);
@@ -460,12 +480,6 @@
 #endif /* MSDOS */
       obstack_save (&guard_obstack, temp_name);
     }
-
-  if (verbose_flag)
-    /* We used to use just .out if spec_name_prefix (-p) was used, but
-       that conflicts with Posix.  */
-    obstack_save (&output_obstack,
-                 stringappend (short_base_name, EXT_OUTPUT));
 
   if (graph_flag)
     obstack_save (&graph_obstack, spec_graph_file);
Index: src/files.h
===================================================================
RCS file: /cvsroot/bison/bison/src/files.h,v
retrieving revision 1.18.2.2
diff -u -u -r1.18.2.2 files.h
--- src/files.h 2001/09/17 23:00:01 1.18.2.2
+++ src/files.h 2001/09/22 19:07:45
@@ -33,9 +33,12 @@
 /* File name pfx specified with -b, or 0 if no -b.  */
 extern char *spec_file_prefix;
 
+/* --verbose. */
+extern char *spec_verbose_file;
+
 /* File name specified for the output VCG graph.  */
 extern char *spec_graph_file;
- 
+
 /* File name specified with --defines.  */
 extern char *spec_defines_file;
 
@@ -69,7 +72,9 @@
 extern char *attrsfile;
 
 void open_files PARAMS((void));
+void close_files PARAMS((void));
 
+void compute_output_file_names PARAMS((void));
 void output_files PARAMS((void));
 
 FILE *xfopen PARAMS ((const char *name, const char *mode));
Index: src/main.c
===================================================================
RCS file: /cvsroot/bison/bison/src/main.c,v
retrieving revision 1.30.2.1
diff -u -u -r1.30.2.1 main.c
--- src/main.c 2001/09/22 18:13:24 1.30.2.1
+++ src/main.c 2001/09/22 19:07:45
@@ -79,8 +79,16 @@
      lookahead is not enough to disambiguate the parsing.  In file
      conflicts.  Also resolve s/r conflicts based on precedence
      declarations.  */
-  initialize_conflicts ();
+  solve_conflicts ();
 
+  /* Output file names. */
+  compute_output_file_names ();
+
+  /* Stop if there were errors, to avoid trashing previous output
+     files.  */
+  if (complain_message_count)
+    exit (1);
+
   /* Print information about results, if requested.  */
   print_results ();
 
@@ -90,12 +98,14 @@
   /* Output the tables and the parser to ftable.  In file output.  */
   output ();
 
+  /* Close the input files. */
+  close_files ();
+
   free_conflicts ();
   free_nullable ();
   free_derives ();
 
-  if (!complain_message_count)
-    output_files ();
+  output_files ();
 
   exit (complain_message_count ? 1 : 0);
 }
Index: src/print.c
===================================================================
RCS file: /cvsroot/bison/bison/src/print.c,v
retrieving revision 1.17.2.1
diff -u -u -r1.17.2.1 print.c
--- src/print.c 2001/09/22 17:43:26 1.17.2.1
+++ src/print.c 2001/09/22 19:07:45
@@ -35,7 +35,7 @@
 static void
 print_token (int extnum, int token)
 {
-  obstack_fgrow2 (&output_obstack, _(" type %d is %s\n"), extnum, tags[token]);
+  fprintf (out, _(" type %d is %s\n"), extnum, tags[token]);
 }
 #endif
 
@@ -45,7 +45,7 @@
 \================================*/
 
 static void
-print_core (int state)
+print_core (FILE *out, int state)
 {
   int i;
   int k;
@@ -68,30 +68,30 @@
        sp++;
 
       rule = -(*sp);
-      obstack_fgrow1 (&output_obstack, "    %s  ->  ", tags[rlhs[rule]]);
+      fprintf (out, "    %s  ->  ", tags[rlhs[rule]]);
 
       for (sp = ritem + rrhs[rule]; sp < sp1; sp++)
        {
-         obstack_fgrow1 (&output_obstack, "%s ", tags[*sp]);
+         fprintf (out, "%s ", tags[*sp]);
        }
 
-      obstack_1grow (&output_obstack, '.');
+      fputc ('.', out);
 
       while (*sp > 0)
        {
-         obstack_fgrow1 (&output_obstack, " %s", tags[*sp]);
+         fprintf (out, " %s", tags[*sp]);
          sp++;
        }
 
-      obstack_fgrow1 (&output_obstack, _("   (rule %d)"), rule);
-      obstack_1grow (&output_obstack, '\n');
+      fprintf (out, _("   (rule %d)"), rule);
+      fputc ('\n', out);
     }
 
-  obstack_1grow (&output_obstack, '\n');
+  fputc ('\n', out);
 }
 
 static void
-print_actions (int state)
+print_actions (FILE *out, int state)
 {
   int i;
   int k;
@@ -109,9 +109,9 @@
   if (!shiftp && !redp)
     {
       if (final_state == state)
-       obstack_sgrow (&output_obstack, _("    $default\taccept\n"));
+       fprintf (out, _("    $default\taccept\n"));
       else
-       obstack_sgrow (&output_obstack, _("    NO ACTIONS\n"));
+       fprintf (out, _("    NO ACTIONS\n"));
       return;
     }
 
@@ -129,16 +129,16 @@
          if (ISVAR (symbol))
            break;
          if (symbol == 0)      /* I.e. strcmp(tags[symbol],"$")==0 */
-           obstack_fgrow1 (&output_obstack,
-                           _("    $   \tgo to state %d\n"), state1);
+           fprintf (out,
+                    _("    $   \tgo to state %d\n"), state1);
          else
-           obstack_fgrow2 (&output_obstack,
-                           _("    %-4s\tshift, and go to state %d\n"),
-                           tags[symbol], state1);
+           fprintf (out,
+                    _("    %-4s\tshift, and go to state %d\n"),
+                    tags[symbol], state1);
        }
 
       if (i > 0)
-       obstack_1grow (&output_obstack, '\n');
+       fputc ('\n', out);
     }
   else
     {
@@ -157,21 +157,20 @@
          if (!errp->errs[j])
            continue;
          symbol = errp->errs[j];
-         obstack_fgrow1 (&output_obstack, _("    %-4s\terror 
(nonassociative)\n"),
+         fprintf (out, _("    %-4s\terror (nonassociative)\n"),
                   tags[symbol]);
        }
 
       if (j > 0)
-       obstack_1grow (&output_obstack, '\n');
+       fputc ('\n', out);
     }
 
   if (consistent[state] && redp)
     {
       rule = redp->rules[0];
       symbol = rlhs[rule];
-      obstack_fgrow2 (&output_obstack,
-                     _("    $default\treduce using rule %d (%s)\n\n"),
-                     rule, tags[symbol]);
+      fprintf (out, _("    $default\treduce using rule %d (%s)\n\n"),
+              rule, tags[symbol]);
     }
   else if (redp)
     {
@@ -186,42 +185,41 @@
            continue;
          state1 = shiftp->shifts[i];
          symbol = accessing_symbol[state1];
-         obstack_fgrow2 (&output_obstack,
-                         _("    %-4s\tgo to state %d\n"),
-                         tags[symbol], state1);
+         fprintf (out, _("    %-4s\tgo to state %d\n"),
+                  tags[symbol], state1);
        }
 
-      obstack_1grow (&output_obstack, '\n');
+      fputc ('\n', out);
     }
 }
 
 static void
-print_state (int state)
+print_state (FILE *out, int state)
 {
-  obstack_sgrow (&output_obstack, "\n\n");
-  obstack_fgrow1 (&output_obstack, _("state %d"), state);
-  obstack_sgrow (&output_obstack, "\n\n");
-  print_core (state);
-  print_actions (state);
+  fputs ("\n\n", out);
+  fprintf (out, _("state %d"), state);
+  fputs ("\n\n", out);
+  print_core (out, state);
+  print_actions (out, state);
 }
 
 /*-----------------------------------------.
 | Print information on the whole grammar.  |
 `-----------------------------------------*/
 
-#define END_TEST(End)                                          \
-do {                                                           \
-  if (column + strlen(buffer) > (End))                         \
-    {                                                          \
-      obstack_fgrow1 (&output_obstack, "%s\n   ", buffer);     \
-      column = 3;                                              \
-      buffer[0] = 0;                                           \
-    }                                                          \
+#define END_TEST(End)                          \
+do {                                           \
+  if (column + strlen(buffer) > (End))         \
+    {                                          \
+      fprintf (out, "%s\n   ", buffer);                \
+      column = 3;                              \
+      buffer[0] = 0;                           \
+    }                                          \
 } while (0)
 
 
 static void
-print_grammar (void)
+print_grammar (FILE *out)
 {
   int i, j;
   short *rule;
@@ -229,37 +227,30 @@
   int column = 0;
 
   /* rule # : LHS -> RHS */
-  obstack_1grow (&output_obstack, '\n');
-  obstack_sgrow (&output_obstack, _("Grammar"));
-  obstack_1grow (&output_obstack, '\n');
+  fprintf (out, "\n%s\n", _("Grammar"));
   for (i = 1; i <= nrules; i++)
     /* Don't print rules disabled in reduce_grammar_tables.  */
     if (rlhs[i] >= 0)
       {
-       obstack_fgrow2 (&output_obstack,
-                       _("rule %-4d %s ->"), i, tags[rlhs[i]]);
+       fprintf (out, _("rule %-4d %s ->"), i, tags[rlhs[i]]);
        rule = &ritem[rrhs[i]];
        if (*rule > 0)
          while (*rule > 0)
-           obstack_fgrow1 (&output_obstack, " %s", tags[*rule++]);
+           fprintf (out, " %s", tags[*rule++]);
        else
-         obstack_sgrow (&output_obstack, _("           /* empty */"));
-       obstack_1grow (&output_obstack, '\n');
+         fprintf (out, "               /* %s */\n", _("empty"));
       }
 
   /* TERMINAL (type #) : rule #s terminal is on RHS */
-  obstack_sgrow (&output_obstack, "\n");
-  obstack_sgrow (&output_obstack,
-                _("Terminals, with rules where they appear"));
-  obstack_sgrow (&output_obstack, "\n\n");
-  obstack_fgrow1 (&output_obstack, "%s (-1)\n", tags[0]);
+  fprintf (out, "\n%s\n\n", _("Terminals, with rules where they appear"));
+  fprintf (out, "%s (-1)\n", tags[0]);
 
   for (i = 0; i <= max_user_token_number; i++)
     if (token_translations[i] != 2)
       {
        buffer[0] = 0;
        column = strlen (tags[token_translations[i]]);
-       obstack_sgrow (&output_obstack, tags[token_translations[i]]);
+       fputs (tags[token_translations[i]], out);
        END_TEST (50);
        sprintf (buffer, " (%d)", i);
 
@@ -271,13 +262,11 @@
                sprintf (buffer + strlen (buffer), " %d", j);
                break;
              }
-       obstack_fgrow1 (&output_obstack, "%s\n", buffer);
+       fprintf (out, "%s\n", buffer);
       }
 
-  obstack_sgrow (&output_obstack, "\n");
-  obstack_sgrow (&output_obstack,
-                _("Nonterminals, with rules where they appear"));
-  obstack_sgrow (&output_obstack, "\n\n");
+  fprintf (out, "\n%s\n\n",
+          _("Nonterminals, with rules where they appear"));
   for (i = ntokens; i <= nsyms - 1; i++)
     {
       int left_count = 0, right_count = 0;
@@ -295,7 +284,7 @@
        }
 
       buffer[0] = 0;
-      obstack_sgrow (&output_obstack, tags[i]);
+      fputs (tags[i], out);
       column = strlen (tags[i]);
       sprintf (buffer, " (%d)", i);
       END_TEST (0);
@@ -330,22 +319,33 @@
                  }
            }
        }
-      obstack_fgrow1 (&output_obstack, "%s\n", buffer);
+      fprintf (out, "%s\n", buffer);
     }
 }
 
 void
 print_results (void)
 {
-  int i;
+  if (verbose_flag)
+    {
+      int i;
 
-  if (any_conflicts)
-    print_conflicts ();
+      /* We used to use just .out if spec_name_prefix (-p) was used, but
+        that conflicts with Posix.  */
+      FILE *out = xfopen (spec_verbose_file, "w");
 
-  if (verbose_flag)
-    print_grammar ();
+      size_t size = obstack_object_size (&output_obstack);
+      fwrite (obstack_finish (&output_obstack), 1, size, out);
 
-  if (verbose_flag)
-    for (i = 0; i < nstates; i++)
-      print_state (i);
+      if (any_conflicts)
+       print_conflicts (out);
+
+      print_grammar (out);
+
+      for (i = 0; i < nstates; i++)
+       print_state (out, i);
+
+      xfclose (out);
+    }
+  obstack_free (&output_obstack, NULL);
 }