Patchwork Go patch committed:

login
register
mail settings
Submitter Ian Taylor
Date Dec. 21, 2012, 10:23 p.m.
Message ID <mcr623vt57l.fsf@google.com>
Download mbox | patch
Permalink /patch/207893/
State New
Headers show

Comments

Ian Taylor - Dec. 21, 2012, 10:23 p.m.
In Go it is an error if the same name is defined in both the package
block (names defined anywhere in the package) and the file block (names
defined only in a single file, which can only happen via import
statements).  The gccgo frontend was not diagnosing this error.  This
patch fixes the problem.  This required a tweak to one of the tests.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r 9a016fbfe7b3 go/gogo.cc
--- a/go/gogo.cc	Fri Dec 21 06:59:06 2012 -0800
+++ b/go/gogo.cc	Fri Dec 21 14:19:44 2012 -0800
@@ -29,6 +29,7 @@ 
     package_(NULL),
     functions_(),
     globals_(new Bindings(NULL)),
+    file_block_names_(),
     imports_(),
     imported_unsafe_(false),
     packages_(),
@@ -1243,6 +1244,33 @@ 
       else if (no->is_unknown())
 	no->unknown_value()->set_real_named_object(global_no);
     }
+
+  // Give an error if any name is defined in both the package block
+  // and the file block.  For example, this can happen if one file
+  // imports "fmt" and another file defines a global variable fmt.
+  for (Bindings::const_declarations_iterator p =
+	 this->package_->bindings()->begin_declarations();
+       p != this->package_->bindings()->end_declarations();
+       ++p)
+    {
+      if (p->second->is_unknown()
+	  && p->second->unknown_value()->real_named_object() == NULL)
+	{
+	  // No point in warning about an undefined name, as we will
+	  // get other errors later anyhow.
+	  continue;
+	}
+      File_block_names::const_iterator pf =
+	this->file_block_names_.find(p->second->name());
+      if (pf != this->file_block_names_.end())
+	{
+	  std::string n = p->second->message_name();
+	  error_at(p->second->location(),
+		   "%qs defined as both imported name and global name",
+		   n.c_str());
+	  inform(pf->second, "%qs imported here", n.c_str());
+	}
+    }
 }
 
 // Clear out names in file scope.
@@ -1250,7 +1278,7 @@ 
 void
 Gogo::clear_file_scope()
 {
-  this->package_->bindings()->clear_file_scope();
+  this->package_->bindings()->clear_file_scope(this);
 
   // Warn about packages which were imported but not used.
   bool quiet = saw_errors();
@@ -4855,7 +4883,7 @@ 
 // Clear imports.
 
 void
-Bindings::clear_file_scope()
+Bindings::clear_file_scope(Gogo* gogo)
 {
   Contour::iterator p = this->bindings_.begin();
   while (p != this->bindings_.end())
@@ -4875,7 +4903,10 @@ 
       if (keep)
 	++p;
       else
-	p = this->bindings_.erase(p);
+	{
+	  gogo->add_file_block_name(p->second->name(), p->second->location());
+	  p = this->bindings_.erase(p);
+	}
     }
 }
 
diff -r 9a016fbfe7b3 go/gogo.h
--- a/go/gogo.h	Fri Dec 21 06:59:06 2012 -0800
+++ b/go/gogo.h	Fri Dec 21 14:19:44 2012 -0800
@@ -377,6 +377,11 @@ 
   void
   add_named_object(Named_object*);
 
+  // Add an identifier to the list of names seen in the file block.
+  void
+  add_file_block_name(const std::string& name, Location location)
+  { this->file_block_names_[name] = location; }
+
   // Mark all local variables in current bindings as used.  This is
   // used when there is a parse error to avoid useless errors.
   void
@@ -678,6 +683,10 @@ 
   // This is used for initialization dependency analysis.
   typedef std::map<Variable*, Named_object*> Var_deps;
 
+  // Type used to map identifiers in the file block to the location
+  // where they were defined.
+  typedef Unordered_map(std::string, Location) File_block_names;
+
   // Type used to queue writing a type specific function.
   struct Specific_type_function
   {
@@ -710,6 +719,8 @@ 
   // The global binding contour.  This includes the builtin functions
   // and the package we are compiling.
   Bindings* globals_;
+  // The list of names we have seen in the file block.
+  File_block_names file_block_names_;
   // Mapping from import file names to packages.
   Imports imports_;
   // Whether the magic unsafe package was imported.
@@ -2265,7 +2276,7 @@ 
 
   // Clear all names in file scope from the bindings.
   void
-  clear_file_scope();
+  clear_file_scope(Gogo*);
 
   // Look up a name in this binding contour and in any enclosing
   // binding contours.  This returns NULL if the name is not found.
diff -r 9a016fbfe7b3 libgo/go/image/image_test.go
--- a/libgo/go/image/image_test.go	Fri Dec 21 06:59:06 2012 -0800
+++ b/libgo/go/image/image_test.go	Fri Dec 21 14:19:44 2012 -0800
@@ -10,7 +10,7 @@ 
 	"testing"
 )
 
-type image interface {
+type timage interface {
 	Image
 	Opaque() bool
 	Set(int, int, color.Color)
@@ -24,7 +24,7 @@ 
 }
 
 func TestImage(t *testing.T) {
-	testImage := []image{
+	testImage := []timage{
 		NewRGBA(Rect(0, 0, 10, 10)),
 		NewRGBA64(Rect(0, 0, 10, 10)),
 		NewNRGBA(Rect(0, 0, 10, 10)),
@@ -52,11 +52,11 @@ 
 			t.Errorf("%T: at (6, 3), want a non-zero color, got %v", m, m.At(6, 3))
 			continue
 		}
-		if !m.SubImage(Rect(6, 3, 7, 4)).(image).Opaque() {
+		if !m.SubImage(Rect(6, 3, 7, 4)).(timage).Opaque() {
 			t.Errorf("%T: at (6, 3) was not opaque", m)
 			continue
 		}
-		m = m.SubImage(Rect(3, 2, 9, 8)).(image)
+		m = m.SubImage(Rect(3, 2, 9, 8)).(timage)
 		if !Rect(3, 2, 9, 8).Eq(m.Bounds()) {
 			t.Errorf("%T: sub-image want bounds %v, got %v", m, Rect(3, 2, 9, 8), m.Bounds())
 			continue
@@ -97,7 +97,7 @@ 
 			continue
 		}
 	}
-	testImage := []image{
+	testImage := []timage{
 		NewRGBA64(Rect(0, 0, 10, 10)),
 		NewNRGBA64(Rect(0, 0, 10, 10)),
 		NewAlpha16(Rect(0, 0, 10, 10)),