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
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);