Patchwork Go patch committed: Fix a, b, c := b, a, 1 when a and b already exist

login
register
mail settings
Submitter Ian Taylor
Date Oct. 3, 2012, 5:03 a.m.
Message ID <mcrhaqcxi3c.fsf@google.com>
Download mbox | patch
Permalink /patch/188707/
State New
Headers show

Comments

Ian Taylor - Oct. 3, 2012, 5:03 a.m.
The Go := operator can declare new variables while also referring to
existing variables.  The Go frontend was miscompiling a case like a, b,
c := b, a, 1 where a and b already exist.  It was assigning to a and
then using the new value of to assign to b.  This patch fixes this bug.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.7 branch.

Ian

Patch

diff -r b66fde007054 go/parse.cc
--- a/go/parse.cc	Tue Oct 02 16:49:04 2012 -0700
+++ b/go/parse.cc	Tue Oct 02 21:50:56 2012 -0700
@@ -1631,12 +1631,16 @@ 
 
   // Note that INIT was already parsed with the old name bindings, so
   // we don't have to worry that it will accidentally refer to the
-  // newly declared variables.
+  // newly declared variables.  But we do have to worry about a mix of
+  // newly declared variables and old variables if the old variables
+  // appear in the initializations.
 
   Expression_list::const_iterator pexpr;
   if (init != NULL)
     pexpr = init->begin();
   bool any_new = false;
+  Expression_list* vars = new Expression_list();
+  Expression_list* vals = new Expression_list();
   for (Typed_identifier_list::const_iterator p = til->begin();
        p != til->end();
        ++p)
@@ -1644,7 +1648,7 @@ 
       if (init != NULL)
 	go_assert(pexpr != init->end());
       this->init_var(*p, type, init == NULL ? NULL : *pexpr, is_coloneq,
-		     false, &any_new);
+		     false, &any_new, vars, vals);
       if (init != NULL)
 	++pexpr;
     }
@@ -1652,6 +1656,7 @@ 
     go_assert(pexpr == init->end());
   if (is_coloneq && !any_new)
     error_at(location, "variables redeclared but no variable is new");
+  this->finish_init_vars(vars, vals, location);
 }
 
 // See if we need to initialize a list of variables from a function
@@ -1674,13 +1679,15 @@ 
   Named_object* first_var = NULL;
   unsigned int index = 0;
   bool any_new = false;
+  Expression_list* ivars = new Expression_list();
+  Expression_list* ivals = new Expression_list();
   for (Typed_identifier_list::const_iterator pv = vars->begin();
        pv != vars->end();
        ++pv, ++index)
     {
       Expression* init = Expression::make_call_result(call, index);
       Named_object* no = this->init_var(*pv, type, init, is_coloneq, false,
-					&any_new);
+					&any_new, ivars, ivals);
 
       if (this->gogo_->in_global_scope() && no->is_variable())
 	{
@@ -1700,6 +1707,8 @@ 
   if (is_coloneq && !any_new)
     error_at(location, "variables redeclared but no variable is new");
 
+  this->finish_init_vars(ivars, ivals, location);
+
   return true;
 }
 
@@ -1725,7 +1734,7 @@ 
   Typed_identifier_list::const_iterator p = vars->begin();
   Expression* init = type == NULL ? index : NULL;
   Named_object* val_no = this->init_var(*p, type, init, is_coloneq,
-					type == NULL, &any_new);
+					type == NULL, &any_new, NULL, NULL);
   if (type == NULL && any_new && val_no->is_variable())
     val_no->var_value()->set_type_from_init_tuple();
   Expression* val_var = Expression::make_var_reference(val_no, location);
@@ -1735,7 +1744,7 @@ 
   if (var_type == NULL)
     var_type = Type::lookup_bool_type();
   Named_object* no = this->init_var(*p, var_type, NULL, is_coloneq, false,
-				    &any_new);
+				    &any_new, NULL, NULL);
   Expression* present_var = Expression::make_var_reference(no, location);
 
   if (is_coloneq && !any_new)
@@ -1790,7 +1799,7 @@ 
   Typed_identifier_list::const_iterator p = vars->begin();
   Expression* init = type == NULL ? receive : NULL;
   Named_object* val_no = this->init_var(*p, type, init, is_coloneq,
-					type == NULL, &any_new);
+					type == NULL, &any_new, NULL, NULL);
   if (type == NULL && any_new && val_no->is_variable())
     val_no->var_value()->set_type_from_init_tuple();
   Expression* val_var = Expression::make_var_reference(val_no, location);
