Patchwork Go patch committed: Accept trailing comma after varargs parameter

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

Comments

Ian Taylor - Dec. 13, 2012, 10:20 p.m.
This patch to the Go compiler fixes it to accept a trailing comma after
a varargs parameter, as required by the language spec.

Testing that uncovered a couple of other related bugs: the compiler was
not seeing interface types that appeared only in a function or method
declaration, as in

func (t *T) f(...interface{})

This led to a compiler crash.  Most of this patch is actually concerned
with fixing that.  I had to tweak one test case because the fixes led to
an extra error for that test case for an undefined type.

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

Ian

Patch

Index: gcc/go/gofrontend/gogo.cc
===================================================================
--- gcc/go/gofrontend/gogo.cc	(revision 193945)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -1764,6 +1764,26 @@  Finalize_methods::type(Type* t)
 	      }
 	  }
 
+	// Finalize the types of all methods that are declared but not
+	// defined, since we won't see the declarations otherwise.
+	if (nt->named_object()->package() == NULL
+	    && nt->local_methods() != NULL)
+	  {
+	    const Bindings* methods = nt->local_methods();
+	    for (Bindings::const_declarations_iterator p =
+		   methods->begin_declarations();
+		 p != methods->end_declarations();
+		 p++)
+	      {
+		if (p->second->is_function_declaration())
+		  {
+		    Type* mt = p->second->func_declaration_value()->type();
+		    if (Type::traverse(mt, this) == TRAVERSE_EXIT)
+		      return TRAVERSE_EXIT;
+		  }
+	      }
+	  }
+
 	return TRAVERSE_SKIP_COMPONENTS;
       }
 
