There were a few cases where the Go compiler was evaluating value
multiple times when they were converted to or from interface types.
This happened when the conversion occurred in a composite literal, or
even, when implicitly converting from one interface type to another,
in a variable declaration. For this patch I added an assertion for
the cases where a value might be evaluated multiple times, and then
patched the compiler until it stopped crashing on the testsuite.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline. This bug does not occur on the 4.9 branch.
I've committed a test case to the master Go repository
(https://go-review.googlesource.com/#/c/1710/).
Ian
diff -r 47792391c17a go/expressions.cc
--- a/go/expressions.cc Wed Dec 17 06:26:14 2014 -0800
+++ b/go/expressions.cc Thu Dec 18 19:29:08 2014 -0800
@@ -302,6 +302,9 @@
bool for_type_guard,
Location location)
{
+ if (Type::are_identical(lhs_type, rhs->type(), false, NULL))
+ return rhs;
+
Interface_type* lhs_interface_type = lhs_type->interface_type();
bool lhs_is_empty = lhs_interface_type->is_empty();
@@ -313,6 +316,9 @@
// to do a runtime check, although we still need to build a new
// method table.
+ // We are going to evaluate RHS multiple times.
+ go_assert(rhs->is_variable());
+
// Get the type descriptor for the right hand side. This will be
// NULL for a nil interface.
Expression* rhs_type_expr = Expression::get_interface_type_descriptor(rhs);
@@ -355,6 +361,9 @@
Expression::convert_interface_to_type(Type *lhs_type, Expression* rhs,
Location location)
{
+ // We are going to evaluate RHS multiple times.
+ go_assert(rhs->is_variable());
+
// Call a function to check that the type is valid. The function
// will panic with an appropriate runtime type error if the type is
// not valid.
@@ -3155,8 +3164,7 @@
{
if (((this->type()->is_string_type()
&& this->expr_->type()->is_slice_type())
- || (this->type()->interface_type() != NULL
- && this->expr_->type()->interface_type() != NULL))
+ || this->expr_->type()->interface_type() != NULL)
&& !this->expr_->is_variable())
{
Temporary_statement* temp =
@@ -8782,12 +8790,17 @@
else
{
Location loc = (*pa)->location();
- Expression* arg =
- Expression::convert_for_assignment(gogo, pp->type(), *pa, loc);
- Temporary_statement* temp =
- Statement::make_temporary(pp->type(), arg, loc);
- inserter->insert(temp);
- args->push_back(Expression::make_temporary_reference(temp, loc));
+ Expression* arg = *pa;
+ if (!arg->is_variable())
+ {
+ Temporary_statement *temp =
+ Statement::make_temporary(NULL, arg, loc);
+ inserter->insert(temp);
+ arg = Expression::make_temporary_reference(temp, loc);
+ }
+ arg = Expression::convert_for_assignment(gogo, pp->type(), arg,
+ loc);
+ args->push_back(arg);
}
}
delete this->args_;
@@ -11602,6 +11615,9 @@
return ret;
}
+ Expression*
+ do_flatten(Gogo*, Named_object*, Statement_inserter*);
+
Bexpression*
do_get_backend(Translate_context*);
@@ -11776,6 +11792,39 @@
go_assert(pv == this->vals_->end());
}
+// Flatten a struct construction expression. Store the values into
+// temporaries in case they need interface conversion.
+
+Expression*
+Struct_construction_expression::do_flatten(Gogo*, Named_object*,
+ Statement_inserter* inserter)
+{
+ if (this->vals_ == NULL)
+ return this;
+
+ // If this is a constant struct, we don't need temporaries.
+ if (this->is_constant_struct())
+ return this;
+
+ Location loc = this->location();
+ for (Expression_list::iterator pv = this->vals_->begin();
+ pv != this->vals_->end();
+ ++pv)
+ {
+ if (*pv != NULL)
+ {
+ if (!(*pv)->is_variable())
+ {
+ Temporary_statement* temp =
+ Statement::make_temporary(NULL, *pv, loc);
+ inserter->insert(temp);
+ *pv = Expression::make_temporary_reference(temp, loc);
+ }
+ }
+ }
+ return this;
+}
+
// Return the backend representation for constructing a struct.
Bexpression*
@@ -11909,6 +11958,9 @@
vals()
{ return this->vals_; }
+ Expression*
+ do_flatten(Gogo*, Named_object*, Statement_inserter*);
+
// Get the backend constructor for the array values.
Bexpression*
get_constructor(Translate_context* context, Btype* btype);
@@ -12024,6 +12076,39 @@
}
}
+// Flatten an array construction expression. Store the values into
+// temporaries in case they need interface conversion.
+
+Expression*
+Array_construction_expression::do_flatten(Gogo*, Named_object*,
+ Statement_inserter* inserter)
+{
+ if (this->vals_ == NULL)
+ return this;
+
+ // If this is a constant array, we don't need temporaries.
+ if (this->is_constant_array())
+ return this;
+
+ Location loc = this->location();
+ for (Expression_list::iterator pv = this->vals_->begin();
+ pv != this->vals_->end();
+ ++pv)
+ {
+ if (*pv != NULL)
+ {
+ if (!(*pv)->is_variable())
+ {
+ Temporary_statement* temp =
+ Statement::make_temporary(NULL, *pv, loc);
+ inserter->insert(temp);
+ *pv = Expression::make_temporary_reference(temp, loc);
+ }
+ }
+ }
+ return this;
+}
+
// Get a constructor expression for the array values.
Bexpression*
diff -r 47792391c17a go/gogo.cc
--- a/go/gogo.cc Wed Dec 17 06:26:14 2014 -0800
+++ b/go/gogo.cc Thu Dec 18 19:29:08 2014 -0800
@@ -5830,6 +5830,21 @@
gogo->flatten_expression(function, inserter, &this->init_);
+ // If an interface conversion is needed, we need a temporary
+ // variable.
+ if (this->type_ != NULL
+ && !Type::are_identical(this->type_, this->init_->type(), false,
+ NULL)
+ && this->init_->type()->interface_type() != NULL
+ && !this->init_->is_variable())
+ {
+ Temporary_statement* temp =
+ Statement::make_temporary(NULL, this->init_, this->location_);
+ inserter->insert(temp);
+ this->init_ = Expression::make_temporary_reference(temp,
+ this->location_);
+ }
+
this->seen_ = false;
this->init_is_flattened_ = true;
}
diff -r 47792391c17a go/statements.cc
--- a/go/statements.cc Wed Dec 17 06:26:14 2014 -0800
+++ b/go/statements.cc Thu Dec 18 19:29:08 2014 -0800
@@ -521,6 +521,9 @@
void
do_check_types(Gogo*);
+ Statement*
+ do_flatten(Gogo*, Named_object*, Block*, Statement_inserter*);
+
Bstatement*
do_get_backend(Translate_context*);
@@ -606,6 +609,28 @@
this->set_is_error();
}
+// Flatten an assignment statement. We may need a temporary for
+// interface conversion.
+
+Statement*
+Assignment_statement::do_flatten(Gogo*, Named_object*, Block*,
+ Statement_inserter* inserter)
+{
+ if (!this->lhs_->is_sink_expression()
+ && !Type::are_identical(this->lhs_->type(), this->rhs_->type(),
+ false, NULL)
+ && this->rhs_->type()->interface_type() != NULL
+ && !this->rhs_->is_variable())
+ {
+ Temporary_statement* temp =
+ Statement::make_temporary(NULL, this->rhs_, this->location());
+ inserter->insert(temp);
+ this->rhs_ = Expression::make_temporary_reference(temp,
+ this->location());
+ }
+ return this;
+}
+
// Convert an assignment statement to the backend representation.
Bstatement*
@@ -2480,12 +2505,13 @@
gogo->add_block(b, location);
gogo->lower_block(function, b);
- gogo->flatten_block(function, b);
// We already ran the determine_types pass, so we need to run it
// just for the call statement now. The other types are known.
call_statement->determine_types();
+ gogo->flatten_block(function, b);
+
if (may_call_recover || recover_arg != NULL)
{
// Dig up the call expression, which may have been changed
@@ -4412,6 +4438,27 @@
}
}
+// Flatten a send statement. We may need a temporary for interface
+// conversion.
+
+Statement*
+Send_statement::do_flatten(Gogo*, Named_object*, Block*,
+ Statement_inserter* inserter)
+{
+ Type* element_type = this->channel_->type()->channel_type()->element_type();
+ if (!Type::are_identical(element_type, this->val_->type(), false, NULL)
+ && this->val_->type()->interface_type() != NULL
+ && !this->val_->is_variable())
+ {
+ Temporary_statement* temp =
+ Statement::make_temporary(NULL, this->val_, this->location());
+ inserter->insert(temp);
+ this->val_ = Expression::make_temporary_reference(temp,
+ this->location());
+ }
+ return this;
+}
+
// Convert a send statement to the backend representation.
Bstatement*
@@ -4421,7 +4468,9 @@
Channel_type* channel_type = this->channel_->type()->channel_type();
Type* element_type = channel_type->element_type();
- Expression* val = Expression::make_cast(element_type, this->val_, loc);
+ Expression* val = Expression::convert_for_assignment(context->gogo(),
+ element_type,
+ this->val_, loc);
bool is_small;
bool can_take_address;
diff -r 47792391c17a go/statements.h
--- a/go/statements.h Wed Dec 17 06:26:14 2014 -0800
+++ b/go/statements.h Thu Dec 18 19:29:08 2014 -0800
@@ -704,6 +704,9 @@
void
do_check_types(Gogo*);
+ Statement*
+ do_flatten(Gogo*, Named_object*, Block*, Statement_inserter*);
+
Bstatement*
do_get_backend(Translate_context*);
