This patch to the Go frontend by Than McIntosh fixes an
order-of-evaluation bug in slice/array composite literals. There was
a phase ordering issue in the handling of "keyed" array literal
expressions: the lowering phase was canonicalizing the indices/vals
before the phase that fixed evaluation order, meaning that the
evaluation order was incorrect. The fix is to capture the original
ordering and use that ordering when doing traversals (there is already
something similar being done for struct literal expressions).
This fixes https://golang.org/issue/17640.
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 241687)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-4ca21c94f00c620bfde2f924e214c78fe2e1ff69
+c353ffbe18d1538cac7f2a3fcefb846dbf1a6591
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc (revision 241667)
+++ gcc/go/gofrontend/expressions.cc (working copy)
@@ -12202,12 +12202,10 @@ Expression::make_allocation(Type* type,
return new Allocation_expression(type, location);
}
-// Class Struct_construction_expression.
-
-// Traversal.
+// Class Ordered_value_list.
int
-Struct_construction_expression::do_traverse(Traverse* traverse)
+Ordered_value_list::traverse_vals(Traverse* traverse)
{
if (this->vals_ != NULL)
{
@@ -12218,8 +12216,8 @@ Struct_construction_expression::do_trave
}
else
{
- for (std::vector<int>::const_iterator p =
- this->traverse_order_->begin();
+ for (std::vector<unsigned long>::const_iterator p =
+ this->traverse_order_->begin();
p != this->traverse_order_->end();
++p)
{
@@ -12229,6 +12227,18 @@ Struct_construction_expression::do_trave
}
}
}
+ return TRAVERSE_CONTINUE;
+}
+
+// Class Struct_construction_expression.
+
+// Traversal.
+
+int
+Struct_construction_expression::do_traverse(Traverse* traverse)
+{
+ if (this->traverse_vals(traverse) == TRAVERSE_EXIT)
+ return TRAVERSE_EXIT;
if (Type::traverse(this->type_, traverse) == TRAVERSE_EXIT)
return TRAVERSE_EXIT;
return TRAVERSE_CONTINUE;
@@ -12239,10 +12249,10 @@ Struct_construction_expression::do_trave
bool
Struct_construction_expression::is_constant_struct() const
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return true;
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL
@@ -12270,10 +12280,10 @@ Struct_construction_expression::is_const
bool
Struct_construction_expression::do_is_immutable() const
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return true;
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL && !(*pv)->is_immutable())
@@ -12287,15 +12297,15 @@ Struct_construction_expression::do_is_im
void
Struct_construction_expression::do_determine_type(const Type_context*)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return;
const Struct_field_list* fields = this->type_->struct_type()->fields();
- Expression_list::const_iterator pv = this->vals_->begin();
+ Expression_list::const_iterator pv = this->vals()->begin();
for (Struct_field_list::const_iterator pf = fields->begin();
pf != fields->end();
++pf, ++pv)
{
- if (pv == this->vals_->end())
+ if (pv == this->vals()->end())
return;
if (*pv != NULL)
{
@@ -12305,7 +12315,7 @@ Struct_construction_expression::do_deter
}
// Extra values are an error we will report elsewhere; we still want
// to determine the type to avoid knockon errors.
- for (; pv != this->vals_->end(); ++pv)
+ for (; pv != this->vals()->end(); ++pv)
(*pv)->determine_type_no_context();
}
@@ -12314,24 +12324,24 @@ Struct_construction_expression::do_deter
void
Struct_construction_expression::do_check_types(Gogo*)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return;
Struct_type* st = this->type_->struct_type();
- if (this->vals_->size() > st->field_count())
+ if (this->vals()->size() > st->field_count())
{
this->report_error(_("too many expressions for struct"));
return;
}
const Struct_field_list* fields = st->fields();
- Expression_list::const_iterator pv = this->vals_->begin();
+ Expression_list::const_iterator pv = this->vals()->begin();
int i = 0;
for (Struct_field_list::const_iterator pf = fields->begin();
pf != fields->end();
++pf, ++pv, ++i)
{
- if (pv == this->vals_->end())
+ if (pv == this->vals()->end())
{
this->report_error(_("too few expressions for struct"));
break;
@@ -12355,7 +12365,7 @@ Struct_construction_expression::do_check
this->set_is_error();
}
}
- go_assert(pv == this->vals_->end());
+ go_assert(pv == this->vals()->end());
}
// Flatten a struct construction expression. Store the values into
@@ -12365,7 +12375,7 @@ Expression*
Struct_construction_expression::do_flatten(Gogo*, Named_object*,
Statement_inserter* inserter)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return this;
// If this is a constant struct, we don't need temporaries.
@@ -12373,8 +12383,8 @@ Struct_construction_expression::do_flatt
return this;
Location loc = this->location();
- for (Expression_list::iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL)
@@ -12404,18 +12414,18 @@ Struct_construction_expression::do_get_b
Gogo* gogo = context->gogo();
Btype* btype = this->type_->get_backend(gogo);
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return gogo->backend()->zero_expression(btype);
const Struct_field_list* fields = this->type_->struct_type()->fields();
- Expression_list::const_iterator pv = this->vals_->begin();
+ Expression_list::const_iterator pv = this->vals()->begin();
std::vector<Bexpression*> init;
for (Struct_field_list::const_iterator pf = fields->begin();
pf != fields->end();
++pf)
{
Btype* fbtype = pf->type()->get_backend(gogo);
- if (pv == this->vals_->end())
+ if (pv == this->vals()->end())
init.push_back(gogo->backend()->zero_expression(fbtype));
else if (*pv == NULL)
{
@@ -12441,8 +12451,8 @@ Struct_construction_expression::do_expor
{
exp->write_c_string("convert(");
exp->write_type(this->type_);
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
exp->write_c_string(", ");
@@ -12460,7 +12470,7 @@ Struct_construction_expression::do_dump_
{
ast_dump_context->dump_type(this->type_);
ast_dump_context->ostream() << "{";
- ast_dump_context->dump_expression_list(this->vals_);
+ ast_dump_context->dump_expression_list(this->vals());
ast_dump_context->ostream() << "}";
}
@@ -12481,8 +12491,7 @@ Expression::make_struct_composite_litera
int
Array_construction_expression::do_traverse(Traverse* traverse)
{
- if (this->vals_ != NULL
- && this->vals_->traverse(traverse) == TRAVERSE_EXIT)
+ if (this->traverse_vals(traverse) == TRAVERSE_EXIT)
return TRAVERSE_EXIT;
if (Type::traverse(this->type_, traverse) == TRAVERSE_EXIT)
return TRAVERSE_EXIT;
@@ -12494,15 +12503,15 @@ Array_construction_expression::do_traver
bool
Array_construction_expression::is_constant_array() const
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return true;
// There are no constant constructors for interfaces.
if (this->type_->array_type()->element_type()->interface_type() != NULL)
return false;
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL
@@ -12519,10 +12528,10 @@ Array_construction_expression::is_consta
bool
Array_construction_expression::do_is_immutable() const
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return true;
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL && !(*pv)->is_immutable())
@@ -12536,11 +12545,11 @@ Array_construction_expression::do_is_imm
void
Array_construction_expression::do_determine_type(const Type_context*)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return;
Type_context subcontext(this->type_->array_type()->element_type(), false);
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL)
@@ -12553,14 +12562,14 @@ Array_construction_expression::do_determ
void
Array_construction_expression::do_check_types(Gogo*)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return;
Array_type* at = this->type_->array_type();
int i = 0;
Type* element_type = at->element_type();
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv, ++i)
{
if (*pv != NULL
@@ -12581,7 +12590,7 @@ Expression*
Array_construction_expression::do_flatten(Gogo*, Named_object*,
Statement_inserter* inserter)
{
- if (this->vals_ == NULL)
+ if (this->vals() == NULL)
return this;
// If this is a constant array, we don't need temporaries.
@@ -12589,8 +12598,8 @@ Array_construction_expression::do_flatte
return this;
Location loc = this->location();
- for (Expression_list::iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
if (*pv != NULL)
@@ -12623,14 +12632,14 @@ Array_construction_expression::get_const
std::vector<unsigned long> indexes;
std::vector<Bexpression*> vals;
Gogo* gogo = context->gogo();
- if (this->vals_ != NULL)
+ if (this->vals() != NULL)
{
size_t i = 0;
std::vector<unsigned long>::const_iterator pi;
if (this->indexes_ != NULL)
pi = this->indexes_->begin();
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv, ++i)
{
if (this->indexes_ != NULL)
@@ -12670,13 +12679,13 @@ Array_construction_expression::do_export
{
exp->write_c_string("convert(");
exp->write_type(this->type_);
- if (this->vals_ != NULL)
+ if (this->vals() != NULL)
{
std::vector<unsigned long>::const_iterator pi;
if (this->indexes_ != NULL)
pi = this->indexes_->begin();
- for (Expression_list::const_iterator pv = this->vals_->begin();
- pv != this->vals_->end();
+ for (Expression_list::const_iterator pv = this->vals()->begin();
+ pv != this->vals()->end();
++pv)
{
exp->write_c_string(", ");
@@ -12717,10 +12726,10 @@ Array_construction_expression::do_dump_e
this->dump_slice_storage_expression(ast_dump_context);
ast_dump_context->ostream() << "{" ;
if (this->indexes_ == NULL)
- ast_dump_context->dump_expression_list(this->vals_);
+ ast_dump_context->dump_expression_list(this->vals());
else
{
- Expression_list::const_iterator pv = this->vals_->begin();
+ Expression_list::const_iterator pv = this->vals()->begin();
for (std::vector<unsigned long>::const_iterator pi =
this->indexes_->begin();
pi != this->indexes_->end();
@@ -13349,7 +13358,7 @@ Composite_literal_expression::lower_stru
size_t field_count = st->field_count();
std::vector<Expression*> vals(field_count);
- std::vector<int>* traverse_order = new(std::vector<int>);
+ std::vector<unsigned long>* traverse_order = new(std::vector<unsigned long>);
Expression_list::const_iterator p = this->vals_->begin();
Expression* external_expr = NULL;
const Named_object* external_no = NULL;
@@ -13481,7 +13490,7 @@ Composite_literal_expression::lower_stru
type->named_type()->message_name().c_str());
vals[index] = val;
- traverse_order->push_back(index);
+ traverse_order->push_back(static_cast<unsigned long>(index));
}
if (!this->all_are_names_)
@@ -13512,15 +13521,16 @@ Composite_literal_expression::lower_stru
return ret;
}
-// Used to sort an index/value array.
+// Index/value/traversal-order triple.
-class Index_value_compare
-{
- public:
- bool
- operator()(const std::pair<unsigned long, Expression*>& a,
- const std::pair<unsigned long, Expression*>& b)
- { return a.first < b.first; }
+struct IVT_triple {
+ unsigned long index;
+ unsigned long traversal_order;
+ Expression* expr;
+ IVT_triple(unsigned long i, unsigned long to, Expression *e)
+ : index(i), traversal_order(to), expr(e) { }
+ bool operator<(const IVT_triple& other) const
+ { return this->index < other.index; }
};
// Lower an array composite literal.
@@ -13624,35 +13634,45 @@ Composite_literal_expression::lower_arra
indexes = NULL;
}
+ std::vector<unsigned long>* traverse_order = NULL;
if (indexes_out_of_order)
{
- typedef std::vector<std::pair<unsigned long, Expression*> > V;
+ typedef std::vector<IVT_triple> V;
V v;
v.reserve(indexes->size());
std::vector<unsigned long>::const_iterator pi = indexes->begin();
+ unsigned long torder = 0;
for (Expression_list::const_iterator pe = vals->begin();
pe != vals->end();
- ++pe, ++pi)
- v.push_back(std::make_pair(*pi, *pe));
+ ++pe, ++pi, ++torder)
+ v.push_back(IVT_triple(*pi, torder, *pe));
- std::sort(v.begin(), v.end(), Index_value_compare());
+ std::sort(v.begin(), v.end());
delete indexes;
delete vals;
+
indexes = new std::vector<unsigned long>();
indexes->reserve(v.size());
vals = new Expression_list();
vals->reserve(v.size());
+ traverse_order = new std::vector<unsigned long>();
+ traverse_order->reserve(v.size());
for (V::const_iterator p = v.begin(); p != v.end(); ++p)
{
- indexes->push_back(p->first);
- vals->push_back(p->second);
+ indexes->push_back(p->index);
+ vals->push_back(p->expr);
+ traverse_order->push_back(p->traversal_order);
}
}
- return this->make_array(type, indexes, vals);
+ Expression* ret = this->make_array(type, indexes, vals);
+ Array_construction_expression* ace = ret->array_literal();
+ if (ace != NULL && traverse_order != NULL)
+ ace->set_traverse_order(traverse_order);
+ return ret;
}
// Actually build the array composite literal. This handles
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h (revision 241341)
+++ gcc/go/gofrontend/expressions.h (working copy)
@@ -3232,31 +3232,64 @@ class Composite_literal_expression : pub
std::vector<bool> key_path_;
};
-// Construct a struct.
+// Helper/mixin class for struct and array construction expressions;
+// encapsulates a list of values plus an optional traversal order
+// recording the order in which the values should be visited.
-class Struct_construction_expression : public Expression
+class Ordered_value_list
{
public:
- Struct_construction_expression(Type* type, Expression_list* vals,
- Location location)
- : Expression(EXPRESSION_STRUCT_CONSTRUCTION, location),
- type_(type), vals_(vals), traverse_order_(NULL)
+ Ordered_value_list(Expression_list* vals)
+ : vals_(vals), traverse_order_(NULL)
{ }
+ Expression_list*
+ vals() const
+ { return this->vals_; }
+
+ int
+ traverse_vals(Traverse* traverse);
+
+ // Get the traversal order (may be NULL)
+ std::vector<unsigned long>*
+ traverse_order()
+ { return traverse_order_; }
+
// Set the traversal order, used to ensure that we implement the
// order of evaluation rules. Takes ownership of the argument.
void
- set_traverse_order(std::vector<int>* traverse_order)
+ set_traverse_order(std::vector<unsigned long>* traverse_order)
{ this->traverse_order_ = traverse_order; }
- // Return whether this is a constant initializer.
+ private:
+ // The list of values, in order of the fields in the struct or in
+ // order of indices in an array. A NULL value of vals_ means that
+ // all fields/slots should be zero-initialized; a single NULL entry
+ // in the list means that the corresponding field or array slot
+ // should be zero-initialized.
+ Expression_list* vals_;
+ // If not NULL, the order in which to traverse vals_. This is used
+ // so that we implement the order of evaluation rules correctly.
+ std::vector<unsigned long>* traverse_order_;
+};
+
+// Construct a struct.
+
+class Struct_construction_expression : public Expression,
+ public Ordered_value_list
+{
+ public:
+ Struct_construction_expression(Type* type, Expression_list* vals,
+ Location location)
+ : Expression(EXPRESSION_STRUCT_CONSTRUCTION, location),
+ Ordered_value_list(vals),
+ type_(type)
+ { }
+
+ // Return whether this is a constant initializer.
bool
is_constant_struct() const;
- Expression_list*
- vals() const
- { return this->vals_; }
-
protected:
int
do_traverse(Traverse* traverse);
@@ -3279,12 +3312,12 @@ class Struct_construction_expression : p
{
Struct_construction_expression* ret =
new Struct_construction_expression(this->type_,
- (this->vals_ == NULL
+ (this->vals() == NULL
? NULL
- : this->vals_->copy()),
+ : this->vals()->copy()),
this->location());
- if (this->traverse_order_ != NULL)
- ret->set_traverse_order(this->traverse_order_);
+ if (this->traverse_order() != NULL)
+ ret->set_traverse_order(this->traverse_order());
return ret;
}
@@ -3303,19 +3336,14 @@ class Struct_construction_expression : p
private:
// The type of the struct to construct.
Type* type_;
- // The list of values, in order of the fields in the struct. A NULL
- // entry means that the field should be zero-initialized.
- Expression_list* vals_;
- // If not NULL, the order in which to traverse vals_. This is used
- // so that we implement the order of evaluation rules correctly.
- std::vector<int>* traverse_order_;
};
// Construct an array. This class is not used directly; instead we
// use the child classes, Fixed_array_construction_expression and
// Slice_construction_expression.
-class Array_construction_expression : public Expression
+class Array_construction_expression : public Expression,
+ public Ordered_value_list
{
protected:
Array_construction_expression(Expression_classification classification,
@@ -3323,7 +3351,8 @@ class Array_construction_expression : pu
const std::vector<unsigned long>* indexes,
Expression_list* vals, Location location)
: Expression(classification, location),
- type_(type), indexes_(indexes), vals_(vals)
+ Ordered_value_list(vals),
+ type_(type), indexes_(indexes)
{ go_assert(indexes == NULL || indexes->size() == vals->size()); }
public:
@@ -3334,12 +3363,7 @@ class Array_construction_expression : pu
// Return the number of elements.
size_t
element_count() const
- { return this->vals_ == NULL ? 0 : this->vals_->size(); }
-
- // The list of values.
- Expression_list*
- vals() const
- { return this->vals_; }
+ { return this->vals() == NULL ? 0 : this->vals()->size(); }
protected:
virtual int
@@ -3385,8 +3409,6 @@ protected:
// The list of indexes into the array, one for each value. This may
// be NULL, in which case the indexes start at zero and increment.
const std::vector<unsigned long>* indexes_;
- // The list of values. This may be NULL if there are no values.
- Expression_list* vals_;
};
// Construct a fixed array.