@@ -4491,7 +4511,7 @@  Type_declaration::has_methods() const
 void
 Type_declaration::define_methods(Named_type* nt)
 {
-  for (Methods::const_iterator p = this->methods_.begin();
+  for (std::vector<Named_object*>::const_iterator p = this->methods_.begin();
        p != this->methods_.end();
        ++p)
     nt->add_existing_method(*p);
@@ -5230,8 +5250,7 @@  Bindings::traverse(Traverse* traverse, b
     }
 
   // If we need to traverse types, check the function declarations,
-  // which have types.  We don't need to check the type declarations,
-  // as those are just names.
+  // which have types.  Also check any methods of a type declaration.
   if ((traverse_mask & e_or_t) != 0)
     {
       for (Bindings::const_declarations_iterator p =
@@ -5246,6 +5265,27 @@  Bindings::traverse(Traverse* traverse, b
 		  == TRAVERSE_EXIT)
 		return TRAVERSE_EXIT;
 	    }
+	  else if (p->second->is_type_declaration())
+	    {
+	      const std::vector<Named_object*>* methods =
+		p->second->type_declaration_value()->methods();
+	      for (std::vector<Named_object*>::const_iterator pm =
+		     methods->begin();
+		   pm != methods->end();
+		   pm++)
+		{
+		  Named_object* no = *pm;
+		  Type *t;
+		  if (no->is_function())
+		    t = no->func_value()->type();
+		  else if (no->is_function_declaration())
+		    t = no->func_declaration_value()->type();
+		  else
+		    continue;
+		  if (Type::traverse(t, traverse) == TRAVERSE_EXIT)
+		    return TRAVERSE_EXIT;
+		}
+	    }
 	}
     }
 
Index: gcc/go/gofrontend/gogo.h
===================================================================
--- gcc/go/gofrontend/gogo.h	(revision 193945)
+++ gcc/go/gofrontend/gogo.h	(working copy)
@@ -37,8 +37,6 @@  class Channel_type;
 class Interface_type;
 class Named_type;
 class Forward_declaration_type;
-class Method;
-class Methods;
 class Named_object;
 class Label;
 class Translate_context;
@@ -1738,6 +1736,11 @@  class Type_declaration
   bool
   has_methods() const;
 
+  // Return the methods.
+  const std::vector<Named_object*>*
+  methods() const
+  { return &this->methods_; }
+
   // Define methods when the real type is known.
   void
   define_methods(Named_type*);
@@ -1748,8 +1751,6 @@  class Type_declaration
   using_type();
 
  private:
-  typedef std::vector<Named_object*> Methods;
-
   // The location of the type declaration.
   Location location_;
   // If this type is declared in a function, a pointer back to the
@@ -1758,7 +1759,7 @@  class Type_declaration
   // The index of this type in IN_FUNCTION_.
   unsigned int in_function_index_;
   // Methods defined before the type is defined.
-  Methods methods_;
+  std::vector<Named_object*> methods_;
   // True if we have issued a warning about a use of this type
   // declaration when it is undefined.
   bool issued_warning_;
Index: gcc/go/gofrontend/types.h
===================================================================
--- gcc/go/gofrontend/types.h	(revision 194011)
+++ gcc/go/gofrontend/types.h	(working copy)
@@ -2534,19 +2534,11 @@  class Interface_type : public Type
   // Return the list of methods.  This will return NULL for an empty
   // interface.
   const Typed_identifier_list*
-  methods() const
-  {
-    go_assert(this->methods_are_finalized_);
-    return this->all_methods_;
-  }
+  methods() const;
 
   // Return the number of methods.
   size_t
-  method_count() const
-  {
-    go_assert(this->methods_are_finalized_);
-    return this->all_methods_ == NULL ? 0 : this->all_methods_->size();
-  }
+  method_count() const;
 
   // Return the method NAME, or NULL.
   const Typed_identifier*
@@ -3024,6 +3016,9 @@  class Forward_declaration_type : public 
   do_traverse(Traverse* traverse);
 
   bool
+  do_verify();
+
+  bool
   do_has_pointer() const
   { return this->real_type()->has_pointer(); }
 
Index: gcc/go/gofrontend/parse.cc
===================================================================
--- gcc/go/gofrontend/parse.cc	(revision 194240)
+++ gcc/go/gofrontend/parse.cc	(working copy)
@@ -975,13 +975,13 @@  Parse::parameter_list(bool* is_varargs)
   this->parameter_decl(parameters_have_names, ret, is_varargs, &mix_error);
   while (this->peek_token()->is_op(OPERATOR_COMMA))
     {
+      if (this->advance_token()->is_op(OPERATOR_RPAREN))
+	break;
       if (is_varargs != NULL && *is_varargs)
 	{
 	  error_at(this->location(), "%<...%> must be last parameter");
 	  saw_error = true;
 	}
-      if (this->advance_token()->is_op(OPERATOR_RPAREN))
-	break;
       this->parameter_decl(parameters_have_names, ret, is_varargs, &mix_error);
     }
   if (mix_error)
Index: gcc/go/gofrontend/types.cc
===================================================================
--- gcc/go/gofrontend/types.cc	(revision 194011)
+++ gcc/go/gofrontend/types.cc	(working copy)
@@ -6500,6 +6500,24 @@  Type::make_channel_type(bool send, bool 
 
 // Class Interface_type.
 
+// Return the list of methods.
+
+const Typed_identifier_list*
+Interface_type::methods() const
+{
+  go_assert(this->methods_are_finalized_ || saw_errors());
+  return this->all_methods_;
+}
+
+// Return the number of methods.
+
+size_t
+Interface_type::method_count() const
+{
+  go_assert(this->methods_are_finalized_ || saw_errors());
+  return this->all_methods_ == NULL ? 0 : this->all_methods_->size();
+}
+
 // Traversal.
 
 int
@@ -9630,6 +9648,19 @@  Forward_declaration_type::do_traverse(Tr
   return TRAVERSE_CONTINUE;
 }
 
+// Verify the type.
+
+bool
+Forward_declaration_type::do_verify()
+{
+  if (!this->is_defined() && !this->is_nil_constant_as_type())
+    {
+      this->warn();
+      return false;
+    }
+  return true;
+}
+
 // Get the backend representation for the type.
 
 Btype*
Index: gcc/testsuite/go.test/test/fixedbugs/bug228.go
===================================================================
--- gcc/testsuite/go.test/test/fixedbugs/bug228.go	(revision 193595)
+++ gcc/testsuite/go.test/test/fixedbugs/bug228.go	(working copy)
@@ -8,7 +8,7 @@  package main
 
 func f(x int, y ...int)	// ok
 
-func g(x int, y float) (...)	// ERROR "[.][.][.]" "final argument"
+func g(x int, y float32) (...)	// ERROR "[.][.][.]" "final argument"
 
 func h(x, y ...int)		// ERROR "[.][.][.]"