diff mbox

Go patch committed: Fix imported embedded builtin types again

Message ID mcrob6nacdy.fsf@iant-glaptop.roam.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor Oct. 17, 2013, 8:08 p.m. UTC
I found another bug in my recent change to fix the handling of imported
embedded builtin types.  Two bugs means I need a different approach.
This patch removes the code I added earlier and adopts a simpler
approach, in which we mark a struct field for whether it is imported.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.8 branch.

Ian
diff mbox

Patch

diff -r 6ecdc3838869 go/types.cc
--- a/go/types.cc	Thu Oct 17 11:40:01 2013 -0700
+++ b/go/types.cc	Thu Oct 17 13:03:06 2013 -0700
@@ -4208,13 +4208,44 @@ 
 
       // This is a horrible hack caused by the fact that we don't pack
       // the names of builtin types.  FIXME.
+      if (!this->is_imported_
+	  && nt != NULL
+	  && nt->is_builtin()
+	  && nt->name() == Gogo::unpack_hidden_name(name))
+	return true;
+
+      return false;
+    }
+}
+
+// Return whether this field is an unexported field named NAME.
+
+bool
+Struct_field::is_unexported_field_name(Gogo* gogo,
+				       const std::string& name) const
+{
+  const std::string& field_name(this->field_name());
+  if (Gogo::is_hidden_name(field_name)
+      && name == Gogo::unpack_hidden_name(field_name)
+      && gogo->pack_hidden_name(name, false) != field_name)
+    return true;
+
+  // Check for the name of a builtin type.  This is like the test in
+  // is_field_name, only there we return false if this->is_imported_,
+  // and here we return true.
+  if (this->is_imported_ && this->is_anonymous())
+    {
+      Type* t = this->typed_identifier_.type();
+      if (t->points_to() != NULL)
+	t = t->points_to();
+      Named_type* nt = t->named_type();
       if (nt != NULL
 	  && nt->is_builtin()
 	  && nt->name() == Gogo::unpack_hidden_name(name))
 	return true;
-
-      return false;
-    }
+    }
+
+  return false;
 }
 
 // Return whether this field is an embedded built-in type.
@@ -4649,13 +4680,8 @@ 
       for (Struct_field_list::const_iterator pf = fields->begin();
 	   pf != fields->end();
 	   ++pf)
-	{
-	  const std::string& field_name(pf->field_name());
-	  if (Gogo::is_hidden_name(field_name)
-	      && name == Gogo::unpack_hidden_name(field_name)
-	      && gogo->pack_hidden_name(name, false) != field_name)
-	    return true;
-	}
+	if (pf->is_unexported_field_name(gogo, name))
+	  return true;
     }
   return false;
 }
@@ -5257,34 +5283,8 @@ 
 	    }
 	  Type* ftype = imp->read_type();
 
-	  // We don't pack the names of builtin types.  In
-	  // Struct_field::is_field_name we cope with a hack.  Now we
-	  // need another hack so that we don't accidentally think
-	  // that an embedded builtin type is accessible from another
-	  // package (we know that all the builtin types are not
-	  // exported).
-	  // This is called during parsing, before anything is
-	  // lowered, so we have to be careful to avoid dereferencing
-	  // an unknown type name.
-	  if (name.empty())
-	    {
-	      Type *t = ftype;
-	      if (t->classification() == Type::TYPE_POINTER)
-		{
-		  // Very ugly.
-		  Pointer_type* ptype = static_cast<Pointer_type*>(t);
-		  t = ptype->points_to();
-		}
-	      std::string tname;
-	      if (t->forward_declaration_type() != NULL)
-		tname = t->forward_declaration_type()->name();
-	      else if (t->named_type() != NULL)
-		tname = t->named_type()->name();
-	      if (!tname.empty() && tname[0] >= 'a' && tname[0] <= 'z')
-		name = '.' + imp->package()->pkgpath() + '.' + tname;
-	    }
-
 	  Struct_field sf(Typed_identifier(name, ftype, imp->location()));
+	  sf.set_is_imported();
 
 	  if (imp->peek_char() == ' ')
 	    {
@@ -9324,7 +9324,9 @@ 
       else
 	{
 	  bool is_unexported;
-	  if (!Gogo::is_hidden_name(name))
+	  // The test for 'a' and 'z' is to handle builtin names,
+	  // which are not hidden.
+	  if (!Gogo::is_hidden_name(name) && (name[0] < 'a' || name[0] > 'z'))
 	    is_unexported = false;
 	  else
 	    {
diff -r 6ecdc3838869 go/types.h
--- a/go/types.h	Thu Oct 17 11:40:01 2013 -0700
+++ b/go/types.h	Thu Oct 17 13:03:06 2013 -0700
@@ -1924,7 +1924,7 @@ 
 {
  public:
   explicit Struct_field(const Typed_identifier& typed_identifier)
-    : typed_identifier_(typed_identifier), tag_(NULL)
+    : typed_identifier_(typed_identifier), tag_(NULL), is_imported_(false)
   { }
 
   // The field name.
@@ -1935,6 +1935,10 @@ 
   bool
   is_field_name(const std::string& name) const;
 
+  // Return whether this struct field is an unexported field named NAME.
+  bool
+  is_unexported_field_name(Gogo*, const std::string& name) const;
+
   // Return whether this struct field is an embedded built-in type.
   bool
   is_embedded_builtin(Gogo*) const;
@@ -1972,6 +1976,11 @@ 
   set_tag(const std::string& tag)
   { this->tag_ = new std::string(tag); }
 
+  // Record that this field is defined in an imported struct.
+  void
+  set_is_imported()
+  { this->is_imported_ = true; }
+
   // Set the type.  This is only used in error cases.
   void
   set_type(Type* type)
@@ -1982,6 +1991,8 @@ 
   Typed_identifier typed_identifier_;
   // The field tag.  This is NULL if the field has no tag.
   std::string* tag_;
+  // Whether this field is defined in an imported struct.
+  bool is_imported_;
 };
 
 // A list of struct fields.