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 --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index 900e080ea0efd1dcb4bf9aa62e90e9efd8165863..b40aa33866eade870f17dac409f452947d6e69ce 100644 (file)
--- 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 bd534ff3befb0b7e04d9bbb6a4c4e4f5cfe84b18..4e352fd3da3d3d6285bdbe3abb02e07092ca8d24 100644 (file)
--- 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 31ebbf14b74e338f528c3a9cf47a49f428047a72..69599cc6185a67122866779a437386bacbfefaae 100644 (file)
--- 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 50b52fbd37fa5dc0abbc02f16111b7f6b2f3c0f1..85e7865833539b7f216c08f7ee1f8c00d1257154 100644 (file)
--- 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 37d51e58c08e15ae52905d5d4d6a1997d31a0fd1..a6e8ea904a8a5b03d7f82378958f117002d5bcab 100644 (file)
--- 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);
        }
     }