From 31766e6833851fc0eeec9eabaa800a8edaa53c60 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 2 Feb 2018 13:44:41 +0000 Subject: [PATCH] compiler: don't incorrectly evaluate range variable The language spec says that in `for i = range x`, in which there is no second iteration variable, if len(x) is constant, then x is not evaluated. This only matters when x is an expression that panics but whose type is an array type; in such a case, we should not evaluate x, since len of any array type is a constant. Fixes golang/go#22313 Reviewed-on: https://go-review.googlesource.com/91555 From-SVN: r257330 --- gcc/go/gofrontend/MERGE | 2 +- gcc/go/gofrontend/statements.cc | 48 ++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 162c4a85b38..34d0e520fdf 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -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. diff --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc index 094966592f6..0b1d7220b5e 100644 --- a/gcc/go/gofrontend/statements.cc +++ b/gcc/go/gofrontend/statements.cc @@ -5307,19 +5307,33 @@ For_range_statement::do_lower(Gogo* gogo, Named_object*, Block* enclosing, 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(Gogo* gogo, 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(Gogo* gogo, // 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);