Patchwork Go patch committed: More tweaking of recursive types

login
register
mail settings
Submitter Ian Taylor
Date Dec. 22, 2010, 4:05 p.m.
Message ID <mcrvd2lhjy1.fsf@google.com>
Download mbox | patch
Permalink /patch/76429/
State New
Headers show

Comments

Ian Taylor - Dec. 22, 2010, 4:05 p.m.
This patch to the Go frontend is another attempt to get the balance
right for recursive named types.  The problem is that named types can
refer to themselves in Go, and that causes trouble when converting to
GENERIC.  A simple check of which types we have already seen gives us
the wrong results when a named type is defined as a function with an
argument which is a pointer to a struct which has a field which is a
pointer to the named type; we can represent that in GENERIC, but only if
we let ourselves resolve the recursive reference to the named type.  But
if we just keep going, we can run into an infinite loop.  This patch
uses a counter to avoid infinite loops while still permitting some
recursive references.  We continue to look for specific cases that we
know will cause trouble in GENERIC.

Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r 478181dd4a99 go/types.cc
--- a/go/types.cc	Wed Dec 22 07:24:00 2010 -0800
+++ b/go/types.cc	Wed Dec 22 07:49:45 2010 -0800
@@ -6511,22 +6511,22 @@ 
 Type*
 Named_type::named_base()
 {
-  if (this->seen_)
+  if (this->seen_ > 0)
     return this;
-  this->seen_ = true;
+  ++this->seen_;
   Type* ret = this->type_->base();
-  this->seen_ = false;
+  --this->seen_;
   return ret;
 }
 
 const Type*
 Named_type::named_base() const
 {
-  if (this->seen_)
+  if (this->seen_ > 0)
     return this;
-  this->seen_ = true;
+  ++this->seen_;
   const Type* ret = this->type_->base();
-  this->seen_ = false;
+  --this->seen_;
   return ret;
 }
 
@@ -6536,11 +6536,11 @@ 
 bool
 Named_type::is_named_error_type() const
 {
-  if (this->seen_)
+  if (this->seen_ > 0)
     return false;
-  this->seen_ = true;
+  ++this->seen_;
   bool ret = this->type_->is_error_type();
-  this->seen_ = false;
+  --this->seen_;
   return ret;
 }
 
@@ -6690,11 +6690,11 @@ 
 bool
 Named_type::named_type_has_hidden_fields(std::string* reason) const
 {
-  if (this->seen_)
+  if (this->seen_ > 0)
     return false;
-  this->seen_ = true;
+  ++this->seen_;
   bool ret = this->type_->has_hidden_fields(this, reason);
-  this->seen_ = false;
+  --this->seen_;
   return ret;
 }
 
@@ -6896,18 +6896,24 @@ 
     case TYPE_FUNCTION:
       // GENERIC can't handle a pointer to a function type whose
       // return type is a pointer to the function type itself.  It
-      // does into infinite loops when walking the types.
-      if (this->seen_)
+      // goes into an infinite loop when walking the types.
+      if (this->seen_ > 0)
 	{
 	  Function_type* fntype = this->type_->function_type();
 	  if (fntype->results() != NULL
 	      && fntype->results()->size() == 1
 	      && fntype->results()->front().type()->forwarded() == this)
 	    return ptr_type_node;
+
+	  // We can legitimately see ourselves here twice when a named
+	  // type is defined using a struct which refers to the named
+	  // type.  If we see ourselves too often we are in a loop.
+	  if (this->seen_ > 3)
+	    return ptr_type_node;
 	}
-      this->seen_ = true;
+      ++this->seen_;
       t = Type::get_named_type_tree(gogo, this->type_);
-      this->seen_ = false;
+      --this->seen_;
       if (t == error_mark_node)
 	return error_mark_node;
       t = build_variant_type_copy(t);
@@ -6917,11 +6923,17 @@ 
       // Don't recur infinitely if a pointer type refers to itself.
       // Ideally we would build a circular data structure here, but
       // GENERIC can't handle them.
-      if (this->seen_)
-	return ptr_type_node;
-      this->seen_ = true;
+      if (this->seen_ > 0)
+	{
+	  if (this->type_->points_to()->forwarded() == this)
+	    return ptr_type_node;
+
+	  if (this->seen_ > 3)
+	    return ptr_type_node;
+	}
+      ++this->seen_;
       t = Type::get_named_type_tree(gogo, this->type_);
-      this->seen_ = false;
+      --this->seen_;
       if (t == error_mark_node)
 	return error_mark_node;
       t = build_variant_type_copy(t);
@@ -6980,11 +6992,10 @@ 
 	// definition of T2 may refer to T1, then we must simply
 	// return the type for T2 here.  It's not precisely correct,
 	// but it's as close as we can get with GENERIC.
-	bool was_seen = this->seen_;
-	this->seen_ = true;
+	++this->seen_;
 	t = Type::get_named_type_tree(gogo, this->type_);
-	this->seen_ = was_seen;
-	if (was_seen)
+	--this->seen_;
+	if (this->seen_ > 0)
 	  return t;
 	if (t == error_mark_node)
 	  return error_mark_node;
diff -r 478181dd4a99 go/types.h
--- a/go/types.h	Wed Dec 22 07:24:00 2010 -0800
+++ b/go/types.h	Wed Dec 22 07:49:45 2010 -0800
@@ -2387,7 +2387,7 @@ 
       local_methods_(NULL), all_methods_(NULL),
       interface_method_tables_(NULL), pointer_interface_method_tables_(NULL),
       location_(location), named_tree_(NULL), is_visible_(true),
-      is_error_(false), seen_(false)
+      is_error_(false), seen_(0)
   { }
 
   // Return the associated Named_object.  This holds the actual name.
@@ -2626,7 +2626,7 @@ 
   // used to prevent infinite recursion when a type refers to itself.
   // This is mutable because it is always reset to false when the
   // function exits.
-  mutable bool seen_;
+  mutable int seen_;
 };
 
 // A forward declaration.  This handles a type which has been declared