Patchwork [c++] use VECs in name-lookup.c:is_associated_namespace

login
register
mail settings
Submitter Nathan Froyd
Date Aug. 8, 2010, 8:59 p.m.
Message ID <20100808205924.GD4130@codesourcery.com>
Download mbox | patch
Permalink /patch/61234/
State New
Headers show

Comments

Nathan Froyd - Aug. 8, 2010, 8:59 p.m.
As $SUBJECT suggests.  Sometimes it's a little more work when TREE_LIST
users use TREE_PURPOSE instead of TREE_VALUE. ;)  The function is
slightly longer because we work to recycle the tree vectors.

Tested on x86_64-unknown-linux-gnu, libstdc++ and g++ testsuites.  OK to
commit?

-Nathan

	* name-lookup.c (is_associated_namespace): Convert local variables
	to be VECs instead of TREE_LISTs.
Mark Mitchell - Aug. 9, 2010, 11 a.m.
Nathan Froyd wrote:

> +  VEC(tree,gc) *seen = make_tree_vector ();
> +  VEC(tree,gc) *todo = make_tree_vector ();

Do these have to be "gc"?  I don't think there are any GC points within
this function, right?  OK, modulo that issue.

Thanks,
Nathan Froyd - Aug. 9, 2010, 11:09 a.m.
On Mon, Aug 09, 2010 at 12:00:15PM +0100, Mark Mitchell wrote:
> Nathan Froyd wrote:
> > +  VEC(tree,gc) *seen = make_tree_vector ();
> > +  VEC(tree,gc) *todo = make_tree_vector ();
> 
> Do these have to be "gc"?  I don't think there are any GC points within
> this function, right?  OK, modulo that issue.

There are no GC points.  Points in favor of using gc allocation:

- vec_member can't take VEC(tree,heap) *.  So we have to inline it or
  write vec_heap_member or somesuch.  (We could make vec_member take
  VEC(tree,base) * or whatever the syntax is, but exposing VEC_BASE
  etc. outside of vec.h seems ugly.)

- We potentially get to reuse vectors via make_tree_vector.

Happy to use heap allocation in the interest of getting things out of
GC.

-Nathan
Mark Mitchell - Aug. 9, 2010, 11:12 a.m.
Nathan Froyd wrote:

> - vec_member can't take VEC(tree,heap) *.  So we have to inline it or
>   write vec_heap_member or somesuch.  (We could make vec_member take
>   VEC(tree,base) * or whatever the syntax is, but exposing VEC_BASE
>   etc. outside of vec.h seems ugly.)

Hmm.  It seems to me that vec_member really should be able to deal with
any kind of VEC.  IIUC, the layout of the vector part of a VEC is
independent of the memory allocation scheme, so there's no reason that
vec_member really needs to depend on a particular allocation scheme.  If
that means exposing VEC(tree,base), so be it.

> - We potentially get to reuse vectors via make_tree_vector.

But, that's a good argument.  Let's go with your original approach.

Patch

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index c6e31c2..01f29e4 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4659,25 +4659,38 @@  add_function (struct arg_lookup *k, tree fn)
 bool
 is_associated_namespace (tree current, tree scope)
 {
-  tree seen = NULL_TREE;
-  tree todo = NULL_TREE;
+  VEC(tree,gc) *seen = make_tree_vector ();
+  VEC(tree,gc) *todo = make_tree_vector ();
   tree t;
+  bool ret;
+
   while (1)
     {
       if (scope == current)
-	return true;
-      seen = tree_cons (scope, NULL_TREE, seen);
+	{
+	  ret = true;
+	  break;
+	}
+      VEC_safe_push (tree, gc, seen, scope);
       for (t = DECL_NAMESPACE_ASSOCIATIONS (scope); t; t = TREE_CHAIN (t))
-	if (!purpose_member (TREE_PURPOSE (t), seen))
-	  todo = tree_cons (TREE_PURPOSE (t), NULL_TREE, todo);
-      if (todo)
+	if (!vec_member (TREE_PURPOSE (t), seen))
+	  VEC_safe_push (tree, gc, todo, TREE_PURPOSE (t));
+      if (!VEC_empty (tree, todo))
 	{
-	  scope = TREE_PURPOSE (todo);
-	  todo = TREE_CHAIN (todo);
+	  scope = VEC_last (tree, todo);
+	  VEC_pop (tree, todo);
 	}
       else
-	return false;
+	{
+	  ret = false;
+	  break;
+	}
     }
+
+  release_tree_vector (seen);
+  release_tree_vector (todo);
+
+  return ret;
 }
 
 /* Add functions of a namespace to the lookup structure.