Go patch committed: Don't incorrectly evaluate range variable

Ian Lance Taylor iant@golang.org
Fri Feb 2 13:44:00 GMT 2018


The Go language spec says that in a range loop with a single iteration
variable, the range value is not evaluated if its length is a
constant.  This only matters when the range value is an array type and
evaluating it causes some side effect.  gccgo never got this right;
this patch fixes it.  This fixes https://golang.org/issue/22313.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
-------------- next part --------------
Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 257324)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-b332ba2f0d0302eeb01a228c217928296cec56f6
+981e6621bcd48670d0b58e51e9eeffe549725378
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/statements.cc
===================================================================
--- gcc/go/gofrontend/statements.cc	(revision 257319)
+++ gcc/go/gofrontend/statements.cc	(working copy)
@@ -5307,19 +5307,33 @@ For_range_statement::do_lower(Gogo* gogo
       return Statement::make_error_statement(this->location());
     }
 
+  // If there is only one iteration variable, and len(this->range_) is
+  // constant, then we do not evaluate the range variable.  len(x) is
+  // a contant if x is a string constant or if x is an array.  If x is
+  // a constant then evaluating it won't make any difference, so the
+  // only case to consider is when x is an array.
+  bool eval = true;
+  if (this->value_var_ == NULL
+      && range_type->array_type() != NULL
+      && !range_type->is_slice_type())
+    eval = false;
+
   Location loc = this->location();
   Block* temp_block = new Block(enclosing, loc);
 
   Named_object* range_object = NULL;
   Temporary_statement* range_temp = NULL;
-  Var_expression* ve = this->range_->var_expression();
-  if (ve != NULL)
-    range_object = ve->named_object();
-  else
+  if (eval)
     {
-      range_temp = Statement::make_temporary(NULL, this->range_, loc);
-      temp_block->add_statement(range_temp);
-      this->range_ = NULL;
+      Var_expression* ve = this->range_->var_expression();
+      if (ve != NULL)
+	range_object = ve->named_object();
+      else
+	{
+	  range_temp = Statement::make_temporary(NULL, this->range_, loc);
+	  temp_block->add_statement(range_temp);
+	  this->range_ = NULL;
+	}
     }
 
   Temporary_statement* index_temp = Statement::make_temporary(index_type,
@@ -5474,12 +5488,22 @@ For_range_statement::lower_range_array(G
 
   Block* init = new Block(enclosing, loc);
 
-  Expression* ref = this->make_range_ref(range_object, range_temp, loc);
-  range_temp = Statement::make_temporary(NULL, ref, loc);
-  Expression* len_call = this->call_builtin(gogo, "len", ref, loc);
+  Expression* len_arg;
+  if (range_object == NULL && range_temp == NULL)
+    {
+      // Don't evaluate this->range_, just get its length.
+      len_arg = this->range_;
+    }
+  else
+    {
+      Expression* ref = this->make_range_ref(range_object, range_temp, loc);
+      range_temp = Statement::make_temporary(NULL, ref, loc);
+      init->add_statement(range_temp);
+      len_arg = ref;
+    }
+  Expression* len_call = this->call_builtin(gogo, "len", len_arg, loc);
   Temporary_statement* len_temp = Statement::make_temporary(index_temp->type(),
 							    len_call, loc);
-  init->add_statement(range_temp);
   init->add_statement(len_temp);
 
   Expression* zexpr = Expression::make_integer_ul(0, NULL, loc);
@@ -5495,7 +5519,7 @@ For_range_statement::lower_range_array(G
   // Set *PCOND to
   //   index_temp < len_temp
 
-  ref = Expression::make_temporary_reference(index_temp, loc);
+  Expression* ref = Expression::make_temporary_reference(index_temp, loc);
   Expression* ref2 = Expression::make_temporary_reference(len_temp, loc);
   Expression* lt = Expression::make_binary(OPERATOR_LT, ref, ref2, loc);
 


More information about the Gcc-patches mailing list