From 47b6f9825f7867fcd91e3150ce6f95676ee3985d Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 19 Dec 2014 04:05:59 +0000 Subject: [PATCH] compiler: Avoid multiple evaluations in interface conversions. Added assertions for cases that might lead to multiple evaluations, and fixed all the problems I saw. Test case already in master Go testsuite (https://go-review.googlesource.com/#/c/1710/). From-SVN: r218884 --- gcc/go/gofrontend/expressions.cc | 104 ++++++++++++++++++++++++++++--- gcc/go/gofrontend/gogo.cc | 15 +++++ gcc/go/gofrontend/statements.cc | 53 +++++++++++++++- gcc/go/gofrontend/statements.h | 3 + 4 files changed, 164 insertions(+), 11 deletions(-) diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc index 83ee6d923bb..6f5acc1f92e 100644 --- a/gcc/go/gofrontend/expressions.cc +++ b/gcc/go/gofrontend/expressions.cc @@ -302,6 +302,9 @@ Expression::convert_interface_to_interface(Type *lhs_type, Expression* rhs, 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 @@ Expression::convert_interface_to_interface(Type *lhs_type, Expression* rhs, // 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* 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 @@ Type_conversion_expression::do_flatten(Gogo*, Named_object*, { 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 = @@ -3551,7 +3559,8 @@ Unsafe_type_conversion_expression::do_get_backend(Translate_context* context) || et->function_type() != NULL || et->points_to() != NULL || et->map_type() != NULL - || et->channel_type() != NULL); + || et->channel_type() != NULL + || et->is_nil_type()); else go_unreachable(); @@ -8782,12 +8791,17 @@ Call_expression::do_flatten(Gogo* gogo, Named_object*, 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 +11616,9 @@ class Struct_construction_expression : public Expression return ret; } + Expression* + do_flatten(Gogo*, Named_object*, Statement_inserter*); + Bexpression* do_get_backend(Translate_context*); @@ -11776,6 +11793,39 @@ Struct_construction_expression::do_check_types(Gogo*) 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 +11959,9 @@ protected: 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 +12077,39 @@ Array_construction_expression::do_check_types(Gogo*) } } +// 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 --git a/gcc/go/gofrontend/gogo.cc b/gcc/go/gofrontend/gogo.cc index d7f4437d4f4..49be6af61f2 100644 --- a/gcc/go/gofrontend/gogo.cc +++ b/gcc/go/gofrontend/gogo.cc @@ -5830,6 +5830,21 @@ Variable::flatten_init_expression(Gogo* gogo, Named_object* function, 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 --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc index e8c3a3e2702..c84df3b5a3e 100644 --- a/gcc/go/gofrontend/statements.cc +++ b/gcc/go/gofrontend/statements.cc @@ -521,6 +521,9 @@ class Assignment_statement : public Statement void do_check_types(Gogo*); + Statement* + do_flatten(Gogo*, Named_object*, Block*, Statement_inserter*); + Bstatement* do_get_backend(Translate_context*); @@ -606,6 +609,28 @@ Assignment_statement::do_check_types(Gogo*) 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 @@ Thunk_statement::build_thunk(Gogo* gogo, const std::string& thunk_name) 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 @@ Send_statement::do_check_types(Gogo*) } } +// 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 @@ Send_statement::do_get_backend(Translate_context* context) 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 --git a/gcc/go/gofrontend/statements.h b/gcc/go/gofrontend/statements.h index e12f60fdd30..aaad66e7fca 100644 --- a/gcc/go/gofrontend/statements.h +++ b/gcc/go/gofrontend/statements.h @@ -704,6 +704,9 @@ class Send_statement : public Statement void do_check_types(Gogo*); + Statement* + do_flatten(Gogo*, Named_object*, Block*, Statement_inserter*); + Bstatement* do_get_backend(Translate_context*); -- 2.30.2