Patchwork [fortran] Factorize symbol refcounting.

login
register
mail settings
Submitter Mikael Morin
Date July 29, 2010, 2:37 p.m.
Message ID <4C519217.4060002@sfr.fr>
Download mbox | patch
Permalink /patch/60269/
State New
Headers show

Comments

Mikael Morin - July 29, 2010, 2:37 p.m.
Hello,

this takes the symbol reference count management code from free_sym_tree 
into its own function so that it can be shared.

I have made the gfc_internal_error (in case reference count becomes 
negative) a gcc_assert as the error message was not very 
informative/useful from a user point of view.

OK for trunk when regression test finishes ?

Mikael
2010-07-29  Mikael Morin  <mikael@gcc.gnu.org>

	* gfortran.h (gfc_release_symbol): New prototype.
	* symbol.c (gfc_release_symbol): New. Code taken from free_sym_tree.
	(gfc_undo_symbols, free_sym_tree, gfc_free_finalizer):
	Use gfc_release_symbol.
	* parse.c (gfc_fixup_sibling_symbols): Ditto.
	* resolve.c (resolve_symbol): Ditto.
Daniel Kraft - July 29, 2010, 2:58 p.m.
Mikael Morin wrote:
> Hello,
> 
> this takes the symbol reference count management code from free_sym_tree 
> into its own function so that it can be shared.
> 
> I have made the gfc_internal_error (in case reference count becomes 
> negative) a gcc_assert as the error message was not very 
> informative/useful from a user point of view.
> 
> OK for trunk when regression test finishes ?

Ok, thanks for the patch!

Just a style comment you may want to consider:

+/* Decrease the reference counter and free memory when we reach zero.  */
+void
+gfc_release_symbol (gfc_symbol *sym)

I'm not sure this is "official", but my impression is that common style 
is to include a blank line between comment and function.

Yours,
Daniel

Patch

diff --git a/gfortran.h b/gfortran.h
index d35a040..d623d0d 100644
--- a/gfortran.h
+++ b/gfortran.h
@@ -2523,6 +2523,7 @@  gfc_symtree *gfc_get_unique_symtree (gfc_namespace *);
 gfc_user_op *gfc_get_uop (const char *);
 gfc_user_op *gfc_find_uop (const char *, gfc_namespace *);
 void gfc_free_symbol (gfc_symbol *);
+void gfc_release_symbol (gfc_symbol *);
 gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *);
 gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *);
 int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **);
diff --git a/parse.c b/parse.c
index 1575b2b..989d644 100644
--- a/parse.c
+++ b/parse.c
@@ -3792,10 +3792,7 @@  gfc_fixup_sibling_symbols (gfc_symbol *sym, gfc_namespace *siblings)
 	  st->n.sym = sym;
 	  sym->refs++;
 
-	  /* Free the old (local) symbol.  */
-	  old_sym->refs--;
-	  if (old_sym->refs == 0)
-	    gfc_free_symbol (old_sym);
+	  gfc_release_symbol (old_sym);
 	}
 
 fixup_contained:
diff --git a/resolve.c b/resolve.c
index dab533d..d14de9e 100644
--- a/resolve.c
+++ b/resolve.c
@@ -11398,9 +11398,7 @@  resolve_symbol (gfc_symbol *sym)
 	    {
 	      this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
 					       sym->name);
-	      sym->refs--;
-	      if (!sym->refs)
-		gfc_free_symbol (sym);
+	      gfc_release_symbol (sym);
 	      symtree->n.sym->refs++;
 	      this_symtree->n.sym = symtree->n.sym;
 	      return;
diff --git a/symbol.c b/symbol.c
index 18f7b25..e713cd8 100644
--- a/symbol.c
+++ b/symbol.c
@@ -2502,6 +2502,31 @@  gfc_free_symbol (gfc_symbol *sym)
 }
 
 
+/* Decrease the reference counter and free memory when we reach zero.  */
+void
+gfc_release_symbol (gfc_symbol *sym)
+{
+  if (sym == NULL)
+    return;
+
+  if (sym->formal_ns != NULL && sym->refs == 2)
+    {
+      /* As formal_ns contains a reference to sym, delete formal_ns just
+	 before the deletion of sym.  */
+      gfc_namespace *ns = sym->formal_ns;
+      sym->formal_ns = NULL;
+      gfc_free_namespace (ns);
+    }
+
+  sym->refs--;
+  if (sym->refs > 0)
+    return;
+
+  gcc_assert (sym->refs == 0);
+  gfc_free_symbol (sym);
+}
+
+
 /* Allocate and initialize a new symbol node.  */
 
 gfc_symbol *
@@ -2893,11 +2918,7 @@  gfc_undo_symbols (void)
 
 	  gfc_delete_symtree (&p->ns->sym_root, p->name);
 
-	  p->refs--;
-	  if (p->refs < 0)
-	    gfc_internal_error ("gfc_undo_symbols(): Negative refs");
-	  if (p->refs == 0)
-	    gfc_free_symbol (p);
+	  gfc_release_symbol (p);
 	  continue;
 	}
 
@@ -3107,35 +3128,13 @@  free_uop_tree (gfc_symtree *uop_tree)
 static void
 free_sym_tree (gfc_symtree *sym_tree)
 {
-  gfc_namespace *ns;
-  gfc_symbol *sym;
-
   if (sym_tree == NULL)
     return;
 
   free_sym_tree (sym_tree->left);
   free_sym_tree (sym_tree->right);
 
-  sym = sym_tree->n.sym;
-
-  sym->refs--;
-  if (sym->refs < 0)
-    gfc_internal_error ("free_sym_tree(): Negative refs");
-
-  if (sym->formal_ns != NULL && sym->refs == 1)
-    {
-      /* As formal_ns contains a reference to sym, delete formal_ns just
-         before the deletion of sym.  */
-      ns = sym->formal_ns;
-      sym->formal_ns = NULL;
-      gfc_free_namespace (ns);
-    }
-  else if (sym->refs == 0)
-    {
-      /* Go ahead and delete the symbol.  */
-      gfc_free_symbol (sym);
-    }
-
+  gfc_release_symbol (sym_tree->n.sym);
   gfc_free (sym_tree);
 }
 
@@ -3189,13 +3188,7 @@  gfc_free_finalizer (gfc_finalizer* el)
 {
   if (el)
     {
-      if (el->proc_sym)
-	{
-	  --el->proc_sym->refs;
-	  if (!el->proc_sym->refs)
-	    gfc_free_symbol (el->proc_sym);
-	}
-
+      gfc_release_symbol (el->proc_sym);
       gfc_free (el);
     }
 }