[gcc r15-8600] gccrs: cleanup our enum type layout to be closer to rustc
Arthur Cohen
cohenarthur@gcc.gnu.org
Fri Mar 21 11:57:49 GMT 2025
https://gcc.gnu.org/g:f821a513843baabed8a39601bbc714558110b748
commit r15-8600-gf821a513843baabed8a39601bbc714558110b748
Author: Philip Herron <herron.philip@googlemail.com>
Date: Tue Jan 7 18:15:37 2025 +0000
gccrs: cleanup our enum type layout to be closer to rustc
This changes our enum type layout so for example:
enum Foo {
A,
B,
C(char),
D { x: i32, y: i32 },
}
Used to get layed out like this in gccrs:
union {
struct A { int RUST$ENUM$DISR; };
struct B { int RUST$ENUM$DISR; };
struct C { int RUST$ENUM$DISR; char __0; };
struct D { int RUST$ENUM$DISR; i64 x; i64 y; };
}
This has some issues notably with the constexpr because this is just a
giant union it means its not simple to constify what enum variant we are
looking at because the discriminant is a mess.
This now gets layed out as:
struct {
int RUST$ENUM$DISR;
union {
struct A { };
struct B { };
struct C { char __0; };
struct D { i64 x; i64 y; };
} payload;
}
This layout is much cleaner and allows for our constexpr to work properly.
gcc/rust/ChangeLog:
* backend/rust-compile-expr.cc (CompileExpr::visit): new layout
* backend/rust-compile-pattern.cc (CompilePatternCheckExpr::visit): likewise
(CompilePatternBindings::visit): likewise
* backend/rust-compile-resolve-path.cc: likewise
* backend/rust-compile-type.cc (TyTyResolveCompile::visit): implement new layout
* rust-gcc.cc (constructor_expression): get rid of useless assert
Signed-off-by: Philip Herron <herron.philip@googlemail.com>
Diff:
---
gcc/rust/backend/rust-compile-expr.cc | 74 ++++++++++++++++-----------
gcc/rust/backend/rust-compile-pattern.cc | 64 +++++++++++------------
gcc/rust/backend/rust-compile-resolve-path.cc | 5 +-
gcc/rust/backend/rust-compile-type.cc | 62 +++++++++++++++++-----
gcc/rust/rust-gcc.cc | 2 +-
5 files changed, 128 insertions(+), 79 deletions(-)
diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index 900e080ea0ef..b40aa33866ea 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -558,24 +558,32 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr)
}
}
- // the constructor depends on whether this is actually an enum or not if
- // its an enum we need to setup the discriminator
- std::vector<tree> ctor_arguments;
- if (adt->is_enum ())
+ if (!adt->is_enum ())
{
- HIR::Expr &discrim_expr = variant->get_discriminant ();
- tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
- tree folded_discrim_expr = fold_expr (discrim_expr_node);
- tree qualifier = folded_discrim_expr;
-
- ctor_arguments.push_back (qualifier);
+ translated
+ = Backend::constructor_expression (compiled_adt_type, adt->is_enum (),
+ arguments, union_disriminator,
+ struct_expr.get_locus ());
+ return;
}
- for (auto &arg : arguments)
- ctor_arguments.push_back (arg);
+
+ HIR::Expr &discrim_expr = variant->get_discriminant ();
+ tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
+ tree folded_discrim_expr = fold_expr (discrim_expr_node);
+ tree qualifier = folded_discrim_expr;
+
+ tree enum_root_files = TYPE_FIELDS (compiled_adt_type);
+ tree payload_root = DECL_CHAIN (enum_root_files);
+
+ tree payload = Backend::constructor_expression (TREE_TYPE (payload_root),
+ adt->is_enum (), arguments,
+ union_disriminator,
+ struct_expr.get_locus ());
+
+ std::vector<tree> ctor_arguments = {qualifier, payload};
translated
- = Backend::constructor_expression (compiled_adt_type, adt->is_enum (),
- ctor_arguments, union_disriminator,
+ = Backend::constructor_expression (compiled_adt_type, 0, ctor_arguments, -1,
struct_expr.get_locus ());
}
@@ -1247,26 +1255,34 @@ CompileExpr::visit (HIR::CallExpr &expr)
arguments.push_back (rvalue);
}
- // the constructor depends on whether this is actually an enum or not if
- // its an enum we need to setup the discriminator
- std::vector<tree> ctor_arguments;
- if (adt->is_enum ())
+ if (!adt->is_enum ())
{
- HIR::Expr &discrim_expr = variant->get_discriminant ();
- tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
- tree folded_discrim_expr = fold_expr (discrim_expr_node);
- tree qualifier = folded_discrim_expr;
-
- ctor_arguments.push_back (qualifier);
+ translated
+ = Backend::constructor_expression (compiled_adt_type,
+ adt->is_enum (), arguments,
+ union_disriminator,
+ expr.get_locus ());
+ return;
}
- for (auto &arg : arguments)
- ctor_arguments.push_back (arg);
- translated
- = Backend::constructor_expression (compiled_adt_type, adt->is_enum (),
- ctor_arguments, union_disriminator,
+ HIR::Expr &discrim_expr = variant->get_discriminant ();
+ tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
+ tree folded_discrim_expr = fold_expr (discrim_expr_node);
+ tree qualifier = folded_discrim_expr;
+
+ tree enum_root_files = TYPE_FIELDS (compiled_adt_type);
+ tree payload_root = DECL_CHAIN (enum_root_files);
+
+ tree payload
+ = Backend::constructor_expression (TREE_TYPE (payload_root), true,
+ {arguments}, union_disriminator,
expr.get_locus ());
+ std::vector<tree> ctor_arguments = {qualifier, payload};
+ translated = Backend::constructor_expression (compiled_adt_type, false,
+ ctor_arguments, -1,
+ expr.get_locus ());
+
return;
}
diff --git a/gcc/rust/backend/rust-compile-pattern.cc b/gcc/rust/backend/rust-compile-pattern.cc
index bd534ff3befb..4e352fd3da3d 100644
--- a/gcc/rust/backend/rust-compile-pattern.cc
+++ b/gcc/rust/backend/rust-compile-pattern.cc
@@ -21,6 +21,7 @@
#include "rust-compile-resolve-path.h"
#include "rust-constexpr.h"
#include "rust-compile-type.h"
+#include "print-tree.h"
namespace Rust {
namespace Compile {
@@ -57,11 +58,8 @@ CompilePatternCheckExpr::visit (HIR::PathInExpression &pattern)
rust_assert (ok);
// find discriminant field of scrutinee
- tree scrutinee_record_expr
- = Backend::struct_field_expression (match_scrutinee_expr, 0,
- pattern.get_locus ());
tree scrutinee_expr_qualifier_expr
- = Backend::struct_field_expression (scrutinee_record_expr, 0,
+ = Backend::struct_field_expression (match_scrutinee_expr, 0,
pattern.get_locus ());
// must be enum
@@ -227,11 +225,8 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern)
tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
// find discriminant field of scrutinee
- tree scrutinee_record_expr
- = Backend::struct_field_expression (match_scrutinee_expr, variant_index,
- pattern.get_path ().get_locus ());
tree scrutinee_expr_qualifier_expr
- = Backend::struct_field_expression (scrutinee_record_expr, 0,
+ = Backend::struct_field_expression (match_scrutinee_expr, 0,
pattern.get_path ().get_locus ());
check_expr
@@ -240,7 +235,7 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern)
discrim_expr_node,
pattern.get_path ().get_locus ());
- match_scrutinee_expr = scrutinee_record_expr;
+ match_scrutinee_expr = scrutinee_expr_qualifier_expr;
}
else
{
@@ -295,8 +290,6 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern)
void
CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
{
- size_t tuple_field_index;
-
// lookup the type
TyTy::BaseType *lookup = nullptr;
bool ok = ctx->get_tyctx ()->lookup_type (
@@ -307,6 +300,7 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
rust_assert (lookup->get_kind () == TyTy::TypeKind::ADT);
TyTy::ADTType *adt = static_cast<TyTy::ADTType *> (lookup);
+ int variant_index = 0;
rust_assert (adt->number_of_variants () > 0);
TyTy::VariantDef *variant = nullptr;
if (adt->is_enum ())
@@ -317,7 +311,6 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
pattern.get_path ().get_mappings ().get_hirid (), &variant_id);
rust_assert (ok);
- int variant_index = 0;
ok = adt->lookup_variant_by_id (variant_id, &variant, &variant_index);
rust_assert (ok);
@@ -326,11 +319,8 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx);
// find discriminant field of scrutinee
- tree scrutinee_record_expr
- = Backend::struct_field_expression (match_scrutinee_expr, variant_index,
- pattern.get_path ().get_locus ());
tree scrutinee_expr_qualifier_expr
- = Backend::struct_field_expression (scrutinee_record_expr, 0,
+ = Backend::struct_field_expression (match_scrutinee_expr, 0,
pattern.get_path ().get_locus ());
check_expr
@@ -338,17 +328,11 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
scrutinee_expr_qualifier_expr,
discrim_expr_node,
pattern.get_path ().get_locus ());
-
- match_scrutinee_expr = scrutinee_record_expr;
- // we are offsetting by + 1 here since the first field in the record
- // is always the discriminator
- tuple_field_index = 1;
}
else
{
variant = adt->get_variants ().at (0);
check_expr = boolean_true_node;
- tuple_field_index = 0;
}
HIR::TupleStructItems &items = pattern.get_items ();
@@ -367,10 +351,20 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
rust_assert (items_no_range.get_patterns ().size ()
== variant->num_fields ());
+ size_t tuple_field_index = 0;
for (auto &pattern : items_no_range.get_patterns ())
{
+ // find payload union field of scrutinee
+ tree payload_ref
+ = Backend::struct_field_expression (match_scrutinee_expr, 1,
+ pattern->get_locus ());
+
+ tree variant_ref
+ = Backend::struct_field_expression (payload_ref, variant_index,
+ pattern->get_locus ());
+
tree field_expr
- = Backend::struct_field_expression (match_scrutinee_expr,
+ = Backend::struct_field_expression (variant_ref,
tuple_field_index++,
pattern->get_locus ());
@@ -470,13 +464,15 @@ CompilePatternBindings::visit (HIR::TupleStructPattern &pattern)
if (adt->is_enum ())
{
- // we are offsetting by + 1 here since the first field in the record
- // is always the discriminator
- size_t tuple_field_index = 1;
+ size_t tuple_field_index = 0;
for (auto &pattern : items_no_range.get_patterns ())
{
+ tree payload_accessor_union
+ = Backend::struct_field_expression (match_scrutinee_expr, 1,
+ pattern->get_locus ());
+
tree variant_accessor
- = Backend::struct_field_expression (match_scrutinee_expr,
+ = Backend::struct_field_expression (payload_accessor_union,
variant_index,
pattern->get_locus ());
@@ -569,16 +565,18 @@ CompilePatternBindings::visit (HIR::StructPattern &pattern)
tree binding = error_mark_node;
if (adt->is_enum ())
{
+ tree payload_accessor_union
+ = Backend::struct_field_expression (match_scrutinee_expr, 1,
+ ident.get_locus ());
+
tree variant_accessor
- = Backend::struct_field_expression (match_scrutinee_expr,
+ = Backend::struct_field_expression (payload_accessor_union,
variant_index,
ident.get_locus ());
- // we are offsetting by + 1 here since the first field in the
- // record is always the discriminator
- binding = Backend::struct_field_expression (variant_accessor,
- offs + 1,
- ident.get_locus ());
+ binding
+ = Backend::struct_field_expression (variant_accessor, offs,
+ ident.get_locus ());
}
else
{
diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc
index 31ebbf14b74e..69599cc6185a 100644
--- a/gcc/rust/backend/rust-compile-resolve-path.cc
+++ b/gcc/rust/backend/rust-compile-resolve-path.cc
@@ -86,8 +86,9 @@ ResolvePathRef::attempt_constructor_expression_lookup (
tree folded_discrim_expr = fold_expr (discrim_expr_node);
tree qualifier = folded_discrim_expr;
- return Backend::constructor_expression (compiled_adt_type, true, {qualifier},
- union_disriminator, expr_locus);
+ // false for is enum but this is an enum but we have a new layout
+ return Backend::constructor_expression (compiled_adt_type, false, {qualifier},
+ -1, expr_locus);
}
tree
diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc
index 50b52fbd37fa..85e786583353 100644
--- a/gcc/rust/backend/rust-compile-type.cc
+++ b/gcc/rust/backend/rust-compile-type.cc
@@ -298,21 +298,39 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type)
// Ada, qual_union_types might still work for this but I am not 100% sure.
// I ran into some issues lets reuse our normal union and ask Ada people
// about it.
+ //
+ // I think the above is actually wrong and it should actually be this
+ //
+ // struct {
+ // int RUST$ENUM$DISR; // take into account the repr for this TODO
+ // union {
+ // // Variant A
+ // struct {
+ // // No additional fields
+ // } A;
+
+ // // Variant B
+ // struct {
+ // // No additional fields
+ // } B;
+
+ // // Variant C
+ // struct {
+ // char c;
+ // } C;
+
+ // // Variant D
+ // struct {
+ // int64_t x;
+ // int64_t y;
+ // } D;
+ // } payload; // The union of all variant data
+ // };
std::vector<tree> variant_records;
for (auto &variant : type.get_variants ())
{
std::vector<Backend::typed_identifier> fields;
-
- // add in the qualifier field for the variant
- tree enumeral_type
- = TyTyResolveCompile::get_implicit_enumeral_node_type ();
- Backend::typed_identifier f (RUST_ENUM_DISR_FIELD_NAME, enumeral_type,
- ctx->get_mappings ().lookup_location (
- variant->get_id ()));
- fields.push_back (std::move (f));
-
- // compile the rest of the fields
for (size_t i = 0; i < variant->num_fields (); i++)
{
const TyTy::StructFieldType *field
@@ -336,9 +354,6 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type)
= Backend::named_type (variant->get_ident ().path.get (),
variant_record, variant->get_ident ().locus);
- // set the qualifier to be a builtin
- DECL_ARTIFICIAL (TYPE_FIELDS (variant_record)) = 1;
-
// add them to the list
variant_records.push_back (named_variant_record);
}
@@ -359,8 +374,27 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type)
enum_fields.push_back (std::move (f));
}
+ //
+ location_t locus = ctx->get_mappings ().lookup_location (type.get_ref ());
+
// finally make the union or the enum
- type_record = Backend::union_type (enum_fields, false);
+ tree variants_union = Backend::union_type (enum_fields, false);
+ layout_type (variants_union);
+ tree named_union_record
+ = Backend::named_type ("payload", variants_union, locus);
+
+ // create the overall struct
+ tree enumeral_type
+ = TyTyResolveCompile::get_implicit_enumeral_node_type ();
+ Backend::typed_identifier discrim (RUST_ENUM_DISR_FIELD_NAME,
+ enumeral_type, locus);
+ Backend::typed_identifier variants_union_field ("payload",
+ named_union_record,
+ locus);
+
+ std::vector<Backend::typed_identifier> fields
+ = {discrim, variants_union_field};
+ type_record = Backend::struct_type (fields, false);
}
// Handle repr options
diff --git a/gcc/rust/rust-gcc.cc b/gcc/rust/rust-gcc.cc
index 37d51e58c08e..a6e8ea904a8a 100644
--- a/gcc/rust/rust-gcc.cc
+++ b/gcc/rust/rust-gcc.cc
@@ -1352,7 +1352,7 @@ constructor_expression (tree type_tree, bool is_variant,
if (!TREE_CONSTANT (elt->value))
is_constant = false;
}
- gcc_assert (field == NULL_TREE);
+ // gcc_assert (field == NULL_TREE);
}
}
More information about the Gcc-cvs
mailing list