Hi Philip,

On Fri, Jul 23, 2021 at 12:19:18PM +0100, Philip Herron wrote:
> 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.
> > ---
> [...]
> 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.

Turned out that there was a bit more work to make constructor
expressions (StructExprStruct) work correctly, but it seems the basics
for union support are ok now. Not every pass (type checking) seems to
handle unions yet though. See below.

> 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.

I did add a enum field to distinquish between regular structs, tuple
structs and unions. But I am not sure this abstraction is the easiest
to work with. We will see when we add enum support. structs and tuple
structs are fairly similar and unions and enums also, but they don't
easily combine imho. And if you are treating all abstract data types
the same, then why are tuples separate?

> 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

According to https://gcc.gnu.org/onlinedocs/gccint/Types.html
QUAL_UNION_TYPE is for Ada variant records. Which may be somewhat
similar to Rust enums, but Rust unions seem to be like 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.

I renamed fill_in_struct to fill_in_fields and handle structs
(RECORD_TYPE) and unions (UNION_TYPE) separately now.

The other thing I needed was to handle constructors in Gcc backend
constructor_expression differently for structs and unions. Unions only
initialize one field. For that I added a union_index to
StructExprStruct to indicate which whether the contructor is for a
union and which field (index) is being initialized. The
TypeCheckStructExpr visitor sets this (and also checks the constructor
only mentions one field and doesn't have a base expression for
unions).

I added two simple testcases to show the basic support for unions
works now. But there are a couple of things that don't work correctly
for unions yet. In particular when enabling warnings for the new
union.rs testcase you'll get:

$ gcc/gccrs -Bgcc -g union.rs 
union.rs:18:3: warning: field is never read: ‘f1’
   18 |   f1: U,
      |   ^
union.rs:19:3: warning: field is never read: ‘f2’
   19 |   f2: V
      |   ^

But those (union) fields in the struct are read. Similarly unused
union fields aren't detected.

And you cannot use unions as function arguments or return values.
This example (sorry for the funny naming, union isn't a keyword, so
you can call basically anything a union, including a union, a union
field, function argument, etc.) doesn't pass the type checker:

union union { union: u32, funion: f32 }

fn funion (union: &union) -> union
{
  let v = unsafe { union.union };
  union { union: v }
}

pub fn main ()
{
  let union = union { union: 1 };
  let u = unsafe { union.union };
  let f = unsafe { union.funion };
  let r = funion (&union);
  let _r3 = unsafe { r.union } + unsafe { union.union } + u;
  let _f2 = f + unsafe { r.funion };
}

$ gcc/gccrs -Bgcc -g fn_union.rs 
fn_union.rs:5:20: error: expected algebraic data type got: [& union{union:Uint:u32:(Ref: 20 TyRef: 3[3,]), funion:Float:f32:(Ref: 23 TyRef: 12[12,])}]
    5 |   let v = unsafe { union.union };
      |                    ^
fn_union.rs:5:20: error: failed to type resolve expression
fn_union.rs:6:3: error: expected an ADT type for constructor
    6 |   union { union: v }
      |   ^
fn_union.rs:3:1: error: expected [union{union:Uint:u32:(Ref: 20 TyRef: 3[3,]), funion:Float:f32:(Ref: 23 TyRef: 12[12,])}] got [<tyty::error>]
    3 | fn funion (union: &union) -> union
      | ^                            ~

But I hope the patch is still useful as basic union support.
Attached and also at
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=union

Cheers,

Mark