[PATCH] Add support for untagged unions

[PATCH] Add support for untagged unions

[Manish Goregaokar]

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

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

From: Manish Goregaokar <manish@mozilla.com>
Subject: [PATCH] Add support for untagged unions

2016-10-28  Manish Goregaokar  <manish@mozilla.com>

gdb/ChangeLog:
    * rust-lang.c (rust_union_is_untagged): Add function to
    check if a union is an untagged unioni
    * rust-lang.c (rust_val_print): Handle printing of untagged union values
    * rust-lang.c (rust_print_type): Handle printing of untagged union types
    * rust-lang.c (rust_evaluate_subexp): Handle evaluating field
    access on untagged unions
---
 gdb/rust-lang.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 9569584..5376efc 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -93,6 +93,28 @@ struct disr_info

 #define RUST_ENCODED_ENUM_HIDDEN 1

+/* Whether or not a TYPE_CODE_UNION value is an untagged union
+   as opposed to being a regular Rust enum.  */
+static bool
+rust_union_is_untagged(struct type *type) {
+  /* Unions must have at least one field.  */
+  if (TYPE_NFIELDS (type) == 0)
+    return false;
+  /* If the first field is named, but the name has the rust enum prefix,
+      it is an enum.  */
+  if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
+         strlen (RUST_ENUM_PREFIX)) == 0)
+    return false;
+  /* Unions only have named fields.  */
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (strlen (TYPE_FIELD_NAME (type, i)) == 0)
+        return false;
+    }
+    return true;
+
+}
+
 /* Utility function to get discriminant info for a given value.  */

 static struct disr_info
