The Go rules for the order of evaluation in tuple assignments were clarified. In a statement like i, x[i] = 1, 2 the value of "i" used in "x[i]" must be the value that "i" had before the assignment statement, not the value it gets while evaluating the assignment. This patch implements that in the Go frontend. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 958b097c010b go/expressions.cc --- a/go/expressions.cc Mon Oct 24 12:42:28 2011 -0700 +++ b/go/expressions.cc Mon Oct 24 21:50:59 2011 -0700 @@ -3993,6 +3993,10 @@ } bool + do_must_eval_subexpressions_in_order(int*) const + { return this->op_ == OPERATOR_MULT; } + + bool do_is_addressable() const { return this->op_ == OPERATOR_MULT; } @@ -10037,6 +10041,13 @@ } bool + do_must_eval_subexpressions_in_order(int* skip) const + { + *skip = 1; + return true; + } + + bool do_is_addressable() const; void @@ -10461,6 +10472,13 @@ this->location()); } + bool + do_must_eval_subexpressions_in_order(int* skip) const + { + *skip = 1; + return true; + } + tree do_get_tree(Translate_context*); diff -r 958b097c010b go/expressions.h --- a/go/expressions.h Mon Oct 24 12:42:28 2011 -0700 +++ b/go/expressions.h Mon Oct 24 21:50:59 2011 -0700 @@ -582,6 +582,18 @@ must_eval_in_order() const { return this->do_must_eval_in_order(); } + // Return whether subexpressions of this expression must be + // evaluated in order. This is true of index expressions and + // pointer indirections. This sets *SKIP to the number of + // subexpressions to skip during traversing, as index expressions + // only requiring moving the index, not the array. + bool + must_eval_subexpressions_in_order(int* skip) const + { + *skip = 0; + return this->do_must_eval_subexpressions_in_order(skip); + } + // Return the tree for this expression. tree get_tree(Translate_context*); @@ -717,6 +729,13 @@ do_must_eval_in_order() const { return false; } + // Child class implements whether this expressions requires that + // subexpressions be evaluated in order. The child implementation + // may set *SKIP if it should be non-zero. + virtual bool + do_must_eval_subexpressions_in_order(int* /* skip */) const + { return false; } + // Child class implements conversion to tree. virtual tree do_get_tree(Translate_context*) = 0; @@ -1526,6 +1545,13 @@ this->location()); } + bool + do_must_eval_subexpressions_in_order(int* skip) const + { + *skip = 1; + return true; + } + void do_dump_expression(Ast_dump_context*) const; @@ -1623,6 +1649,13 @@ this->location()); } + bool + do_must_eval_subexpressions_in_order(int* skip) const + { + *skip = 1; + return true; + } + // A map index expression is an lvalue but it is not addressable. tree diff -r 958b097c010b go/statements.cc --- a/go/statements.cc Mon Oct 24 12:42:28 2011 -0700 +++ b/go/statements.cc Mon Oct 24 21:50:59 2011 -0700 @@ -652,6 +652,47 @@ return new Assignment_statement(lhs, rhs, location); } +// The Move_subexpressions class is used to move all top-level +// subexpressions of an expression. This is used for things like +// index expressions in which we must evaluate the index value before +// it can be changed by a multiple assignment. + +class Move_subexpressions : public Traverse +{ + public: + Move_subexpressions(int skip, Block* block) + : Traverse(traverse_expressions), + skip_(skip), block_(block) + { } + + protected: + int + expression(Expression**); + + private: + // The number of subexpressions to skip moving. This is used to + // avoid moving the array itself, as we only need to move the index. + int skip_; + // The block where new temporary variables should be added. + Block* block_; +}; + +int +Move_subexpressions::expression(Expression** pexpr) +{ + if (this->skip_ > 0) + --this->skip_; + else if ((*pexpr)->temporary_reference_expression() == NULL) + { + source_location loc = (*pexpr)->location(); + Temporary_statement* temp = Statement::make_temporary(NULL, *pexpr, loc); + this->block_->add_statement(temp); + *pexpr = Expression::make_temporary_reference(temp, loc); + } + // We only need to move top-level subexpressions. + return TRAVERSE_SKIP_COMPONENTS; +} + // The Move_ordered_evals class is used to find any subexpressions of // an expression that have an evaluation order dependency. It creates // temporary variables to hold them. @@ -679,6 +720,15 @@ // We have to look at subexpressions first. if ((*pexpr)->traverse_subexpressions(this) == TRAVERSE_EXIT) return TRAVERSE_EXIT; + + int i; + if ((*pexpr)->must_eval_subexpressions_in_order(&i)) + { + Move_subexpressions ms(i, this->block_); + if ((*pexpr)->traverse_subexpressions(&ms) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; + } + if ((*pexpr)->must_eval_in_order()) { source_location loc = (*pexpr)->location(); @@ -901,10 +951,10 @@ // First move out any subexpressions on the left hand side. The // right hand side will be evaluated in the required order anyhow. Move_ordered_evals moe(b); - for (Expression_list::const_iterator plhs = this->lhs_->begin(); + for (Expression_list::iterator plhs = this->lhs_->begin(); plhs != this->lhs_->end(); ++plhs) - (*plhs)->traverse_subexpressions(&moe); + Expression::traverse(&*plhs, &moe); std::vector<Temporary_statement*> temps; temps.reserve(this->lhs_->size());