Patchwork Go patch committed: Check for mismatch between results and uses

login
register
mail settings
Submitter Ian Taylor
Date July 18, 2014, 9:59 p.m.
Message ID <mcr7g3al45p.fsf@iant-glaptop.roam.corp.google.com>
Download mbox | patch
Permalink /patch/371728/
State New
Headers show

Comments

Ian Taylor - July 18, 2014, 9:59 p.m.
The Go frontend had a bug in that it would not give an error for
    a, b := F()
when F returned more than two results.  This patch fixes the bug.  I've
proposed a test case for the master testsuite in
http://codereview.appspot.com/111360045 .  Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu.  Committed to mainline.

Ian

Patch

diff -r d56f774d848e go/expressions.cc
--- a/go/expressions.cc	Fri Jul 11 16:53:55 2014 -0700
+++ b/go/expressions.cc	Fri Jul 18 14:15:40 2014 -0700
@@ -9065,6 +9065,15 @@ 
   return (*this->results_)[i];
 }
 
+// Set the number of results expected from a call expression.
+
+void
+Call_expression::set_expected_result_count(size_t count)
+{
+  go_assert(this->expected_result_count_ == 0);
+  this->expected_result_count_ = count;
+}
+
 // Return whether this is a call to the predeclared function recover.
 
 bool
@@ -9252,6 +9261,15 @@ 
       return;
     }
 
+  if (this->expected_result_count_ != 0
+      && this->expected_result_count_ != this->result_count())
+    {
+      if (this->issue_error())
+	this->report_error(_("function result count mismatch"));
+      this->set_is_error();
+      return;
+    }
+
   bool is_method = fntype->is_method();
   if (is_method)
     {
@@ -9302,6 +9320,20 @@ 
       if (!is_method || this->args_->size() > 1)
 	this->report_error(_("too many arguments"));
     }
+  else if (this->args_->size() == 1
+	   && this->args_->front()->call_expression() != NULL
+	   && this->args_->front()->call_expression()->result_count() > 1)
+    {
+      // This is F(G()) when G returns more than one result.  If the
+      // results can be matched to parameters, it would have been
+      // lowered in do_lower.  If we get here we know there is a
+      // mismatch.
+      if (this->args_->front()->call_expression()->result_count()
+	  < parameters->size())
+	this->report_error(_("not enough arguments"));
+      else
+	this->report_error(_("too many arguments"));
+    }
   else
     {
       int i = 0;
diff -r d56f774d848e go/expressions.h
--- a/go/expressions.h	Fri Jul 11 16:53:55 2014 -0700
+++ b/go/expressions.h	Fri Jul 18 14:15:40 2014 -0700
@@ -1606,9 +1606,9 @@ 
 		  Location location)
     : Expression(EXPRESSION_CALL, location),
       fn_(fn), args_(args), type_(NULL), results_(NULL), call_(NULL),
-      call_temp_(NULL), is_varargs_(is_varargs), are_hidden_fields_ok_(false),
-      varargs_are_lowered_(false), types_are_determined_(false),
-      is_deferred_(false), issued_error_(false)
+      call_temp_(NULL), expected_result_count_(0), is_varargs_(is_varargs),
+      are_hidden_fields_ok_(false), varargs_are_lowered_(false),
+      types_are_determined_(false), is_deferred_(false), issued_error_(false)
   { }
 
   // The function to call.
@@ -1639,6 +1639,12 @@ 
   Temporary_statement*
   result(size_t i) const;
 
+  // Set the number of results expected from this call.  This is used
+  // when the call appears in a context that expects multiple results,
+  // such as a, b = f().
+  void
+  set_expected_result_count(size_t);
+
   // Return whether this is a call to the predeclared function
   // recover.
   bool
@@ -1767,6 +1773,9 @@ 
   Bexpression* call_;
   // A temporary variable to store this call if the function returns a tuple.
   Temporary_statement* call_temp_;
+  // If not 0, the number of results expected from this call, when
+  // used in a context that expects multiple values.
+  size_t expected_result_count_;
   // True if the last argument is a varargs argument (f(a...)).
   bool is_varargs_;
   // True if this statement may pass hidden fields in the arguments.
diff -r d56f774d848e go/parse.cc
--- a/go/parse.cc	Fri Jul 11 16:53:55 2014 -0700
+++ b/go/parse.cc	Fri Jul 18 14:15:40 2014 -0700
@@ -1694,6 +1694,8 @@ 
   // the right number of values, but it might.  Declare the variables,
   // and then assign the results of the call to them.
 
+  call->set_expected_result_count(vars->size());
+
   Named_object* first_var = NULL;
   unsigned int index = 0;
   bool any_new = false;
@@ -4101,6 +4103,7 @@ 
     {
       if (op != OPERATOR_EQ)
 	error_at(location, "multiple results only permitted with %<=%>");
+      call->set_expected_result_count(lhs->size());
       delete vals;
       vals = new Expression_list;
       for (unsigned int i = 0; i < lhs->size(); ++i)
diff -r d56f774d848e go/statements.cc
--- a/go/statements.cc	Fri Jul 11 16:53:55 2014 -0700
+++ b/go/statements.cc	Fri Jul 18 14:15:40 2014 -0700
@@ -2664,6 +2664,7 @@ 
       && vals->front()->call_expression() != NULL)
     {
       Call_expression* call = vals->front()->call_expression();
+      call->set_expected_result_count(results_count);
       delete vals;
       vals = new Expression_list;
       for (size_t i = 0; i < results_count; ++i)