@@ -566,6 +588,14 @@ rust_val_print (struct type *type, const gdb_byte
*valaddr, int embedded_offset,
     struct value_print_options opts;
     struct cleanup *cleanup;

+  /* Untagged unions are printed as if they are structs.
+     Since the field bit positions overlap in the debuginfo,
+     the code for printing a union is same as that for a struct,
+     the only difference is that the input type will have overlapping
+     fields.  */
+  if (rust_union_is_untagged (type))
+      goto struct_val;
+
     opts = *options;
     opts.deref_ref = 0;

@@ -638,6 +668,7 @@ rust_val_print (struct type *type, const gdb_byte
*valaddr, int embedded_offset,
       break;

     case TYPE_CODE_STRUCT:
+    struct_val:
       {
     int i;
     int first_field;
@@ -809,6 +840,7 @@ rust_print_type (struct type *type, const char *varstring,
       break;

     case TYPE_CODE_STRUCT:
+    struct_printer:
       {
     int is_tuple_struct;

@@ -823,7 +855,12 @@ rust_print_type (struct type *type, const char *varstring,
     if (TYPE_N_BASECLASSES (type) > 0)
       goto c_printer;

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

@@ -899,6 +936,13 @@ rust_print_type (struct type *type, const char *varstring,
     /* Skip the discriminant field.  */
     int skip_to = 1;

+  /* Unions and structs have the same syntax in Rust,
+     the only difference is that structs are declared with `struct`
+     and union with `union`. This difference is handled in the struct
+     printer.  */
+  if (rust_union_is_untagged (type))
+    goto struct_printer;
+
     fputs_filtered ("enum ", stream);
     if (TYPE_TAG_NAME (type) != NULL)
       {
@@ -1637,7 +1681,10 @@ rust_evaluate_subexp (struct type *expect_type,
struct expression *exp,
         lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);

         type = value_type (lhs);
-        if (TYPE_CODE (type) == TYPE_CODE_UNION)
+        /* Untagged unions can't have anonymous field access since
+           they can only have named fields.  */
+        if (TYPE_CODE (type) == TYPE_CODE_UNION
+            && !rust_union_is_untagged (type))
       {
         struct cleanup *cleanup;

@@ -1712,8 +1759,8 @@ tuple structs, and tuple-like enum variants"));
         lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);

         type = value_type (lhs);
-
-        if (TYPE_CODE (type) == TYPE_CODE_UNION)
+        if (TYPE_CODE (type) == TYPE_CODE_UNION
+            && !rust_union_is_untagged (type))
       {
         int i, start;
         struct disr_info disr;
@@ -1762,6 +1809,7 @@ which has only anonymous fields"),
       }
     else
       {
+      /* Field access in structs and untagged unions works like C.  */
         *pos = pc;
         result = evaluate_subexp_standard (expect_type, exp, pos, noside);
       }
-- 
2.10.1

Re: [PATCH] Add support for untagged unions

[Manish Goregaokar]

Commit message of this and the previous patch should probably say "in
Rust" somewhere (will amend before pushing)
-Manish


On Fri, Oct 28, 2016 at 6:02 PM, Manish Goregaokar <manish@mozilla.com> wrote:
> Rust supports untagged unions (C unions) now (using the same syntax as
> structs but with `union` instead of `struct` in the declaration).
> These are mainly used for FFI.
>
> No tests because stable Rust doesn't have these yet. Let me know if I
> should add them.
>
> From: Manish Goregaokar <manish@mozilla.com>
> Subject: [PATCH] Add support for untagged unions
>
> 2016-10-28  Manish Goregaokar  <manish@mozilla.com>
>
> gdb/ChangeLog:
>     * rust-lang.c (rust_union_is_untagged): Add function to
>     check if a union is an untagged unioni
>     * rust-lang.c (rust_val_print): Handle printing of untagged union values
>     * rust-lang.c (rust_print_type): Handle printing of untagged union types
>     * rust-lang.c (rust_evaluate_subexp): Handle evaluating field
>     access on untagged unions
> ---
>  gdb/rust-lang.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 9569584..5376efc 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -93,6 +93,28 @@ struct disr_info
>
>  #define RUST_ENCODED_ENUM_HIDDEN 1
>
> +/* Whether or not a TYPE_CODE_UNION value is an untagged union
> +   as opposed to being a regular Rust enum.  */
> +static bool
> +rust_union_is_untagged(struct type *type) {
> +  /* Unions must have at least one field.  */
> +  if (TYPE_NFIELDS (type) == 0)
> +    return false;
> +  /* If the first field is named, but the name has the rust enum prefix,
> +      it is an enum.  */
> +  if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
> +         strlen (RUST_ENUM_PREFIX)) == 0)
> +    return false;
> +  /* Unions only have named fields.  */
> +  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
> +    {
> +      if (strlen (TYPE_FIELD_NAME (type, i)) == 0)
> +        return false;
> +    }
> +    return true;
> +
> +}
> +
>  /* Utility function to get discriminant info for a given value.  */
>
>  static struct disr_info
> @@ -566,6 +588,14 @@ rust_val_print (struct type *type, const gdb_byte
> *valaddr, int embedded_offset,
>      struct value_print_options opts;
>      struct cleanup *cleanup;
>
> +  /* Untagged unions are printed as if they are structs.
> +     Since the field bit positions overlap in the debuginfo,
> +     the code for printing a union is same as that for a struct,
> +     the only difference is that the input type will have overlapping
> +     fields.  */
> +  if (rust_union_is_untagged (type))
> +      goto struct_val;
> +
>      opts = *options;
>      opts.deref_ref = 0;
>
> @@ -638,6 +668,7 @@ rust_val_print (struct type *type, const gdb_byte
> *valaddr, int embedded_offset,
>        break;
>
>      case TYPE_CODE_STRUCT:
> +    struct_val:
>        {
>      int i;
>      int first_field;
> @@ -809,6 +840,7 @@ rust_print_type (struct type *type, const char *varstring,
>        break;
>
>      case TYPE_CODE_STRUCT:
> +    struct_printer:
>        {
>      int is_tuple_struct;
>
> @@ -823,7 +855,12 @@ rust_print_type (struct type *type, const char *varstring,
>      if (TYPE_N_BASECLASSES (type) > 0)
>        goto c_printer;
>
> -    fputs_filtered ("struct ", stream);
> +  /* This code path is also used by unions.  */
> +  if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
> +      fputs_filtered ("struct ", stream);
> +  else
> +    fputs_filtered ("union ", stream);
> +
>      if (TYPE_TAG_NAME (type) != NULL)
>        fputs_filtered (TYPE_TAG_NAME (type), stream);
>
> @@ -899,6 +936,13 @@ rust_print_type (struct type *type, const char *varstring,
>      /* Skip the discriminant field.  */
>      int skip_to = 1;
>
> +  /* Unions and structs have the same syntax in Rust,
> +     the only difference is that structs are declared with `struct`
> +     and union with `union`. This difference is handled in the struct
> +     printer.  */
> +  if (rust_union_is_untagged (type))
> +    goto struct_printer;
> +
>      fputs_filtered ("enum ", stream);
>      if (TYPE_TAG_NAME (type) != NULL)
>        {
> @@ -1637,7 +1681,10 @@ rust_evaluate_subexp (struct type *expect_type,
> struct expression *exp,
>          lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);
>
>          type = value_type (lhs);
> -        if (TYPE_CODE (type) == TYPE_CODE_UNION)
> +        /* Untagged unions can't have anonymous field access since
> +           they can only have named fields.  */
> +        if (TYPE_CODE (type) == TYPE_CODE_UNION
> +            && !rust_union_is_untagged (type))
>        {
>          struct cleanup *cleanup;
>
> @@ -1712,8 +1759,8 @@ tuple structs, and tuple-like enum variants"));
>          lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);
>
>          type = value_type (lhs);
> -
> -        if (TYPE_CODE (type) == TYPE_CODE_UNION)
> +        if (TYPE_CODE (type) == TYPE_CODE_UNION
> +            && !rust_union_is_untagged (type))
>        {
>          int i, start;
>          struct disr_info disr;
> @@ -1762,6 +1809,7 @@ which has only anonymous fields"),
>        }
>      else
>        {
> +      /* Field access in structs and untagged unions works like C.  */
>          *pos = pc;
>          result = evaluate_subexp_standard (expect_type, exp, pos, noside);
>        }
> --
> 2.10.1

Re: [PATCH] Add support for untagged unions

[Tom Tromey]

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

Re: [PATCH] Add support for untagged unions

[Manish Goregaokar]

updated:

From cc3807d12c8cc1e205d8d545eb888be43eb2ec1b Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manish@mozilla.com>
Date: Fri, 28 Oct 2016 18:00:43 -0700
Subject: [PATCH 2/3] Add support for untagged unions in Rust

2016-10-28  Manish Goregaokar  <manish@mozilla.com>

gdb/ChangeLog:
    * rust-lang.c (rust_union_is_untagged): Add function to
    check if a union is an untagged unioni
    (rust_val_print): Handle printing of untagged union values
    (rust_print_type): Handle printing of untagged union types
    (rust_evaluate_subexp): Handle evaluating field
    access on untagged unions
---
 gdb/rust-lang.c | 309 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 189 insertions(+), 120 deletions(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 0ced4bb..a9446bc 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -93,6 +93,28 @@ struct disr_info

 #define RUST_ENCODED_ENUM_HIDDEN 1

+/* Whether or not a TYPE_CODE_UNION value is an untagged union
+   as opposed to being a regular Rust enum.  */
+static bool
+rust_union_is_untagged(struct type *type) {
+  /* Unions must have at least one field.  */
+  if (TYPE_NFIELDS (type) == 0)
+    return false;
+  /* If the first field is named, but the name has the rust enum prefix,
+      it is an enum.  */
+  if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
+           strlen (RUST_ENUM_PREFIX)) == 0)
+    return false;
+  /* Unions only have named fields.  */
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (strlen (TYPE_FIELD_NAME (type, i)) == 0)
+        return false;
+    }
+    return true;
+
+}
+
 /* Utility function to get discriminant info for a given value.  */

 static struct disr_info
@@ -455,6 +477,81 @@ rust_printstr (struct ui_file *stream, struct type *type,



+static void
+val_print_struct (struct type *type, const gdb_byte *valaddr,
+        int embedded_offset, CORE_ADDR address, struct ui_file *stream,
+        int recurse, const struct value *val,
+        const struct value_print_options *options) {
+  int i;
+  int first_field;
+  int is_tuple = rust_tuple_type_p (type);
+  int is_tuple_struct = !is_tuple && rust_tuple_struct_type_p (type);
+  struct value_print_options opts;
+
+  if (!is_tuple)
+    {
+      if (TYPE_TAG_NAME (type) != NULL)
+        fprintf_filtered (stream, "%s", TYPE_TAG_NAME (type));
+
+      if (TYPE_NFIELDS (type) == 0)
+        return;
+
+      if (TYPE_TAG_NAME (type) != NULL)
+        fputs_filtered (" ", stream);
+    }
+
+  if (is_tuple || is_tuple_struct)
+    fputs_filtered ("(", stream);
+  else
+    fputs_filtered ("{", stream);
+
+  opts = *options;
+  opts.deref_ref = 0;
+
+  first_field = 1;
+  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (field_is_static (&TYPE_FIELD (type, i)))
+        continue;
+
+      if (!first_field)
+        fputs_filtered (",", stream);
+
+      if (options->prettyformat)
+        {
+    fputs_filtered ("\n", stream);
+    print_spaces_filtered (2 + 2 * recurse, stream);
+        }
+      else if (!first_field)
+        fputs_filtered (" ", stream);
+
+      first_field = 0;
+
+      if (!is_tuple && !is_tuple_struct)
+        {
+    fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+    fputs_filtered (": ", stream);
+        }
+
+      val_print (TYPE_FIELD_TYPE (type, i),
+           valaddr,
+           embedded_offset + TYPE_FIELD_BITPOS (type, i) / 8,
+           address,
+           stream, recurse + 1, val, &opts,
+           current_language);
+    }
+
+  if (options->prettyformat)
+    {
+      fputs_filtered ("\n", stream);
+      print_spaces_filtered (2 * recurse, stream);
+    }
+
+  if (is_tuple || is_tuple_struct)
+    fputs_filtered (")", stream);
+  else
+    fputs_filtered ("}", stream);
+}
 static const struct generic_val_print_decorations rust_decorations =
 {
   /* Complex isn't used in Rust, but we provide C-ish values just in
@@ -566,6 +663,18 @@ rust_val_print (struct type *type, const gdb_byte
*valaddr, int embedded_offset,
     struct value_print_options opts;
     struct cleanup *cleanup;

+  /* Untagged unions are printed as if they are structs.
+     Since the field bit positions overlap in the debuginfo,
+     the code for printing a union is same as that for a struct,
+     the only difference is that the input type will have overlapping
+     fields.  */
+  if (rust_union_is_untagged (type))
+    {
+      val_print_struct (type, valaddr, embedded_offset, address, stream,
+              recurse, val, options);
+      break;
+    }
+
     opts = *options;
     opts.deref_ref = 0;

@@ -638,89 +747,84 @@ rust_val_print (struct type *type, const
gdb_byte *valaddr, int embedded_offset,
       break;

     case TYPE_CODE_STRUCT:
-      {
-    int i;
-    int first_field;
-    int is_tuple = rust_tuple_type_p (type);
-    int is_tuple_struct = !is_tuple && rust_tuple_struct_type_p (type);
-    struct value_print_options opts;
-
-    if (!is_tuple)
-      {
-        if (TYPE_TAG_NAME (type) != NULL)
-          fprintf_filtered (stream, "%s", TYPE_TAG_NAME (type));
+      val_print_struct (type, valaddr, embedded_offset, address, stream,
+       recurse, val, options);
+      break;

-        if (TYPE_NFIELDS (type) == 0)
-          break;
+    default:
+    generic_print:
+      /* Nothing special yet.  */
+      generic_val_print (type, valaddr, embedded_offset, address, stream,
+             recurse, val, options, &rust_decorations);
+    }
+}

-        if (TYPE_TAG_NAME (type) != NULL)
-          fputs_filtered (" ", stream);
-      }
+

-    if (is_tuple || is_tuple_struct)
-      fputs_filtered ("(", stream);
-    else
-      fputs_filtered ("{", stream);
+void
+rust_print_type (struct type *type, const char *varstring,
+         struct ui_file *stream, int show, int level,
+         const struct type_print_options *flags);

-    opts = *options;
-    opts.deref_ref = 0;
+/* Print a struct or union typedef.  */
+static void
+rust_print_struct_def (struct type *type, const char *varstring,
+         struct ui_file *stream, int show, int level,
+         const struct type_print_options *flags) {
+    int is_tuple_struct, i;

-    first_field = 1;
-    for (i = 0; i < TYPE_NFIELDS (type); ++i)
+    /* Print a tuple type simply.  */
+    if (rust_tuple_type_p (type))
       {
-        if (field_is_static (&TYPE_FIELD (type, i)))
-          continue;
+        fputs_filtered (TYPE_TAG_NAME (type), stream);
+        return;
+      }

-        if (!first_field)
-          fputs_filtered (",", stream);
+    /* If we see a base class, delegate to C.  */
+    if (TYPE_N_BASECLASSES (type) > 0)
+      c_print_type (type, varstring, stream, show, level, flags);

-        if (options->prettyformat)
-          {
-        fputs_filtered ("\n", stream);
-        print_spaces_filtered (2 + 2 * recurse, stream);
-          }
-        else if (!first_field)
-          fputs_filtered (" ", stream);
+  /* This code path is also used by unions.  */
+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+      fputs_filtered ("struct ", stream);
+  else
+      fputs_filtered ("union ", stream);

-        first_field = 0;
+    if (TYPE_TAG_NAME (type) != NULL)
+      fputs_filtered (TYPE_TAG_NAME (type), stream);

-        if (!is_tuple && !is_tuple_struct)
-          {
-        fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
-        fputs_filtered (": ", stream);
-          }
+    is_tuple_struct = rust_tuple_struct_type_p (type);

-        val_print (TYPE_FIELD_TYPE (type, i),
-               valaddr,
-               embedded_offset + TYPE_FIELD_BITPOS (type, i) / 8,
-               address,
-               stream, recurse + 1, val, &opts,
-               current_language);
-      }
+    if (TYPE_NFIELDS (type) == 0 && !rust_tuple_type_p (type))
+      return;
+    fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream);

-    if (options->prettyformat)
+    for (i = 0; i < TYPE_NFIELDS (type); ++i)
       {
-        fputs_filtered ("\n", stream);
-        print_spaces_filtered (2 * recurse, stream);
-      }
+        const char *name;

-    if (is_tuple || is_tuple_struct)
-      fputs_filtered (")", stream);
-    else
-      fputs_filtered ("}", stream);
-      }
-      break;
+        QUIT;
+        if (field_is_static (&TYPE_FIELD (type, i)))
+          continue;

-    default:
-    generic_print:
-      /* Nothing special yet.  */
-      generic_val_print (type, valaddr, embedded_offset, address, stream,
-             recurse, val, options, &rust_decorations);
-    }
-}
+        /* We'd like to print "pub" here as needed, but rustc
+           doesn't emit the debuginfo, and our types don't have
+           cplus_struct_type attached.  */

-
+        /* For a tuple struct we print the type but nothing
+           else.  */
+        print_spaces_filtered (level + 2, stream);
+        if (!is_tuple_struct)
+          fprintf_filtered (stream, "%s: ", TYPE_FIELD_NAME (type, i));

+        rust_print_type (TYPE_FIELD_TYPE (type, i), NULL,
+                 stream, show - 1, level + 2,
+                 flags);
+        fputs_filtered (",\n", stream);
+      }
+
+    fprintfi_filtered (level, stream, is_tuple_struct ? ")" : "}");
+}
 /* la_print_typedef implementation for Rust.  */

 static void
@@ -736,7 +840,7 @@ rust_print_typedef (struct type *type,

 /* la_print_type implementation for Rust.  */

-static void
+void
 rust_print_type (struct type *type, const char *varstring,
          struct ui_file *stream, int show, int level,
          const struct type_print_options *flags)
@@ -809,56 +913,7 @@ rust_print_type (struct type *type, const char *varstring,
       break;

     case TYPE_CODE_STRUCT:
-      {
-    int is_tuple_struct;
-
-    /* Print a tuple type simply.  */
-    if (rust_tuple_type_p (type))
-      {
-        fputs_filtered (TYPE_TAG_NAME (type), stream);
-        break;
-      }
-
-    /* If we see a base class, delegate to C.  */
-    if (TYPE_N_BASECLASSES (type) > 0)
-      goto c_printer;
-
-    fputs_filtered ("struct ", stream);
-    if (TYPE_TAG_NAME (type) != NULL)
-      fputs_filtered (TYPE_TAG_NAME (type), stream);
-
-    is_tuple_struct = rust_tuple_struct_type_p (type);
-
-    if (TYPE_NFIELDS (type) == 0 && !rust_tuple_type_p (type))
-      break;
-    fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream);
-
-    for (i = 0; i < TYPE_NFIELDS (type); ++i)
-      {
-        const char *name;
-
-        QUIT;
-        if (field_is_static (&TYPE_FIELD (type, i)))
-          continue;
-
-        /* We'd like to print "pub" here as needed, but rustc
-           doesn't emit the debuginfo, and our types don't have
-           cplus_struct_type attached.  */
-
-        /* For a tuple struct we print the type but nothing
-           else.  */
-        print_spaces_filtered (level + 2, stream);
-        if (!is_tuple_struct)
-          fprintf_filtered (stream, "%s: ", TYPE_FIELD_NAME (type, i));
-
-        rust_print_type (TYPE_FIELD_TYPE (type, i), NULL,
-                 stream, show - 1, level + 2,
-                 flags);
-        fputs_filtered (",\n", stream);
-      }
-
-    fprintfi_filtered (level, stream, is_tuple_struct ? ")" : "}");
-      }
+        rust_print_struct_def (type, varstring, stream, show, level, flags);
       break;

     case TYPE_CODE_ENUM:
@@ -899,6 +954,16 @@ rust_print_type (struct type *type, const char *varstring,
     /* Skip the discriminant field.  */
     int skip_to = 1;

+  /* Unions and structs have the same syntax in Rust,
+     the only difference is that structs are declared with `struct`
+     and union with `union`. This difference is handled in the struct
+     printer.  */
+  if (rust_union_is_untagged (type))
+    {
+      rust_print_struct_def (type, varstring, stream, show, level, flags);
+      break;
+    }
+
     fputs_filtered ("enum ", stream);
     if (TYPE_TAG_NAME (type) != NULL)
       {
@@ -1637,7 +1702,10 @@ rust_evaluate_subexp (struct type *expect_type,
struct expression *exp,
         lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);

         type = value_type (lhs);
-        if (TYPE_CODE (type) == TYPE_CODE_UNION)
+        /* Untagged unions can't have anonymous field access since
+           they can only have named fields.  */
+        if (TYPE_CODE (type) == TYPE_CODE_UNION
+            && !rust_union_is_untagged (type))
       {
         struct cleanup *cleanup;

@@ -1712,8 +1780,8 @@ tuple structs, and tuple-like enum variants"));
         lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);

         type = value_type (lhs);
-
-        if (TYPE_CODE (type) == TYPE_CODE_UNION)
+        if (TYPE_CODE (type) == TYPE_CODE_UNION
+            && !rust_union_is_untagged (type))
       {
         int i, start;
         struct disr_info disr;
@@ -1762,6 +1830,7 @@ which has only anonymous fields"),
       }
     else
       {
+        /* Field access in structs and untagged unions works like C.  */
         *pos = pc;
         result = evaluate_subexp_standard (expect_type, exp, pos, noside);
       }
-- 
2.10.1

-Manish


On Sun, Oct 30, 2016 at 8:12 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "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

Re: [PATCH] Add support for untagged unions

[Tom Tromey]

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

Re-CCing the list, I trust you don't mind.

Manish> I haven't added inline union ctor expressions yet. They're similar to
Manish> struct ctors but with only one field. Filed at
Manish> https://sourceware.org/bugzilla/show_bug.cgi?id=20761 , might not get
Manish> time to fix that for a bit.

Thank you.

Manish> I tried to fix the indentation, but I'm not really sure what the
Manish> heuristic here is. Everything seems to use a mix of tabs followed by
Manish> spaces, and I'm not sure how many of each to use. I have my tab width
Manish> set to 2 and am using two tabs followed by however many spaces
Manish> necessary to align it, but that might not be right. When pasting into
Manish> gmail it seems like tabs automatically get converted to spaces.

In the GNU style a tab is 8 spaces (really it moves to the next multiple
of 8, but there should only be leading tabs).

There's been occasional debate on just using spaces.


This version looks ok to me.  Please check it in.
If there's some lingering indentation issue I can fix it up later.

Tom

Re: [PATCH] Add support for untagged unions

[Manish Goregaokar]

Pushed all three patches. Thanks.
-Manish


On Thu, Nov 3, 2016 at 2:32 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:
>
> Re-CCing the list, I trust you don't mind.
>
> Manish> I haven't added inline union ctor expressions yet. They're similar to
> Manish> struct ctors but with only one field. Filed at
> Manish> https://sourceware.org/bugzilla/show_bug.cgi?id=20761 , might not get
> Manish> time to fix that for a bit.
>
> Thank you.
>
> Manish> I tried to fix the indentation, but I'm not really sure what the
> Manish> heuristic here is. Everything seems to use a mix of tabs followed by
> Manish> spaces, and I'm not sure how many of each to use. I have my tab width
> Manish> set to 2 and am using two tabs followed by however many spaces
> Manish> necessary to align it, but that might not be right. When pasting into
> Manish> gmail it seems like tabs automatically get converted to spaces.
>
> In the GNU style a tab is 8 spaces (really it moves to the next multiple
> of 8, but there should only be leading tabs).
>
> There's been occasional debate on just using spaces.
>
>
> This version looks ok to me.  Please check it in.
> If there's some lingering indentation issue I can fix it up later.
>
> Tom