public inbox for gcc-rust@gcc.gnu.org
 help / color / mirror / Atom feed
From: Philip Herron <philip.herron@embecosm.com>
To: gcc-rust@gcc.gnu.org
Subject: Re: [PATCH 2/2] WIP union hir-lowering and type support
Date: Fri, 23 Jul 2021 12:19:18 +0100	[thread overview]
Message-ID: <af5cb314-72b3-ce55-f3f3-b39cd0c1b312@embecosm.com> (raw)
In-Reply-To: <20210722231902.7401-3-mark@klomp.org>


[-- Attachment #1.1: Type: text/plain, Size: 24182 bytes --]

On 23/07/2021 00:19, Mark Wielaard wrote:
> Treat a union as a Struct variant like a tuple struct.  Add an
> iterator and get_identifier functions to the AST Union class.  Same
> for the HIR Union class, plus a get_generics_params method. Add a
> get_is_union method tot the ADTType.
> ---
>  gcc/rust/ast/rust-item.h                      | 11 ++++
>  gcc/rust/hir/rust-ast-lower-item.h            | 51 +++++++++++++++++
>  gcc/rust/hir/rust-ast-lower-stmt.h            | 53 ++++++++++++++++++
>  gcc/rust/hir/tree/rust-hir-item.h             | 16 ++++++
>  gcc/rust/resolve/rust-ast-resolve-item.h      | 22 ++++++++
>  gcc/rust/resolve/rust-ast-resolve-stmt.h      | 32 +++++++++++
>  gcc/rust/resolve/rust-ast-resolve-toplevel.h  | 14 +++++
>  gcc/rust/typecheck/rust-hir-type-check-stmt.h | 55 ++++++++++++++++++-
>  .../typecheck/rust-hir-type-check-toplevel.h  | 54 +++++++++++++++++-
>  gcc/rust/typecheck/rust-hir-type-check.cc     | 12 +++-
>  gcc/rust/typecheck/rust-tycheck-dump.h        |  6 ++
>  gcc/rust/typecheck/rust-tyty.cc               |  4 +-
>  gcc/rust/typecheck/rust-tyty.h                | 12 ++--
>  13 files changed, 331 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
> index 30cab0ed900..1e928e8111a 100644
> --- a/gcc/rust/ast/rust-item.h
> +++ b/gcc/rust/ast/rust-item.h
> @@ -2489,6 +2489,15 @@ public:
>    std::vector<StructField> &get_variants () { return variants; }
>    const std::vector<StructField> &get_variants () const { return variants; }
>  
> +  void iterate (std::function<bool (StructField &)> cb)
> +  {
> +    for (auto &variant : variants)
> +      {
> +	if (!cb (variant))
> +	  return;
> +      }
> +  }
> +
>    std::vector<std::unique_ptr<GenericParam> > &get_generic_params ()
>    {
>      return generic_params;
> @@ -2505,6 +2514,8 @@ public:
>      return where_clause;
>    }
>  
> +  Identifier get_identifier () const { return union_name; }
> +
>  protected:
>    /* Use covariance to implement clone function as returning this object
>     * rather than base */
> diff --git a/gcc/rust/hir/rust-ast-lower-item.h b/gcc/rust/hir/rust-ast-lower-item.h
> index 5ba59183179..b6af00f6b54 100644
> --- a/gcc/rust/hir/rust-ast-lower-item.h
> +++ b/gcc/rust/hir/rust-ast-lower-item.h
> @@ -192,6 +192,57 @@ public:
>  			       struct_decl.get_locus ());
>    }
>  
> +  void visit (AST::Union &union_decl) override
> +  {
> +    std::vector<std::unique_ptr<HIR::GenericParam> > generic_params;
> +    if (union_decl.has_generics ())
> +      {
> +	generic_params
> +	  = lower_generic_params (union_decl.get_generic_params ());
> +      }
> +
> +    std::vector<std::unique_ptr<HIR::WhereClauseItem> > where_clause_items;
> +    HIR::WhereClause where_clause (std::move (where_clause_items));
> +    HIR::Visibility vis = HIR::Visibility::create_public ();
> +
> +    std::vector<HIR::StructField> variants;
> +    union_decl.iterate ([&] (AST::StructField &variant) mutable -> bool {
> +      HIR::Visibility vis = HIR::Visibility::create_public ();
> +      HIR::Type *type
> +	= ASTLoweringType::translate (variant.get_field_type ().get ());
> +
> +      auto crate_num = mappings->get_current_crate ();
> +      Analysis::NodeMapping mapping (crate_num, variant.get_node_id (),
> +				     mappings->get_next_hir_id (crate_num),
> +				     mappings->get_next_localdef_id (
> +				       crate_num));
> +
> +      HIR::StructField translated_variant (mapping, variant.get_field_name (),
> +					   std::unique_ptr<HIR::Type> (type),
> +					   vis, variant.get_locus (),
> +					   variant.get_outer_attrs ());
> +      variants.push_back (std::move (translated_variant));
> +      return true;
> +    });
> +
> +    auto crate_num = mappings->get_current_crate ();
> +    Analysis::NodeMapping mapping (crate_num, union_decl.get_node_id (),
> +				   mappings->get_next_hir_id (crate_num),
> +				   mappings->get_next_localdef_id (crate_num));
> +
> +    translated
> +      = new HIR::Union (mapping, union_decl.get_identifier (), vis,
> +			std::move (generic_params), std::move (where_clause),
> +			std::move (variants), union_decl.get_outer_attrs (),
> +			union_decl.get_locus ());
> +
> +    mappings->insert_defid_mapping (mapping.get_defid (), translated);
> +    mappings->insert_hir_item (mapping.get_crate_num (), mapping.get_hirid (),
> +			       translated);
> +    mappings->insert_location (crate_num, mapping.get_hirid (),
> +			       union_decl.get_locus ());
> +  }
> +
>    void visit (AST::StaticItem &var) override
>    {
>      HIR::Visibility vis = HIR::Visibility::create_public ();
> diff --git a/gcc/rust/hir/rust-ast-lower-stmt.h b/gcc/rust/hir/rust-ast-lower-stmt.h
> index 9df6b746bb7..2e97ca63a13 100644
> --- a/gcc/rust/hir/rust-ast-lower-stmt.h
> +++ b/gcc/rust/hir/rust-ast-lower-stmt.h
> @@ -215,6 +215,59 @@ public:
>  			       struct_decl.get_locus ());
>    }
>  
> +  void visit (AST::Union &union_decl) override
> +  {
> +    std::vector<std::unique_ptr<HIR::GenericParam> > generic_params;
> +    if (union_decl.has_generics ())
> +      {
> +	generic_params
> +	  = lower_generic_params (union_decl.get_generic_params ());
> +      }
> +
> +    std::vector<std::unique_ptr<HIR::WhereClauseItem> > where_clause_items;
> +    HIR::WhereClause where_clause (std::move (where_clause_items));
> +    HIR::Visibility vis = HIR::Visibility::create_public ();
> +
> +    std::vector<HIR::StructField> variants;
> +    union_decl.iterate ([&] (AST::StructField &variant) mutable -> bool {
> +      HIR::Visibility vis = HIR::Visibility::create_public ();
> +      HIR::Type *type
> +	= ASTLoweringType::translate (variant.get_field_type ().get ());
> +
> +      auto crate_num = mappings->get_current_crate ();
> +      Analysis::NodeMapping mapping (crate_num, variant.get_node_id (),
> +				     mappings->get_next_hir_id (crate_num),
> +				     mappings->get_next_localdef_id (
> +				       crate_num));
> +
> +      // FIXME
> +      // AST::StructField is missing Location info
> +      Location variant_locus;
> +      HIR::StructField translated_variant (mapping, variant.get_field_name (),
> +					   std::unique_ptr<HIR::Type> (type),
> +					   vis, variant_locus,
> +					   variant.get_outer_attrs ());
> +      variants.push_back (std::move (translated_variant));
> +      return true;
> +    });
> +
> +    auto crate_num = mappings->get_current_crate ();
> +    Analysis::NodeMapping mapping (crate_num, union_decl.get_node_id (),
> +				   mappings->get_next_hir_id (crate_num),
> +				   mappings->get_next_localdef_id (crate_num));
> +
> +    translated
> +      = new HIR::Union (mapping, union_decl.get_identifier (), vis,
> +			std::move (generic_params), std::move (where_clause),
> +			std::move (variants), union_decl.get_outer_attrs (),
> +			union_decl.get_locus ());
> +
> +    mappings->insert_hir_stmt (mapping.get_crate_num (), mapping.get_hirid (),
> +			       translated);
> +    mappings->insert_location (crate_num, mapping.get_hirid (),
> +			       union_decl.get_locus ());
> +  }
> +
>    void visit (AST::EmptyStmt &empty) override
>    {
>      auto crate_num = mappings->get_current_crate ();
> diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
> index e7e110fda92..cfe45d73d85 100644
> --- a/gcc/rust/hir/tree/rust-hir-item.h
> +++ b/gcc/rust/hir/tree/rust-hir-item.h
> @@ -1989,10 +1989,26 @@ public:
>    Union (Union &&other) = default;
>    Union &operator= (Union &&other) = default;
>  
> +  std::vector<std::unique_ptr<GenericParam> > &get_generic_params ()
> +  {
> +    return generic_params;
> +  }
> +
> +  Identifier get_identifier () const { return union_name; }
> +
>    Location get_locus () const { return locus; }
>  
>    void accept_vis (HIRVisitor &vis) override;
>  
> +  void iterate (std::function<bool (StructField &)> cb)
> +  {
> +    for (auto &variant : variants)
> +      {
> +	if (!cb (variant))
> +	  return;
> +      }
> +  }
> +
>  protected:
>    /* Use covariance to implement clone function as returning this object
>     * rather than base */
> diff --git a/gcc/rust/resolve/rust-ast-resolve-item.h b/gcc/rust/resolve/rust-ast-resolve-item.h
> index 0714f5d5706..54f1fe15533 100644
> --- a/gcc/rust/resolve/rust-ast-resolve-item.h
> +++ b/gcc/rust/resolve/rust-ast-resolve-item.h
> @@ -260,6 +260,28 @@ public:
>      resolver->get_type_scope ().pop ();
>    }
>  
> +  void visit (AST::Union &union_decl) override
> +  {
> +    NodeId scope_node_id = union_decl.get_node_id ();
> +    resolver->get_type_scope ().push (scope_node_id);
> +
> +    if (union_decl.has_generics ())
> +      {
> +	for (auto &generic : union_decl.get_generic_params ())
> +	  {
> +	    ResolveGenericParam::go (generic.get (), union_decl.get_node_id ());
> +	  }
> +      }
> +
> +    union_decl.iterate ([&] (AST::StructField &field) mutable -> bool {
> +      ResolveType::go (field.get_field_type ().get (),
> +		       union_decl.get_node_id ());
> +      return true;
> +    });
> +
> +    resolver->get_type_scope ().pop ();
> +  }
> +
>    void visit (AST::StaticItem &var) override
>    {
>      ResolveType::go (var.get_type ().get (), var.get_node_id ());
> diff --git a/gcc/rust/resolve/rust-ast-resolve-stmt.h b/gcc/rust/resolve/rust-ast-resolve-stmt.h
> index 210a9fc047d..b6044327b27 100644
> --- a/gcc/rust/resolve/rust-ast-resolve-stmt.h
> +++ b/gcc/rust/resolve/rust-ast-resolve-stmt.h
> @@ -131,6 +131,38 @@ public:
>      resolver->get_type_scope ().pop ();
>    }
>  
> +  void visit (AST::Union &union_decl) override
> +  {
> +    auto path = CanonicalPath::new_seg (union_decl.get_node_id (),
> +					union_decl.get_identifier ());
> +    resolver->get_type_scope ().insert (
> +      path, union_decl.get_node_id (), union_decl.get_locus (), false,
> +      [&] (const CanonicalPath &, NodeId, Location locus) -> void {
> +	RichLocation r (union_decl.get_locus ());
> +	r.add_range (locus);
> +	rust_error_at (r, "redefined multiple times");
> +      });
> +
> +    NodeId scope_node_id = union_decl.get_node_id ();
> +    resolver->get_type_scope ().push (scope_node_id);
> +
> +    if (union_decl.has_generics ())
> +      {
> +	for (auto &generic : union_decl.get_generic_params ())
> +	  {
> +	    ResolveGenericParam::go (generic.get (), union_decl.get_node_id ());
> +	  }
> +      }
> +
> +    union_decl.iterate ([&] (AST::StructField &field) mutable -> bool {
> +      ResolveType::go (field.get_field_type ().get (),
> +		       union_decl.get_node_id ());
> +      return true;
> +    });
> +
> +    resolver->get_type_scope ().pop ();
> +  }
> +
>    void visit (AST::Function &function) override
>    {
>      auto path = ResolveFunctionItemToCanonicalPath::resolve (function);
> diff --git a/gcc/rust/resolve/rust-ast-resolve-toplevel.h b/gcc/rust/resolve/rust-ast-resolve-toplevel.h
> index 9abbb18e080..4df0467b994 100644
> --- a/gcc/rust/resolve/rust-ast-resolve-toplevel.h
> +++ b/gcc/rust/resolve/rust-ast-resolve-toplevel.h
> @@ -81,6 +81,20 @@ public:
>        });
>    }
>  
> +  void visit (AST::Union &union_decl) override
> +  {
> +    auto path
> +      = prefix.append (CanonicalPath::new_seg (union_decl.get_node_id (),
> +					       union_decl.get_identifier ()));
> +    resolver->get_type_scope ().insert (
> +      path, union_decl.get_node_id (), union_decl.get_locus (), false,
> +      [&] (const CanonicalPath &, NodeId, Location locus) -> void {
> +	RichLocation r (union_decl.get_locus ());
> +	r.add_range (locus);
> +	rust_error_at (r, "redefined multiple times");
> +      });
> +  }
> +
>    void visit (AST::StaticItem &var) override
>    {
>      auto path = prefix.append (
> diff --git a/gcc/rust/typecheck/rust-hir-type-check-stmt.h b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> index 1b6f47c1595..fad2b7183df 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> +++ b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> @@ -159,7 +159,7 @@ public:
>      TyTy::BaseType *type
>        = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
>  			   mappings->get_next_hir_id (),
> -			   struct_decl.get_identifier (), true,
> +			   struct_decl.get_identifier (), true, false,
>  			   std::move (fields), std::move (substitutions));
>  
>      context->insert_type (struct_decl.get_mappings (), type);
> @@ -209,13 +209,64 @@ public:
>      TyTy::BaseType *type
>        = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
>  			   mappings->get_next_hir_id (),
> -			   struct_decl.get_identifier (), false,
> +			   struct_decl.get_identifier (), false, false,
>  			   std::move (fields), std::move (substitutions));
>  
>      context->insert_type (struct_decl.get_mappings (), type);
>      infered = type;
>    }
>  
> +  void visit (HIR::Union &union_decl) override
> +  {
> +    std::vector<TyTy::SubstitutionParamMapping> substitutions;
> +    if (union_decl.has_generics ())
> +      {
> +	for (auto &generic_param : union_decl.get_generic_params ())
> +	  {
> +	    switch (generic_param.get ()->get_kind ())
> +	      {
> +	      case HIR::GenericParam::GenericKind::LIFETIME:
> +		// Skipping Lifetime completely until better handling.
> +		break;
> +
> +		case HIR::GenericParam::GenericKind::TYPE: {
> +		  auto param_type
> +		    = TypeResolveGenericParam::Resolve (generic_param.get ());
> +		  context->insert_type (generic_param->get_mappings (),
> +					param_type);
> +
> +		  substitutions.push_back (TyTy::SubstitutionParamMapping (
> +		    static_cast<HIR::TypeParam &> (*generic_param),
> +		    param_type));
> +		}
> +		break;
> +	      }
> +	  }
> +      }
> +
> +    std::vector<TyTy::StructFieldType *> variants;
> +    union_decl.iterate ([&] (HIR::StructField &variant) mutable -> bool {
> +      TyTy::BaseType *variant_type
> +	= TypeCheckType::Resolve (variant.get_field_type ().get ());
> +      TyTy::StructFieldType *ty_variant
> +	= new TyTy::StructFieldType (variant.get_mappings ().get_hirid (),
> +				     variant.get_field_name (), variant_type);
> +      variants.push_back (ty_variant);
> +      context->insert_type (variant.get_mappings (),
> +			    ty_variant->get_field_type ());
> +      return true;
> +    });
> +
> +    TyTy::BaseType *type
> +      = new TyTy::ADTType (union_decl.get_mappings ().get_hirid (),
> +			   mappings->get_next_hir_id (),
> +			   union_decl.get_identifier (), false, true,
> +			   std::move (variants), std::move (substitutions));
> +
> +    context->insert_type (union_decl.get_mappings (), type);
> +    infered = type;
> +  }
> +
>    void visit (HIR::Function &function) override
>    {
>      std::vector<TyTy::SubstitutionParamMapping> substitutions;
> diff --git a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> index dd3dd751ad6..a723e7e679f 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> +++ b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> @@ -94,7 +94,7 @@ public:
>      TyTy::BaseType *type
>        = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
>  			   mappings->get_next_hir_id (),
> -			   struct_decl.get_identifier (), true,
> +			   struct_decl.get_identifier (), true, false,
>  			   std::move (fields), std::move (substitutions));
>  
>      context->insert_type (struct_decl.get_mappings (), type);
> @@ -143,12 +143,62 @@ public:
>      TyTy::BaseType *type
>        = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
>  			   mappings->get_next_hir_id (),
> -			   struct_decl.get_identifier (), false,
> +			   struct_decl.get_identifier (), false, false,
>  			   std::move (fields), std::move (substitutions));
>  
>      context->insert_type (struct_decl.get_mappings (), type);
>    }
>  
> +  void visit (HIR::Union &union_decl) override
> +  {
> +    std::vector<TyTy::SubstitutionParamMapping> substitutions;
> +    if (union_decl.has_generics ())
> +      {
> +	for (auto &generic_param : union_decl.get_generic_params ())
> +	  {
> +	    switch (generic_param.get ()->get_kind ())
> +	      {
> +	      case HIR::GenericParam::GenericKind::LIFETIME:
> +		// Skipping Lifetime completely until better handling.
> +		break;
> +
> +		case HIR::GenericParam::GenericKind::TYPE: {
> +		  auto param_type
> +		    = TypeResolveGenericParam::Resolve (generic_param.get ());
> +		  context->insert_type (generic_param->get_mappings (),
> +					param_type);
> +
> +		  substitutions.push_back (TyTy::SubstitutionParamMapping (
> +		    static_cast<HIR::TypeParam &> (*generic_param),
> +		    param_type));
> +		}
> +		break;
> +	      }
> +	  }
> +      }
> +
> +    std::vector<TyTy::StructFieldType *> variants;
> +    union_decl.iterate ([&] (HIR::StructField &variant) mutable -> bool {
> +      TyTy::BaseType *variant_type
> +	= TypeCheckType::Resolve (variant.get_field_type ().get ());
> +      TyTy::StructFieldType *ty_variant
> +	= new TyTy::StructFieldType (variant.get_mappings ().get_hirid (),
> +				     variant.get_field_name (), variant_type);
> +      variants.push_back (ty_variant);
> +      context->insert_type (variant.get_mappings (),
> +			    ty_variant->get_field_type ());
> +      return true;
> +    });
> +
> +    TyTy::BaseType *type
> +      = new TyTy::ADTType (union_decl.get_mappings ().get_hirid (),
> +			   mappings->get_next_hir_id (),
> +			   union_decl.get_identifier (), false, true,
> +			   std::move (variants), std::move (substitutions));
> +
> +    context->insert_type (union_decl.get_mappings (), type);
> +  }
> +
>    void visit (HIR::StaticItem &var) override
>    {
>      TyTy::BaseType *type = TypeCheckType::Resolve (var.get_type ());
> diff --git a/gcc/rust/typecheck/rust-hir-type-check.cc b/gcc/rust/typecheck/rust-hir-type-check.cc
> index cb2896c0bb4..da528d7878a 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check.cc
> +++ b/gcc/rust/typecheck/rust-hir-type-check.cc
> @@ -180,7 +180,17 @@ TypeCheckStructExpr::visit (HIR::StructExprStructFields &struct_expr)
>    // check the arguments are all assigned and fix up the ordering
>    if (fields_assigned.size () != struct_path_resolved->num_fields ())
>      {
> -      if (!struct_expr.has_struct_base ())
> +      if (struct_def->get_is_union ())
> +	{
> +	  if (fields_assigned.size () != 1)
> +	    {
> +	      rust_error_at (
> +		struct_expr.get_locus (),
> +		"union must have exactly one field variant assigned");
> +	      return;
> +	    }
> +	}
> +      else if (!struct_expr.has_struct_base ())
>  	{
>  	  rust_error_at (struct_expr.get_locus (),
>  			 "constructor is missing fields");
> diff --git a/gcc/rust/typecheck/rust-tycheck-dump.h b/gcc/rust/typecheck/rust-tycheck-dump.h
> index b80372b2a9c..cc2e3c01110 100644
> --- a/gcc/rust/typecheck/rust-tycheck-dump.h
> +++ b/gcc/rust/typecheck/rust-tycheck-dump.h
> @@ -48,6 +48,12 @@ public:
>  	    + "\n";
>    }
>  
> +  void visit (HIR::Union &union_decl) override
> +  {
> +    dump
> +      += indent () + "union " + type_string (union_decl.get_mappings ()) + "\n";
> +  }
> +
>    void visit (HIR::ImplBlock &impl_block) override
>    {
>      dump += indent () + "impl "
> diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc
> index f043c7eabda..d059134f8a0 100644
> --- a/gcc/rust/typecheck/rust-tyty.cc
> +++ b/gcc/rust/typecheck/rust-tyty.cc
> @@ -517,8 +517,8 @@ ADTType::clone ()
>      cloned_fields.push_back ((StructFieldType *) f->clone ());
>  
>    return new ADTType (get_ref (), get_ty_ref (), identifier, get_is_tuple (),
> -		      cloned_fields, clone_substs (), used_arguments,
> -		      get_combined_refs ());
> +		      get_is_union (), cloned_fields, clone_substs (),
> +		      used_arguments, get_combined_refs ());
>  }
>  
>  ADTType *
> diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h
> index 2152c1b6d76..b7cf46bb783 100644
> --- a/gcc/rust/typecheck/rust-tyty.h
> +++ b/gcc/rust/typecheck/rust-tyty.h
> @@ -848,7 +848,7 @@ protected:
>  class ADTType : public BaseType, public SubstitutionRef
>  {
>  public:
> -  ADTType (HirId ref, std::string identifier, bool is_tuple,
> +  ADTType (HirId ref, std::string identifier, bool is_tuple, bool is_union,
>  	   std::vector<StructFieldType *> fields,
>  	   std::vector<SubstitutionParamMapping> subst_refs,
>  	   SubstitutionArgumentMappings generic_arguments
> @@ -856,21 +856,24 @@ public:
>  	   std::set<HirId> refs = std::set<HirId> ())
>      : BaseType (ref, ref, TypeKind::ADT, refs),
>        SubstitutionRef (std::move (subst_refs), std::move (generic_arguments)),
> -      identifier (identifier), fields (fields), is_tuple (is_tuple)
> +      identifier (identifier), fields (fields), is_tuple (is_tuple),
> +      is_union (is_union)
>    {}
>  
>    ADTType (HirId ref, HirId ty_ref, std::string identifier, bool is_tuple,
> -	   std::vector<StructFieldType *> fields,
> +	   bool is_union, std::vector<StructFieldType *> fields,
>  	   std::vector<SubstitutionParamMapping> subst_refs,
>  	   SubstitutionArgumentMappings generic_arguments
>  	   = SubstitutionArgumentMappings::error (),
>  	   std::set<HirId> refs = std::set<HirId> ())
>      : BaseType (ref, ty_ref, TypeKind::ADT, refs),
>        SubstitutionRef (std::move (subst_refs), std::move (generic_arguments)),
> -      identifier (identifier), fields (fields), is_tuple (is_tuple)
> +      identifier (identifier), fields (fields), is_tuple (is_tuple),
> +      is_union (is_union)
>    {}
>  
>    bool get_is_tuple () { return is_tuple; }
> +  bool get_is_union () { return is_union; }
>  
>    bool is_unit () const override { return this->fields.empty (); }
>  
> @@ -957,6 +960,7 @@ private:
>    std::string identifier;
>    std::vector<StructFieldType *> fields;
>    bool is_tuple;
> +  bool is_union;
>  };
>  
>  class FnType : public BaseType, public SubstitutionRef
Hi Mark

Nice work Mark, I think this is nearly there. As far as i can tell there
is potentially 1 change to make in the ADTType and 1 more change to the
rust-gcc.cc backend stuff to finish it.

The ADTType is going to be used to represent not just structs and unions
but also enums.
https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/adt/struct.AdtDef.html
has a field called flags which looks like a u32 bitfield to have
IS_ENUM, IS_UNION etc it might make sense to turn your is_union flag
into a bitmask now as part of this patch.

The other missing piece is that we need to have another function in
rust-gcc.cc to create the GCC's union_type_node since this will
currently be creating a RECORD_TYPE at the moment.

https://github.com/Rust-GCC/gccrs/blob/44472c580cedd836081c82e621482e479a69729c/gcc/rust/rust-gcc.cc#L1085

I am slightly stuck though since I don't understand the difference
between UNION_TYPE and QUAL_UNION_TYPE:
https://github.com/Rust-GCC/gccrs/blob/master/gcc/tree.def#L216-L222

Maybe someone on the list can give an example between those union types.

When we are able to create the GCC union type we should be able to
create an if statement over inside:
https://github.com/Rust-GCC/gccrs/blob/44472c580cedd836081c82e621482e479a69729c/gcc/rust/backend/rust-compile-context.h#L415
to call union_type or something similar instead of the struct_type.

Moving forward I think this patch looks pretty good to be merged and you
can make another patch to change the is_union flag into a bitmask with
just the IS_UNION piece for now. Then once that's done i would suggest
looking into how the GENERIC part to create the union_type.

I hope some of this makes sense. Nice work.

--Phil


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

  reply	other threads:[~2021-07-23 11:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 23:19 union support Mark Wielaard
2021-07-22 23:19 ` [PATCH 1/2] Better union support in the parser Mark Wielaard
2021-07-23 11:19   ` Philip Herron
2021-07-22 23:19 ` [PATCH 2/2] WIP union hir-lowering and type support Mark Wielaard
2021-07-23 11:19   ` Philip Herron [this message]
2021-08-01 11:29     ` Mark Wielaard
2021-08-01 22:37       ` Mark Wielaard
2021-08-02 12:33         ` Thomas Schwinge
2021-08-04 21:04       ` Mark Wielaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=af5cb314-72b3-ce55-f3f3-b39cd0c1b312@embecosm.com \
    --to=philip.herron@embecosm.com \
    --cc=gcc-rust@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).