@@ -1800,7 +1809,7 @@ 
   if (var_type == NULL)
     var_type = Type::lookup_bool_type();
   Named_object* no = this->init_var(*p, var_type, NULL, is_coloneq, false,
-				    &any_new);
+				    &any_new, NULL, NULL);
   Expression* received_var = Expression::make_var_reference(no, location);
 
   if (is_coloneq && !any_new)
@@ -1857,7 +1866,7 @@ 
   if (var_type == NULL)
     var_type = type_guard->type();
   Named_object* val_no = this->init_var(*p, var_type, NULL, is_coloneq, false,
-					&any_new);
+					&any_new, NULL, NULL);
   Expression* val_var = Expression::make_var_reference(val_no, location);
 
   ++p;
@@ -1865,7 +1874,7 @@ 
   if (var_type == NULL)
     var_type = Type::lookup_bool_type();
   Named_object* no = this->init_var(*p, var_type, NULL, is_coloneq, false,
-				    &any_new);
+				    &any_new, NULL, NULL);
   Expression* ok_var = Expression::make_var_reference(no, location);
 
   Expression* texpr = type_guard->expr();
@@ -1904,7 +1913,8 @@ 
 
 Named_object*
 Parse::init_var(const Typed_identifier& tid, Type* type, Expression* init,
-		bool is_coloneq, bool type_from_init, bool* is_new)
+		bool is_coloneq, bool type_from_init, bool* is_new,
+		Expression_list* vars, Expression_list* vals)
 {
   Location location = tid.location();
 
@@ -1946,9 +1956,9 @@ 
 	  // like v, ok := x.(int).
 	  if (!type_from_init && init != NULL)
 	    {
-	      Expression *v = Expression::make_var_reference(no, location);
-	      Statement *s = Statement::make_assignment(v, init, location);
-	      this->gogo_->add_statement(s);
+	      go_assert(vars != NULL && vals != NULL);
+	      vars->push_back(Expression::make_var_reference(no, location));
+	      vals->push_back(init);
 	    }
 	  return no;
 	}
@@ -1983,6 +1993,36 @@ 
   return this->gogo_->add_variable(buf, var);
 }
 
+// Finish the variable initialization by executing any assignments to
+// existing variables when using :=.  These must be done as a tuple
+// assignment in case of something like n, a, b := 1, b, a.
+
+void
+Parse::finish_init_vars(Expression_list* vars, Expression_list* vals,
+			Location location)
+{
+  if (vars->empty())
+    {
+      delete vars;
+      delete vals;
+    }
+  else if (vars->size() == 1)
+    {
+      go_assert(!this->gogo_->in_global_scope());
+      this->gogo_->add_statement(Statement::make_assignment(vars->front(),
+							    vals->front(),
+							    location));
+      delete vars;
+      delete vals;
+    }
+  else
+    {
+      go_assert(!this->gogo_->in_global_scope());
+      this->gogo_->add_statement(Statement::make_tuple_assignment(vars, vals,
+								  location));
+    }
+}
+
 // SimpleVarDecl = identifier ":=" Expression .
 
 // We've already seen the identifier.
@@ -5113,7 +5153,8 @@ 
   bool any_new = false;
 
   const Typed_identifier* pti = &til->front();
-  Named_object* no = this->init_var(*pti, NULL, expr, true, true, &any_new);
+  Named_object* no = this->init_var(*pti, NULL, expr, true, true, &any_new,
+				    NULL, NULL);
   if (any_new && no->is_variable())
     no->var_value()->set_type_from_range_index();
   p_range_clause->index = Expression::make_var_reference(no, location);
@@ -5124,7 +5165,7 @@ 
     {
       pti = &til->back();
       bool is_new = false;
-      no = this->init_var(*pti, NULL, expr, true, true, &is_new);
+      no = this->init_var(*pti, NULL, expr, true, true, &is_new, NULL, NULL);
       if (is_new && no->is_variable())
 	no->var_value()->set_type_from_range_value();
       if (is_new)
diff -r b66fde007054 go/parse.h
--- a/go/parse.h	Tue Oct 02 16:49:04 2012 -0700
+++ b/go/parse.h	Tue Oct 02 21:50:56 2012 -0700
@@ -206,8 +206,11 @@ 
 				 Expression*, bool is_coloneq,
 				 Location);
   Named_object* init_var(const Typed_identifier&, Type*, Expression*,
-			 bool is_coloneq, bool type_from_init, bool* is_new);
+			 bool is_coloneq, bool type_from_init, bool* is_new,
+			 Expression_list* vars, Expression_list* vals);
   Named_object* create_dummy_global(Type*, Expression*, Location);
+  void finish_init_vars(Expression_list* vars, Expression_list* vals,
+			Location);
   void simple_var_decl_or_assignment(const std::string&, Location,
 				     bool may_be_composite_lit,
 				     Range_clause*, Type_switch*);