Re: [PATCH] Add support for untagged unions

[Tom Tromey <tom at tromey dot com>]

>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:

Manish> Rust supports untagged unions (C unions) now (using the same
Manish> syntax as structs but with `union` instead of `struct` in the
Manish> declaration).  These are mainly used for FFI.

Thank you.

Manish> No tests because stable Rust doesn't have these yet. Let me know
Manish> if I should add them.

I think they're necessary but I want to understand the options.

Is it hard to determine if rustc supports unions?
Is there some flag that must be passed?

I suppose one idea would be to feature test rustc in the test suite,
then change the new tests to be unsupported if this fails.  And, if a
flag is needed, pass it -- but go back once the feature hits stable and
remove the flag?

I think the essentials of the patch are fine.

Manish>     * rust-lang.c (rust_union_is_untagged): Add function to
Manish>     check if a union is an untagged unioni
Manish>     * rust-lang.c (rust_val_print): Handle printing of untagged union values

Just the first entry needs the "* FILENAME" bit.

Manish> +/* Whether or not a TYPE_CODE_UNION value is an untagged union
Manish> +   as opposed to being a regular Rust enum.  */
Manish> +static bool
Manish> +rust_union_is_untagged(struct type *type) {

This has various style issues.

Thanks for using bool.

Manish> +     Since the field bit positions overlap in the debuginfo,
Manish> +     the code for printing a union is same as that for a struct,
Manish> +     the only difference is that the input type will have overlapping
Manish> +     fields.  */
Manish> +  if (rust_union_is_untagged (type))
Manish> +      goto struct_val;

I think it'd be better to turn the specific case's body into a helper
function.  I'd like to get rid of a lot of these gotos; and I also plan
to remove as many cleanups as possible from the rust code in gdb...

Manish> +  /* This code path is also used by unions.  */
Manish> +  if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
Manish> +      fputs_filtered ("struct ", stream);
Manish> +  else
Manish> +    fputs_filtered ("union ", stream);
Manish> +
Manish>      if (TYPE_TAG_NAME (type) != NULL)

Indentation of that addition looks wrong.

Manish> +      /* Field access in structs and untagged unions works like C.  */
Manish>          *pos = pc;

Same here.

Tom