[gfortran PR32147] Make module files more predictable

[gfortran PR32147] Make module files more predictable

[Tobias Schlüter]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]


The PR is concerned with the issue that our module files change more or 
less randomly if one changes the sources only slightly.  This is because 
we build up our symbol trees balancing them randomly, and then traverse 
them top-down.

This patch changes the order to always be left-to-right.  This is fairly 
trivial, and amounts to replacing
{
    walk_left ();
    walk_right ();
    do_something ();
}
with
{
    walk_left ();
    do_something ();
    walk_right ();
}
keeping in mind that do_something() may return early, which made a few 
safeguards necessary.

Unfortunately the bug reporter didn't have time to verify that it indeed 
fixes his problem, but due to the trivial nature of this patch, I think 
it's worthy having anyway.  It also gives us cleaner -fdump-parse-tree 
output after all! :-)

Built and tested on i386-darwin.  Ok?

Cheers,
- Tobi

[-- Attachment #2: pr32147.diff.txt --]
[-- Type: text/plain, Size: 3651 bytes --]

2007-10-05  Tobias Schlüter  <tobi@gcc.gnu.org>

	PR fortran/32147
	* module.c (write_symbol): Fix whitespace.
	(write_symbol0): Walk symtree from left-to-right instead
	breadth-first.
	(write_symbol1): Similarly change walk of pointer info tree.
	(write_module): Insert linebreak.
	* symbol.c (gfc_traverse_symtree): Change to left-to-right order.
	(traverse_ns): Likewise.

diff -r 06395d4340f4 gcc/fortran/module.c
--- a/gcc/fortran/module.c	Thu Oct 04 23:16:32 2007 +0200
+++ b/gcc/fortran/module.c	Fri Oct 05 19:30:30 2007 +0200
@@ -3880,7 +3880,7 @@ static void
 static void
 write_symbol (int n, gfc_symbol *sym)
 {
-   const char *label;
+  const char *label;
 
   if (sym->attr.flavor == FL_UNKNOWN || sym->attr.flavor == FL_LABEL)
     gfc_internal_error ("write_symbol(): bad module symbol '%s'", sym->name);
@@ -3913,12 +3913,12 @@ write_symbol0 (gfc_symtree *st)
 {
   gfc_symbol *sym;
   pointer_info *p;
+  bool dont_write = false;
 
   if (st == NULL)
     return;
 
   write_symbol0 (st->left);
-  write_symbol0 (st->right);
 
   sym = st->n.sym;
   if (sym->module == NULL)
@@ -3926,20 +3926,25 @@ write_symbol0 (gfc_symtree *st)
 
   if (sym->attr.flavor == FL_PROCEDURE && sym->attr.generic
       && !sym->attr.subroutine && !sym->attr.function)
-    return;
+    dont_write = true;
 
   if (!gfc_check_access (sym->attr.access, sym->ns->default_access))
-    return;
-
-  p = get_pointer (sym);
-  if (p->type == P_UNKNOWN)
-    p->type = P_SYMBOL;
-
-  if (p->u.wsym.state == WRITTEN)
-    return;
-
-  write_symbol (p->integer, sym);
-  p->u.wsym.state = WRITTEN;
+    dont_write = true;
+
+  if (!dont_write)
+    {
+      p = get_pointer (sym);
+      if (p->type == P_UNKNOWN)
+	p->type = P_SYMBOL;
+
+      if (p->u.wsym.state != WRITTEN)
+	{
+	  write_symbol (p->integer, sym);
+	  p->u.wsym.state = WRITTEN;
+	}
+    }
+
+  write_symbol0 (st->right);
 }
 
 
@@ -3953,22 +3958,22 @@ static int
 static int
 write_symbol1 (pointer_info *p)
 {
-
-  if (p == NULL)
+  int result;
+
+  if (!p)
     return 0;
 
-  if (write_symbol1 (p->left))
-    return 1;
-  if (write_symbol1 (p->right))
-    return 1;
-
-  if (p->type != P_SYMBOL || p->u.wsym.state != NEEDS_WRITE)
-    return 0;
-
-  p->u.wsym.state = WRITTEN;
-  write_symbol (p->integer, p->u.wsym.sym);
-
-  return 1;
+  result = write_symbol1 (p->left);
+
+  if (!(p->type != P_SYMBOL || p->u.wsym.state != NEEDS_WRITE))
+    {
+      p->u.wsym.state = WRITTEN;
+      write_symbol (p->integer, p->u.wsym.sym);
+      result = 1;
+    }
+
+  result |= write_symbol1 (p->right);
+  return result;
 }
 
 
@@ -4103,7 +4108,8 @@ write_module (void)
   mio_lparen ();
 
   write_symbol0 (gfc_current_ns->sym_root);
-  while (write_symbol1 (pi_root));
+  while (write_symbol1 (pi_root))
+    /* Nothing.  */;
 
   mio_rparen ();
 
diff -r 06395d4340f4 gcc/fortran/symbol.c
--- a/gcc/fortran/symbol.c	Thu Oct 04 23:16:32 2007 +0200
+++ b/gcc/fortran/symbol.c	Fri Oct 05 19:30:30 2007 +0200
@@ -2927,13 +2927,12 @@ void
 void
 gfc_traverse_symtree (gfc_symtree *st, void (*func) (gfc_symtree *))
 {
-  if (st != NULL)
-    {
-      (*func) (st);
-
-      gfc_traverse_symtree (st->left, func);
-      gfc_traverse_symtree (st->right, func);
-    }
+  if (!st)
+    return;
+
+  gfc_traverse_symtree (st->left, func);
+  (*func) (st);
+  gfc_traverse_symtree (st->right, func);
 }
 
 
@@ -2945,12 +2944,13 @@ traverse_ns (gfc_symtree *st, void (*fun
 
   if (st == NULL)
     return;
+
+  traverse_ns (st->left, func);
 
   if (st->n.sym->mark == 0)
     (*func) (st->n.sym);
   st->n.sym->mark = 1;
 
-  traverse_ns (st->left, func);
   traverse_ns (st->right, func);
 }
 

Re: [gfortran PR32147] Make module files more predictable

[Tobias Schlüter]

I forgot to say: I will of course adapt module_md5_1.f90 to the new md5 
value.

Cheers,
- Tobi

Re: [gfortran PR32147] Make module files more predictable

[FX Coudert]

> 2007-10-05  Tobias Schlüter  <tobi@gcc.gnu.org>
>
> 	PR fortran/32147
> 	* module.c (write_symbol): Fix whitespace.
> 	(write_symbol0): Walk symtree from left-to-right instead
> 	breadth-first.
> 	(write_symbol1): Similarly change walk of pointer info tree.
> 	(write_module): Insert linebreak.
> 	* symbol.c (gfc_traverse_symtree): Change to left-to-right order.
> 	(traverse_ns): Likewise.

Looks OK to me. Thanks!

FX