Skip to content

Commit b4a862e

Browse files
committed
Fix evaluation order in struct field expressions
Fixes #4445 gcc/rust/ChangeLog: * backend/rust-compile-expr.cc (CompileExpr::visit): Get struct field type with field name * typecheck/rust-hir-type-check-struct.cc (TypeCheckStructExpr::resolve): Cancel reordering of struct field exprs Signed-off-by: Mohamed El Shorbagy <mohrizq895@gmail.com>
1 parent 461ab85 commit b4a862e

2 files changed

Lines changed: 45 additions & 26 deletions

File tree

gcc/rust/backend/rust-compile-expr.cc

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -559,10 +559,8 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr)
559559
rust_assert (ok);
560560
}
561561

562-
// compile it
563-
tree compiled_adt_type = TyTyResolveCompile::compile (ctx, tyty);
564-
565562
std::vector<tree> arguments;
563+
566564
if (adt->is_union ())
567565
{
568566
rust_assert (struct_expr.get_fields ().size () == 1);
@@ -595,17 +593,42 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr)
595593
}
596594
else
597595
{
598-
// this assumes all fields are in order from type resolution and if a
599-
// base struct was specified those fields are filed via accessors
596+
std::vector<TyTy::StructFieldType *> ordered_variant_fields;
597+
600598
for (size_t i = 0; i < struct_expr.get_fields ().size (); i++)
601599
{
600+
std::string field_name;
601+
const auto &argument = struct_expr.get_fields ().at (i);
602+
switch (argument->get_kind ())
603+
{
604+
case HIR::StructExprField::StructExprFieldKind::IDENTIFIER:
605+
field_name
606+
= (static_cast<HIR::StructExprFieldIdentifier &> (*argument))
607+
.get_field_name ();
608+
break;
609+
610+
case HIR::StructExprField::StructExprFieldKind::IDENTIFIER_VALUE:
611+
field_name = (static_cast<HIR::StructExprFieldIdentifierValue &> (
612+
*argument))
613+
.get_field_name ();
614+
break;
615+
616+
case HIR::StructExprField::StructExprFieldKind::INDEX_VALUE:
617+
field_name = std::to_string (
618+
(static_cast<HIR::StructExprFieldIndexValue &> (*argument))
619+
.get_tuple_index ());
620+
break;
621+
}
602622
// assignments are coercion sites so lets convert the rvalue if
603623
// necessary
604-
auto respective_field = variant->get_field_at_index (i);
624+
TyTy::StructFieldType *respective_field = nullptr;
625+
auto _ok
626+
= variant->lookup_field (field_name, &respective_field, nullptr);
627+
rust_assert (_ok);
628+
629+
ordered_variant_fields.push_back (respective_field);
605630
auto expected = respective_field->get_field_type ();
606631

607-
// process arguments
608-
auto &argument = struct_expr.get_fields ().at (i);
609632
auto lvalue_locus
610633
= ctx->get_mappings ().lookup_location (expected->get_ty_ref ());
611634
auto rvalue_locus = argument->get_locus ();
@@ -616,7 +639,7 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr)
616639
argument->get_mappings ().get_hirid (), &actual);
617640

618641
// coerce it if required/possible see
619-
// compile/torture/struct_base_init_1.rs
642+
// compile/torture/struct_base_init.rs
620643
if (ok)
621644
{
622645
rvalue
@@ -627,8 +650,21 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr)
627650
// add it to the list
628651
arguments.push_back (rvalue);
629652
}
653+
// We evaluate field expressions in the order they are found, not in the
654+
// order of their declaration, so we reorder the field types according to
655+
// their values and use this correspondence to compile field expressions
656+
// and the ADT type itself. Note that we do not overwrite the fields in
657+
// the original ADT variant type because if the struct fields have a base
658+
// struct, they will be evaluated in the order of the base struct's
659+
// evaluation, which is incorrect.
660+
auto _adt = static_cast<TyTy::ADTType *> (adt->clone ());
661+
_adt->get_variants ()[0]->get_fields () = ordered_variant_fields;
662+
tyty = _adt;
630663
}
631664

665+
// compile it
666+
tree compiled_adt_type = TyTyResolveCompile::compile (ctx, tyty);
667+
632668
if (!adt->is_enum ())
633669
{
634670
translated

gcc/rust/typecheck/rust-hir-type-check-struct.cc

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -235,23 +235,6 @@ TypeCheckStructExpr::resolve (HIR::StructExprStructFields &struct_expr)
235235
}
236236
rust_assert (struct_expr.union_index != -1);
237237
}
238-
else
239-
{
240-
// everything is ok, now we need to ensure all field values are ordered
241-
// correctly. The GIMPLE backend uses a simple algorithm that assumes each
242-
// assigned field in the constructor is in the same order as the field in
243-
// the type
244-
for (auto &field : struct_expr.get_fields ())
245-
field.release ();
246-
247-
std::vector<std::unique_ptr<HIR::StructExprField> > ordered_fields;
248-
ordered_fields.reserve (adtFieldIndexToField.size ());
249-
250-
for (size_t i = 0; i < adtFieldIndexToField.size (); i++)
251-
ordered_fields.emplace_back (adtFieldIndexToField[i]);
252-
253-
struct_expr.set_fields_as_owner (std::move (ordered_fields));
254-
}
255238

256239
resolved = struct_def;
257240
}

0 commit comments

Comments
 (0)