Re: [PATCH] Add support for untagged unions

Tom Tromey
2016-10-31T03:13:52Z

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