[RFA,v2,3/4] Convert Rust to use discriminated unions

Message ID 20180222203018.23551-4-tom@tromey.com
State New
Headers show
Series
  • variants and variant parts
Related show

Commit Message

Tom Tromey Feb. 22, 2018, 8:30 p.m.
A Rust enum is, essentially, a discriminated union.  Currently the
Rust language support handles Rust enums locally, in rust-lang.c.
However, because I am changing the Rust compiler to use
DW_TAG_variant* to represent enums, it seemed better to have a single
internal representation for Rust enums in gdb.

This patch implements this idea by moving the current Rust enum
handling code to dwarf2read.  This allows the simplification of some
parts of rust-lang.c as well.

2018-02-22  Tom Tromey  <tom@tromey.com>

	* rust-lang.h (rust_last_path_segment): Declare.
	* rust-lang.c (rust_last_path_segment): Now public.  Change
	contract.
	(struct disr_info): Remove.
	(RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL)
	(RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged)
	(rust_get_disr_info, rust_tuple_variant_type_p): Remove.
	(rust_enum_p, rust_enum_variant): New function.
	(rust_underscore_fields): Remove "offset" parameter.
	(rust_print_enum): New function.
	(rust_val_print) <TYPE_CODE_UNION>: Remove enum code.
	<TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate.
	(rust_print_struct_def): Add "for_rust_enum" parameter.  Handle
	enums.
	(rust_internal_print_type): New function, from rust_print_type.
	Remove enum code.
	(rust_print_type): Call rust_internal_print_type.
	(rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>:
	Update enum handling.
	* dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field.
	(rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum)
	(rust_union_quirks): New functions.
	(process_full_comp_unit, process_full_type_unit): Call
	rust_union_quirks.
	(process_structure_scope): Update rust_unions if necessary.

2018-02-22  Tom Tromey  <tom@tromey.com>

	* gdb.rust/simple.exp: Accept more possible results in enum test.
---
 gdb/ChangeLog                     |  28 ++
 gdb/dwarf2read.c                  | 313 +++++++++++++++
 gdb/rust-lang.c                   | 796 ++++++++++++++------------------------
 gdb/rust-lang.h                   |   5 +
 gdb/testsuite/ChangeLog           |   4 +
 gdb/testsuite/gdb.rust/simple.exp |   4 +-
 6 files changed, 640 insertions(+), 510 deletions(-)

-- 
2.13.6

Comments

Joel Brobecker Feb. 26, 2018, 6:50 a.m. | #1
> 2018-02-22  Tom Tromey  <tom@tromey.com>

> 

> 	* rust-lang.h (rust_last_path_segment): Declare.

> 	* rust-lang.c (rust_last_path_segment): Now public.  Change

> 	contract.

> 	(struct disr_info): Remove.

> 	(RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL)

> 	(RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged)

> 	(rust_get_disr_info, rust_tuple_variant_type_p): Remove.

> 	(rust_enum_p, rust_enum_variant): New function.

> 	(rust_underscore_fields): Remove "offset" parameter.

> 	(rust_print_enum): New function.

> 	(rust_val_print) <TYPE_CODE_UNION>: Remove enum code.

> 	<TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate.

> 	(rust_print_struct_def): Add "for_rust_enum" parameter.  Handle

> 	enums.

> 	(rust_internal_print_type): New function, from rust_print_type.

> 	Remove enum code.

> 	(rust_print_type): Call rust_internal_print_type.

> 	(rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>:

> 	Update enum handling.

> 	* dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field.

> 	(rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum)

> 	(rust_union_quirks): New functions.

> 	(process_full_comp_unit, process_full_type_unit): Call

> 	rust_union_quirks.

> 	(process_structure_scope): Update rust_unions if necessary.

> 

> 2018-02-22  Tom Tromey  <tom@tromey.com>

> 

> 	* gdb.rust/simple.exp: Accept more possible results in enum test.


Looks good to me.

Thanks Tom.
-- 
Joel
Pedro Alves Feb. 27, 2018, 11:22 p.m. | #2
Hi Tom,

I was comparing test results from 20180207 and today, I
noticed that gdb.rust/simple.exp regressed:

~~~~~~
Running src/gdb/testsuite/gdb.rust/simple.exp ...
-PASS: gdb.rust/simple.exp: print a 
-PASS: gdb.rust/simple.exp: ptype a 
-PASS: gdb.rust/simple.exp: print sizeof(a) 
-PASS: gdb.rust/simple.exp: print b 
(...)
+UNTESTED: gdb.rust/simple.exp: could not run to breakpoint
~~~~~~

Looking at gdb.log I see:

~~~~~~~~~~~~~~~~~~~~~~
gdb) break simple.rs:152
src/gdb/dwarf2read.c:10367: internal-error: discriminant_info* alloc_discriminant_info(type*, int, int): Assertion `default_index == -1 || (default_index > 0 && default_index < TYPE_NFIELDS (type))' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) Resyncing due to internal error.
n

This is a bug, please report it.  For instructions, see:
<http://www.gnu.org/software/gdb/bugs/>.

src/gdb/dwarf2read.c:10367: internal-error: discriminant_info* alloc_discriminant_info(type*, int, int): Assertion `default_index == -1 || (default_index > 0 && default_index < TYPE_NFIELDS (type))' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n) n
Command aborted.
(gdb) UNTESTED: gdb.rust/simple.exp: could not run to breakpointt
~~~~~~~~~~~~~~~~~~~~~~

I haven't bisected, but the references to discriminant info make me suspect
this patch I'm replying to.

This is Fedora 27 with:

 $ rustc --version
 rustc 1.13.0 (2c6933acc 2016-11-07)

Thanks,
Pedro Alves
Tom Tromey Feb. 28, 2018, 12:22 a.m. | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> I was comparing test results from 20180207 and today, I
Pedro> noticed that gdb.rust/simple.exp regressed:

Thanks.

Pedro> I haven't bisected, but the references to discriminant info make me suspect
Pedro> this patch I'm replying to.

Yeah, it has to be.

Pedro> This is Fedora 27 with:
Pedro>  $ rustc --version
Pedro>  rustc 1.13.0 (2c6933acc 2016-11-07)

I will look into it tomorrow.

Tom
Jan Kratochvil April 10, 2018, 8:36 p.m. | #4
Hi Tom,

c9317f214b274b805190b8e878c79f4181d93bb4 is the first bad commit
commit c9317f214b274b805190b8e878c79f4181d93bb4
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Feb 9 13:31:51 2018 -0700     
    Convert Rust to use discriminated unions

https://people.redhat.com/jkratoch/rustgdbbug.tar.xz

./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug 
Segmentation fault


Jan
Tom Tromey April 11, 2018, 2:52 a.m. | #5
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:


Jan> Hi Tom,
Jan> c9317f214b274b805190b8e878c79f4181d93bb4 is the first bad commit
Jan> commit c9317f214b274b805190b8e878c79f4181d93bb4
Jan> Author: Tom Tromey <tom@tromey.com>
Jan> Date:   Fri Feb 9 13:31:51 2018 -0700     
Jan>     Convert Rust to use discriminated unions

Jan> https://people.redhat.com/jkratoch/rustgdbbug.tar.xz

Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug 
Jan> Segmentation fault

Try this:

https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html

Tom
Jan Kratochvil April 11, 2018, 7:04 a.m. | #6
On Wed, 11 Apr 2018 04:52:09 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug 

> Jan> Segmentation fault

> 

> Try this:

> 

> https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html


Still crashing:

#0  __strrchr_avx2 () at ../sysdeps/x86_64/multiarch/strrchr-avx2.S:54
#1  0x0000000000aaff44 in rust_last_path_segment (path=0x0) at rust-lang.c:44
#2  0x00000000008ca07d in quirk_rust_enum (type=0x4c53b80, objfile=0x3089a00) at dwarf2read.c:10076
#3  0x00000000008ca3cd in rust_union_quirks (cu=0x30fc820) at dwarf2read.c:10101
#4  0x00000000008cab85 in process_full_comp_unit (per_cu=0x3149f10, pretend_language=language_minimal) at dwarf2read.c:10289
#5  0x00000000008c7cfa in process_queue (dwarf2_per_objfile=0x329f5d0) at dwarf2read.c:9501
#6  0x00000000008b4edb in dw2_do_instantiate_symtab (per_cu=0x3149f10) at dwarf2read.c:2888
#7  0x00000000008b4fd2 in dw2_instantiate_symtab (per_cu=0x3149f10) at dwarf2read.c:2909
#8  0x00000000008b9566 in dw2_expand_all_symtabs (objfile=0x3089a00) at dwarf2read.c:4147


Jan
Tom Tromey April 11, 2018, 7:49 p.m. | #7
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:


Jan> On Wed, 11 Apr 2018 04:52:09 +0200, Tom Tromey wrote:
>> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> ./gdb/gdb -batch -readnow /tmp/rustgdbbug/a/b/rustdoc-1.24.0-2.fc27.x86_64.debug 
Jan> Segmentation fault
>> 

>> Try this:

>> 

>> https://sourceware.org/ml/gdb-patches/2018-03/msg00600.html


Jan> Still crashing:

Jan> #0  __strrchr_avx2 () at ../sysdeps/x86_64/multiarch/strrchr-avx2.S:54
Jan> #1  0x0000000000aaff44 in rust_last_path_segment (path=0x0) at rust-lang.c:44
Jan> #2  0x00000000008ca07d in quirk_rust_enum (type=0x4c53b80, objfile=0x3089a00) at dwarf2read.c:10076
Jan> #3  0x00000000008ca3cd in rust_union_quirks (cu=0x30fc820) at dwarf2read.c:10101
Jan> #4 0x00000000008cab85 in process_full_comp_unit (per_cu=0x3149f10,

I think this another variant of
https://sourceware.org/bugzilla/show_bug.cgi?id=23010

The immediate bug here is that the (rust) enum member is being read from
a partial unit, but it is being read as language_minimal, not language_rust.
This causes it to have a TYPE_NAME==NULL, whereas language_rust would
have set it to follow TYPE_TAG_NAME.

I have a hack to fix that but it reveals another problem, which is that
some of the type rewriting can be done multiple times, causing other bugs.
I hadn't considered this possibility.  I'm looking into a good way to
fix it.

Tom
Tom Tromey April 12, 2018, 6:09 p.m. | #8
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


Tom> I have a hack to fix that but it reveals another problem, which is that
Tom> some of the type rewriting can be done multiple times, causing other bugs.
Tom> I hadn't considered this possibility.  I'm looking into a good way to
Tom> fix it.

This analysis was mistaken, and it turned out to be simpler -- good
news.

I will send out my patches, but writing a test case for 23010 seems
quite painful.  Based on the stack trace I got, and investigating the
DWARF of the reproducer, you need:

* A partial CU with no language, which has a function that has some parameters
* A full C++ CU that references that function using DW_AT_abstract_origin

In the webkit reproducer this DW_AT_abstract_origin occurs in an inlined
function IIRC.

Tom
Keith Seitz April 12, 2018, 6:45 p.m. | #9
On 04/12/2018 11:09 AM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

> 

> Tom> I have a hack to fix that but it reveals another problem, which is that

> Tom> some of the type rewriting can be done multiple times, causing other bugs.

> Tom> I hadn't considered this possibility.  I'm looking into a good way to

> Tom> fix it.

> 

> This analysis was mistaken, and it turned out to be simpler -- good

> news.


This is excellent! I am looking forward to reading about this.

> I will send out my patches, but writing a test case for 23010 seems

> quite painful.  Based on the stack trace I got, and investigating the

> DWARF of the reproducer, you need:

> 

> * A partial CU with no language, which has a function that has some parameters

> * A full C++ CU that references that function using DW_AT_abstract_origin

> 

> In the webkit reproducer this DW_AT_abstract_origin occurs in an inlined

> function IIRC.


It /is/ painful. Very. I have extracted (most of?) the DIE tree from the webkit reproducer, but it does not reproduce the problem, so I am missing something in my test. Maybe it's the inlined function... [If you'd like it, just lemme know. I may yet play with it some more (re: inlined function).]

Keith

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2b9d886b76..74efd6383a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,33 @@ 
 2018-02-22  Tom Tromey  <tom@tromey.com>
 
+	* rust-lang.h (rust_last_path_segment): Declare.
+	* rust-lang.c (rust_last_path_segment): Now public.  Change
+	contract.
+	(struct disr_info): Remove.
+	(RUST_ENUM_PREFIX, RUST_ENCODED_ENUM_REAL)
+	(RUST_ENCODED_ENUM_HIDDEN, rust_union_is_untagged)
+	(rust_get_disr_info, rust_tuple_variant_type_p): Remove.
+	(rust_enum_p, rust_enum_variant): New function.
+	(rust_underscore_fields): Remove "offset" parameter.
+	(rust_print_enum): New function.
+	(rust_val_print) <TYPE_CODE_UNION>: Remove enum code.
+	<TYPE_CODE_STRUCT>: Call rust_print_enum when appropriate.
+	(rust_print_struct_def): Add "for_rust_enum" parameter.  Handle
+	enums.
+	(rust_internal_print_type): New function, from rust_print_type.
+	Remove enum code.
+	(rust_print_type): Call rust_internal_print_type.
+	(rust_evaluate_subexp) <STRUCTOP_ANONYMOUS, STRUCTOP_STRUCT>:
+	Update enum handling.
+	* dwarf2read.c (struct dwarf2_cu) <rust_unions>: New field.
+	(rust_fully_qualify, alloc_discriminant_info, quirk_rust_enum)
+	(rust_union_quirks): New functions.
+	(process_full_comp_unit, process_full_type_unit): Call
+	rust_union_quirks.
+	(process_structure_scope): Update rust_unions if necessary.
+
+2018-02-22  Tom Tromey  <tom@tromey.com>
+
 	* value.h (value_union_variant): Declare.
 	* valops.c (value_union_variant): New function.
 	* gdbtypes.h (TYPE_FLAG_DISCRIMINATED_UNION): New macro.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6cdb96335f..c6e76caf1d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -86,6 +86,7 @@ 
 #include <cmath>
 #include <set>
 #include <forward_list>
+#include "rust-lang.h"
 
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
@@ -762,6 +763,14 @@  struct dwarf2_cu
      whether the DW_AT_ranges attribute came from the skeleton or DWO.  */
   ULONGEST ranges_base = 0;
 
+  /* When reading debug info generated by older versions of rustc, we
+     have to rewrite some union types to be struct types with a
+     variant part.  This rewriting must be done after the CU is fully
+     read in, because otherwise at the point of rewriting some struct
+     type might not have been fully processed.  So, we keep a list of
+     all such types here and process them after expansion.  */
+  std::vector<struct type *> rust_unions;
+
   /* Mark used when releasing cached dies.  */
   unsigned int mark : 1;
 
@@ -10274,6 +10283,302 @@  fixup_go_packaging (struct dwarf2_cu *cu)
     }
 }
 
+/* Allocate a fully-qualified name consisting of the two parts on the
+   obstack.  */
+
+static const char *
+rust_fully_qualify (struct obstack *obstack, const char *p1, const char *p2)
+{
+  return obconcat (obstack, p1, "::", p2, (char *) NULL);
+}
+
+/* A helper that allocates a struct discriminant_info to attach to a
+   union type.  */
+
+static struct discriminant_info *
+alloc_discriminant_info (struct type *type, int discriminant_index,
+			 int default_index)
+{
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION);
+  gdb_assert (default_index == -1
+	      || (default_index > 0 && default_index < TYPE_NFIELDS (type)));
+
+  TYPE_FLAG_DISCRIMINATED_UNION (type) = 1;
+
+  struct discriminant_info *disc
+    = ((struct discriminant_info *)
+       TYPE_ZALLOC (type,
+		    offsetof (struct discriminant_info, discriminants)
+		    + TYPE_NFIELDS (type) * sizeof (disc->discriminants[0])));
+  disc->default_index = default_index;
+  disc->discriminant_index = discriminant_index;
+
+  struct dynamic_prop prop;
+  prop.kind = PROP_UNDEFINED;
+  prop.data.baton = disc;
+
+  add_dyn_prop (DYN_PROP_DISCRIMINATED, prop, type);
+
+  return disc;
+}
+
+/* Some versions of rustc emitted enums in an unusual way.
+
+   Ordinary enums were emitted as unions.  The first element of each
+   structure in the union was named "RUST$ENUM$DISR".  This element
+   held the discriminant.
+
+   These versions of Rust also implemented the "non-zero"
+   optimization.  When the enum had two values, and one is empty and
+   the other holds a pointer that cannot be zero, the pointer is used
+   as the discriminant, with a zero value meaning the empty variant.
+   Here, the union's first member is of the form
+   RUST$ENCODED$ENUM$<fieldno>$<fieldno>$...$<variantname>
+   where the fieldnos are the indices of the fields that should be
+   traversed in order to find the field (which may be several fields deep)
+   and the variantname is the name of the variant of the case when the
+   field is zero.
+
+   This function recognizes whether TYPE is of one of these forms,
+   and, if so, smashes it to be a variant type.  */
+
+static void
+quirk_rust_enum (struct type *type, struct objfile *objfile)
+{
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION);
+
+  /* We don't need to deal with empty enums.  */
+  if (TYPE_NFIELDS (type) == 0)
+    return;
+
+#define RUST_ENUM_PREFIX "RUST$ENCODED$ENUM$"
+  if (TYPE_NFIELDS (type) == 1
+      && startswith (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX))
+    {
+      const char *name = TYPE_FIELD_NAME (type, 0) + strlen (RUST_ENUM_PREFIX);
+
+      /* Decode the field name to find the offset of the
+	 discriminant.  */
+      ULONGEST bit_offset = 0;
+      struct type *field_type = TYPE_FIELD_TYPE (type, 0);
+      while (name[0] >= '0' && name[0] <= '9')
+	{
+	  char *tail;
+	  unsigned long index = strtoul (name, &tail, 10);
+	  name = tail;
+	  if (*name != '$'
+	      || index >= TYPE_NFIELDS (field_type)
+	      || (TYPE_FIELD_LOC_KIND (field_type, index)
+		  != FIELD_LOC_KIND_BITPOS))
+	    {
+	      complaint (&symfile_complaints,
+			 _("Could not parse Rust enum encoding string \"%s\""
+			   "[in module %s]"),
+			 TYPE_FIELD_NAME (type, 0),
+			 objfile_name (objfile));
+	      return;
+	    }
+	  ++name;
+
+	  bit_offset += TYPE_FIELD_BITPOS (field_type, index);
+	  field_type = TYPE_FIELD_TYPE (field_type, index);
+	}
+
+      /* Make a union to hold the variants.  */
+      struct type *union_type = alloc_type (objfile);
+      TYPE_CODE (union_type) = TYPE_CODE_UNION;
+      TYPE_NFIELDS (union_type) = 3;
+      TYPE_FIELDS (union_type)
+	= (struct field *) TYPE_ZALLOC (type, 3 * sizeof (struct field));
+      TYPE_LENGTH (union_type) = TYPE_LENGTH (type);
+
+      /* Put the discriminant must at index 0.  */
+      TYPE_FIELD_TYPE (union_type, 0) = field_type;
+      TYPE_FIELD_ARTIFICIAL (union_type, 0) = 1;
+      TYPE_FIELD_NAME (union_type, 0) = "<<discriminant>>";
+      SET_FIELD_BITPOS (TYPE_FIELD (union_type, 0), bit_offset);
+
+      /* The order of fields doesn't really matter, so put the real
+	 field at index 1 and the data-less field at index 2.  */
+      struct discriminant_info *disc
+	= alloc_discriminant_info (union_type, 0, 1);
+      TYPE_FIELD (union_type, 1) = TYPE_FIELD (type, 0);
+      TYPE_FIELD_NAME (union_type, 1)
+	= rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (union_type, 1)));
+      TYPE_NAME (TYPE_FIELD_TYPE (union_type, 1))
+	= rust_fully_qualify (&objfile->objfile_obstack, TYPE_NAME (type),
+			      TYPE_FIELD_NAME (union_type, 1));
+
+      const char *dataless_name
+	= rust_fully_qualify (&objfile->objfile_obstack, TYPE_NAME (type),
+			      name);
+      struct type *dataless_type = init_type (objfile, TYPE_CODE_VOID, 0,
+					      dataless_name);
+      TYPE_FIELD_TYPE (union_type, 2) = dataless_type;
+      /* NAME points into the original discriminant name, which
+	 already has the correct lifetime.  */
+      TYPE_FIELD_NAME (union_type, 2) = name;
+      SET_FIELD_BITPOS (TYPE_FIELD (union_type, 2), 0);
+      disc->discriminants[2] = 0;
+
+      /* Smash this type to be a structure type.  We have to do this
+	 because the type has already been recorded.  */
+      TYPE_CODE (type) = TYPE_CODE_STRUCT;
+      TYPE_NFIELDS (type) = 1;
+      TYPE_FIELDS (type)
+	= (struct field *) TYPE_ZALLOC (type, sizeof (struct field));
+
+      /* Install the variant part.  */
+      TYPE_FIELD_TYPE (type, 0) = union_type;
+      SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0);
+      TYPE_FIELD_NAME (type, 0) = "<<variants>>";
+    }
+  else if (TYPE_NFIELDS (type) == 1)
+    {
+      /* We assume that a union with a single field is a univariant
+	 enum.  */
+      /* Smash this type to be a structure type.  We have to do this
+	 because the type has already been recorded.  */
+      TYPE_CODE (type) = TYPE_CODE_STRUCT;
+
+      /* Make a union to hold the variants.  */
+      struct type *union_type = alloc_type (objfile);
+      TYPE_CODE (union_type) = TYPE_CODE_UNION;
+      TYPE_NFIELDS (union_type) = TYPE_NFIELDS (type);
+      TYPE_LENGTH (union_type) = TYPE_LENGTH (type);
+      TYPE_FIELDS (union_type) = TYPE_FIELDS (type);
+
+      struct type *field_type = TYPE_FIELD_TYPE (union_type, 0);
+      const char *variant_name
+	= rust_last_path_segment (TYPE_NAME (field_type));
+      TYPE_FIELD_NAME (union_type, 0) = variant_name;
+      TYPE_NAME (field_type)
+	= rust_fully_qualify (&objfile->objfile_obstack,
+			      TYPE_NAME (field_type), variant_name);
+
+      /* Install the union in the outer struct type.  */
+      TYPE_NFIELDS (type) = 1;
+      TYPE_FIELDS (type)
+	= (struct field *) TYPE_ZALLOC (union_type, sizeof (struct field));
+      TYPE_FIELD_TYPE (type, 0) = union_type;
+      TYPE_FIELD_NAME (type, 0) = "<<variants>>";
+      SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0);
+
+      alloc_discriminant_info (union_type, -1, 0);
+    }
+  else
+    {
+      struct type *disr_type = nullptr;
+      for (int i = 0; i < TYPE_NFIELDS (type); ++i)
+	{
+	  disr_type = TYPE_FIELD_TYPE (type, i);
+
+	  if (TYPE_NFIELDS (disr_type) == 0)
+	    {
+	      /* Could be data-less variant, so keep going.  */
+	    }
+	  else if (strcmp (TYPE_FIELD_NAME (disr_type, 0),
+			   "RUST$ENUM$DISR") != 0)
+	    {
+	      /* Not a Rust enum.  */
+	      return;
+	    }
+	  else
+	    {
+	      /* Found one.  */
+	      break;
+	    }
+	}
+
+      /* If we got here without a discriminant, then it's probably
+	 just a union.  */
+      if (disr_type == nullptr)
+	return;
+
+      /* Smash this type to be a structure type.  We have to do this
+	 because the type has already been recorded.  */
+      TYPE_CODE (type) = TYPE_CODE_STRUCT;
+
+      /* Make a union to hold the variants.  */
+      struct field *disr_field = &TYPE_FIELD (disr_type, 0);
+      struct type *union_type = alloc_type (objfile);
+      TYPE_CODE (union_type) = TYPE_CODE_UNION;
+      TYPE_NFIELDS (union_type) = 1 + TYPE_NFIELDS (type);
+      TYPE_LENGTH (union_type) = TYPE_LENGTH (type);
+      TYPE_FIELDS (union_type)
+	= (struct field *) TYPE_ZALLOC (union_type,
+					(TYPE_NFIELDS (union_type)
+					 * sizeof (struct field)));
+
+      memcpy (TYPE_FIELDS (union_type) + 1, TYPE_FIELDS (type),
+	      TYPE_NFIELDS (type) * sizeof (struct field));
+
+      /* Install the discriminant at index 0 in the union.  */
+      TYPE_FIELD (union_type, 0) = *disr_field;
+      TYPE_FIELD_ARTIFICIAL (union_type, 0) = 1;
+      TYPE_FIELD_NAME (union_type, 0) = "<<discriminant>>";
+
+      /* Install the union in the outer struct type.  */
+      TYPE_FIELD_TYPE (type, 0) = union_type;
+      TYPE_FIELD_NAME (type, 0) = "<<variants>>";
+      TYPE_NFIELDS (type) = 1;
+
+      /* Set the size and offset of the union type.  */
+      SET_FIELD_BITPOS (TYPE_FIELD (type, 0), 0);
+
+      /* We need a way to find the correct discriminant given a
+	 variant name.  For convenience we build a map here.  */
+      struct type *enum_type = FIELD_TYPE (*disr_field);
+      std::unordered_map<std::string, ULONGEST> discriminant_map;
+      for (int i = 0; i < TYPE_NFIELDS (enum_type); ++i)
+	{
+	  if (TYPE_FIELD_LOC_KIND (enum_type, i) == FIELD_LOC_KIND_ENUMVAL)
+	    {
+	      const char *name
+		= rust_last_path_segment (TYPE_FIELD_NAME (enum_type, i));
+	      discriminant_map[name] = TYPE_FIELD_ENUMVAL (enum_type, i);
+	    }
+	}
+
+      int n_fields = TYPE_NFIELDS (union_type);
+      struct discriminant_info *disc
+	= alloc_discriminant_info (union_type, 0, -1);
+      /* Skip the discriminant here.  */
+      for (int i = 1; i < n_fields; ++i)
+	{
+	  /* Find the final word in the name of this variant's type.
+	     That name can be used to look up the correct
+	     discriminant.  */
+	  const char *variant_name
+	    = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (union_type,
+								  i)));
+
+	  auto iter = discriminant_map.find (variant_name);
+	  if (iter != discriminant_map.end ())
+	    disc->discriminants[i] = iter->second;
+
+	  /* Remove the discriminant field.  */
+	  struct type *sub_type = TYPE_FIELD_TYPE (union_type, i);
+	  --TYPE_NFIELDS (sub_type);
+	  ++TYPE_FIELDS (sub_type);
+	  TYPE_FIELD_NAME (union_type, i) = variant_name;
+	  TYPE_NAME (sub_type)
+	    = rust_fully_qualify (&objfile->objfile_obstack,
+				  TYPE_NAME (type), variant_name);
+	}
+    }
+}
+
+/* Rewrite some Rust unions to be structures with variants parts.  */
+
+static void
+rust_union_quirks (struct dwarf2_cu *cu)
+{
+  gdb_assert (cu->language == language_rust);
+  for (struct type *type : cu->rust_unions)
+    quirk_rust_enum (type, cu->per_cu->dwarf2_per_objfile->objfile);
+}
+
 /* Return the symtab for PER_CU.  This works properly regardless of
    whether we're using the index or psymtabs.  */
 
@@ -10458,6 +10763,9 @@  process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
      physnames.  */
   compute_delayed_physnames (cu);
 
+  if (cu->language == language_rust)
+    rust_union_quirks (cu);
+
   /* Some compilers don't define a DW_AT_high_pc attribute for the
      compilation unit.  If the DW_AT_high_pc is missing, synthesize
      it, by scanning the DIE's below the compilation unit.  */
@@ -10560,6 +10868,9 @@  process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
      physnames.  */
   compute_delayed_physnames (cu);
 
+  if (cu->language == language_rust)
+    rust_union_quirks (cu);
+
   /* TUs share symbol tables.
      If this is the first TU to use this symtab, complete the construction
      of it with end_expandable_symtab.  Otherwise, complete the addition of
@@ -16138,6 +16449,8 @@  process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     }
 
   quirk_gcc_member_function_pointer (type, objfile);
+  if (cu->language == language_rust && die->tag == DW_TAG_union_type)
+    cu->rust_unions.push_back (type);
 
   /* NOTE: carlton/2004-03-16: GCC 3.4 (or at least one of its
      snapshots) has been known to create a die giving a declaration
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 5ff80b249b..35126da7a6 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -36,17 +36,15 @@ 
 #include <string>
 #include <vector>
 
-/* Returns the last segment of a Rust path like foo::bar::baz.  Will
-   not handle cases where the last segment contains generics.  This
-   will return NULL if the last segment cannot be found.  */
+/* See rust-lang.h.  */
 
-static const char *
-rust_last_path_segment (const char * path)
+const char *
+rust_last_path_segment (const char *path)
 {
   const char *result = strrchr (path, ':');
 
   if (result == NULL)
-    return NULL;
+    return path;
   return result + 1;
 }
 
@@ -63,209 +61,29 @@  rust_crate_for_block (const struct block *block)
   return std::string (scope, cp_find_first_component (scope));
 }
 
-/* Information about the discriminant/variant of an enum */
-
-struct disr_info
-{
-  /* Name of field.  */
-  std::string name;
-  /* Field number in union.  Negative on error.  For an encoded enum,
-     the "hidden" member will always be field 1, and the "real" member
-     will always be field 0.  */
-  int field_no;
-  /* True if this is an encoded enum that has a single "real" member
-     and a single "hidden" member.  */
-  unsigned int is_encoded : 1;
-};
-
-/* The prefix of a specially-encoded enum.  */
+/* Return true if TYPE, which must be a struct type, represents a Rust
+   enum.  */
 
-#define RUST_ENUM_PREFIX "RUST$ENCODED$ENUM$"
-
-/* The number of the real field.  */
-
-#define RUST_ENCODED_ENUM_REAL 0
-
-/* The number of the hidden field.  */
-
-#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)
+rust_enum_p (const 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;
+  return (TYPE_CODE (type) == TYPE_CODE_STRUCT
+	  && TYPE_NFIELDS (type) == 1
+	  && TYPE_FLAG_DISCRIMINATED_UNION (TYPE_FIELD_TYPE (type, 0)));
 }
 
-/* Utility function to get discriminant info for a given value.  */
+/* Given an enum type and contents, find which variant is active.  */
 
-static struct disr_info
-rust_get_disr_info (struct type *type, const gdb_byte *valaddr,
-                    int embedded_offset, CORE_ADDR address,
-                    struct value *val)
+struct field *
+rust_enum_variant (struct type *type, const gdb_byte *contents)
 {
-  int i;
-  struct disr_info ret;
-  struct type *disr_type;
-  struct value_print_options opts;
-  const char *name_segment;
-
-  get_no_prettyformat_print_options (&opts);
-
-  ret.field_no = -1;
-  ret.is_encoded = 0;
-
-  if (TYPE_NFIELDS (type) == 0)
-    error (_("Encountered void enum value"));
-
-  /* If an enum has two values where one is empty and the other holds
-     a pointer that cannot be zero; then the Rust compiler optimizes
-     away the discriminant and instead uses a zero value in the
-     pointer field to indicate the empty variant.  */
-  if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
-	       strlen (RUST_ENUM_PREFIX)) == 0)
-    {
-      char *tail, *token, *saveptr = NULL;
-      unsigned long fieldno;
-      struct type *member_type;
-      LONGEST value;
-
-      ret.is_encoded = 1;
-
-      if (TYPE_NFIELDS (type) != 1)
-	error (_("Only expected one field in %s type"), RUST_ENUM_PREFIX);
-
-      /* Optimized enums have only one field.  */
-      member_type = TYPE_FIELD_TYPE (type, 0);
-
-      std::string name (TYPE_FIELD_NAME (type, 0));
-      tail = &name[0] + strlen (RUST_ENUM_PREFIX);
-
-      /* The location of the value that doubles as a discriminant is
-         stored in the name of the field, as
-         RUST$ENCODED$ENUM$<fieldno>$<fieldno>$...$<variantname>
-         where the fieldnos are the indices of the fields that should be
-         traversed in order to find the field (which may be several fields deep)
-         and the variantname is the name of the variant of the case when the
-         field is zero.  */
-      for (token = strtok_r (tail, "$", &saveptr);
-           token != NULL;
-           token = strtok_r (NULL, "$", &saveptr))
-        {
-	  if (sscanf (token, "%lu", &fieldno) != 1)
-	    {
-	      /* We have reached the enum name, which cannot start
-		 with a digit.  */
-	      break;
-	    }
-          if (fieldno >= TYPE_NFIELDS (member_type))
-	    error (_("%s refers to field after end of member type"),
-		   RUST_ENUM_PREFIX);
-
-          embedded_offset += TYPE_FIELD_BITPOS (member_type, fieldno) / 8;
-          member_type = TYPE_FIELD_TYPE (member_type, fieldno);
-        }
-
-      if (token == NULL)
-	error (_("Invalid form for %s"), RUST_ENUM_PREFIX);
-      value = unpack_long (member_type, valaddr + embedded_offset);
-
-      if (value == 0)
-	{
-	  ret.field_no = RUST_ENCODED_ENUM_HIDDEN;
-	  ret.name = std::string (TYPE_NAME (type)) + "::" + token;
-	}
-      else
-	{
-	  ret.field_no = RUST_ENCODED_ENUM_REAL;
-	  ret.name = (std::string (TYPE_NAME (type)) + "::"
-		      + rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, 0))));
-	}
-
-      return ret;
-    }
-
-  disr_type = TYPE_FIELD_TYPE (type, 0);
-
-  if (TYPE_NFIELDS (disr_type) == 0)
-    {
-      /* This is a bounds check and should never be hit unless Rust
-	 has changed its debuginfo format.  */
-      error (_("Could not find enum discriminant field"));
-    }
-  else if (TYPE_NFIELDS (type) == 1)
-    {
-      /* Sometimes univariant enums are encoded without a
-         discriminant.  In that case, treating it as an encoded enum
-         with the first field being the actual type works.  */
-      const char *field_name = TYPE_NAME (TYPE_FIELD_TYPE (type, 0));
-      const char *last = rust_last_path_segment (field_name);
-      ret.name = std::string (TYPE_NAME (type)) + "::" + last;
-      ret.field_no = RUST_ENCODED_ENUM_REAL;
-      ret.is_encoded = 1;
-      return ret;
-    }
-
-  if (strcmp (TYPE_FIELD_NAME (disr_type, 0), "RUST$ENUM$DISR") != 0)
-    error (_("Rust debug format has changed"));
-
-  string_file temp_file;
-  /* The first value of the first field (or any field)
-     is the discriminant value.  */
-  c_val_print (TYPE_FIELD_TYPE (disr_type, 0),
-	       (embedded_offset + TYPE_FIELD_BITPOS (type, 0) / 8
-		+ TYPE_FIELD_BITPOS (disr_type, 0) / 8),
-	       address, &temp_file,
-	       0, val, &opts);
-
-  ret.name = std::move (temp_file.string ());
-  name_segment = rust_last_path_segment (ret.name.c_str ());
-  if (name_segment != NULL)
-    {
-      for (i = 0; i < TYPE_NFIELDS (type); ++i)
-	{
-	  /* Sadly, the discriminant value paths do not match the type
-	     field name paths ('core::option::Option::Some' vs
-	     'core::option::Some').  However, enum variant names are
-	     unique in the last path segment and the generics are not
-	     part of this path, so we can just compare those.  This is
-	     hackish and would be better fixed by improving rustc's
-	     metadata for enums.  */
-	  const char *field_type = TYPE_NAME (TYPE_FIELD_TYPE (type, i));
-
-	  if (field_type != NULL
-	      && strcmp (name_segment,
-			 rust_last_path_segment (field_type)) == 0)
-	    {
-	      ret.field_no = i;
-	      break;
-	    }
-	}
-    }
+  /* In Rust the enum always fills the containing structure.  */
+  gdb_assert (TYPE_FIELD_BITPOS (type, 0) == 0);
 
-  if (ret.field_no == -1 && !ret.name.empty ())
-    {
-      /* Somehow the discriminant wasn't found.  */
-      error (_("Could not find variant of %s with discriminant %s"),
-	     TYPE_TAG_NAME (type), ret.name.c_str ());
-    }
+  struct type *union_type = TYPE_FIELD_TYPE (type, 0);
 
-  return ret;
+  int fieldno = value_union_variant (union_type, contents);
+  return &TYPE_FIELD (union_type, fieldno);
 }
 
 /* See rust-lang.h.  */
@@ -281,12 +99,11 @@  rust_tuple_type_p (struct type *type)
 	  && TYPE_TAG_NAME (type)[0] == '(');
 }
 
-
 /* Return true if all non-static fields of a structlike type are in a
-   sequence like __0, __1, __2.  OFFSET lets us skip fields.  */
+   sequence like __0, __1, __2.  */
 
 static bool
-rust_underscore_fields (struct type *type, int offset)
+rust_underscore_fields (struct type *type)
 {
   int i, field_number;
 
@@ -298,17 +115,12 @@  rust_underscore_fields (struct type *type, int offset)
     {
       if (!field_is_static (&TYPE_FIELD (type, i)))
 	{
-	  if (offset > 0)
-	    offset--;
-	  else
-	    {
-	      char buf[20];
+	  char buf[20];
 
-	      xsnprintf (buf, sizeof (buf), "__%d", field_number);
-	      if (strcmp (buf, TYPE_FIELD_NAME (type, i)) != 0)
-		return false;
-	      field_number++;
-	    }
+	  xsnprintf (buf, sizeof (buf), "__%d", field_number);
+	  if (strcmp (buf, TYPE_FIELD_NAME (type, i)) != 0)
+	    return false;
+	  field_number++;
 	}
     }
   return true;
@@ -322,16 +134,7 @@  rust_tuple_struct_type_p (struct type *type)
   /* This is just an approximation until DWARF can represent Rust more
      precisely.  We exclude zero-length structs because they may not
      be tuple structs, and there's no way to tell.  */
-  return TYPE_NFIELDS (type) > 0 && rust_underscore_fields (type, 0);
-}
-
-/* Return true if a variant TYPE is a tuple variant, false otherwise.  */
-
-static bool
-rust_tuple_variant_type_p (struct type *type)
-{
-  /* First field is discriminant */
-  return rust_underscore_fields (type, 1);
+  return TYPE_NFIELDS (type) > 0 && rust_underscore_fields (type);
 }
 
 /* Return true if TYPE is a slice type, otherwise false.  */
@@ -516,7 +319,7 @@  rust_val_print_str (struct ui_file *stream, struct value *val,
 		    options);
 }
 
-/* rust_print_type branch for structs and untagged unions.  */
+/* rust_val_print helper for structs and untagged unions.  */
 
 static void
 val_print_struct (struct type *type, int embedded_offset,
@@ -601,6 +404,69 @@  val_print_struct (struct type *type, int embedded_offset,
     fputs_filtered ("}", stream);
 }
 
+/* rust_val_print helper for discriminated unions (Rust enums).  */
+
+static void
+rust_print_enum (struct type *type, int embedded_offset,
+		 CORE_ADDR address, struct ui_file *stream,
+		 int recurse, struct value *val,
+		 const struct value_print_options *options)
+{
+  struct value_print_options opts = *options;
+
+  opts.deref_ref = 0;
+
+  const gdb_byte *valaddr = value_contents_for_printing (val);
+  struct field *variant_field = rust_enum_variant (type, valaddr);
+  embedded_offset += FIELD_BITPOS (*variant_field) / 8;
+  struct type *variant_type = FIELD_TYPE (*variant_field);
+
+  int nfields = TYPE_NFIELDS (variant_type);
+
+  bool is_tuple = rust_tuple_struct_type_p (variant_type);
+
+  fprintf_filtered (stream, "%s", TYPE_NAME (variant_type));
+  if (nfields == 0)
+    {
+      /* In case of a nullary variant like 'None', just output
+	 the name. */
+      return;
+    }
+
+  /* In case of a non-nullary variant, we output 'Foo(x,y,z)'. */
+  if (is_tuple)
+    fprintf_filtered (stream, "(");
+  else
+    {
+      /* struct variant.  */
+      fprintf_filtered (stream, "{");
+    }
+
+  bool first_field = true;
+  for (int j = 0; j < TYPE_NFIELDS (variant_type); j++)
+    {
+      if (!first_field)
+	fputs_filtered (", ", stream);
+      first_field = false;
+
+      if (!is_tuple)
+	fprintf_filtered (stream, "%s: ",
+			  TYPE_FIELD_NAME (variant_type, j));
+
+      val_print (TYPE_FIELD_TYPE (variant_type, j),
+		 (embedded_offset
+		  + TYPE_FIELD_BITPOS (variant_type, j) / 8),
+		 address,
+		 stream, recurse + 1, val, &opts,
+		 current_language);
+    }
+
+  if (is_tuple)
+    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
@@ -707,93 +573,22 @@  rust_val_print (struct type *type, int embedded_offset,
       break;
 
     case TYPE_CODE_UNION:
-      {
-	int j, nfields, first_field, is_tuple, start;
-	struct type *variant_type;
-	struct disr_info disr;
-	struct value_print_options opts;
-
-	/* 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, embedded_offset, address, stream,
-			      recurse, val, options);
-	    break;
-	  }
-
-	opts = *options;
-	opts.deref_ref = 0;
-
-	disr = rust_get_disr_info (type, valaddr, embedded_offset, address,
-				   val);
-
-	if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN)
-	  {
-	    fprintf_filtered (stream, "%s", disr.name.c_str ());
-	    break;
-	  }
-
-	first_field = 1;
-	variant_type = TYPE_FIELD_TYPE (type, disr.field_no);
-	nfields = TYPE_NFIELDS (variant_type);
-
-	is_tuple = (disr.is_encoded
-		    ? rust_tuple_struct_type_p (variant_type)
-		    : rust_tuple_variant_type_p (variant_type));
-	start = disr.is_encoded ? 0 : 1;
-
-	if (nfields > start)
-	  {
-	    /* In case of a non-nullary variant, we output 'Foo(x,y,z)'. */
-	    if (is_tuple)
-	      fprintf_filtered (stream, "%s(", disr.name.c_str ());
-	    else
-	      {
-		/* struct variant.  */
-		fprintf_filtered (stream, "%s{", disr.name.c_str ());
-	      }
-	  }
-	else
-	  {
-	    /* In case of a nullary variant like 'None', just output
-	       the name. */
-	    fprintf_filtered (stream, "%s", disr.name.c_str ());
-	    break;
-	  }
-
-	for (j = start; j < TYPE_NFIELDS (variant_type); j++)
-	  {
-	    if (!first_field)
-	      fputs_filtered (", ", stream);
-	    first_field = 0;
-
-	    if (!is_tuple)
-	      fprintf_filtered (stream, "%s: ",
-				TYPE_FIELD_NAME (variant_type, j));
-
-	    val_print (TYPE_FIELD_TYPE (variant_type, j),
-		       (embedded_offset
-			+ TYPE_FIELD_BITPOS (type, disr.field_no) / 8
-			+ TYPE_FIELD_BITPOS (variant_type, j) / 8),
-		       address,
-		       stream, recurse + 1, val, &opts,
-		       current_language);
-	  }
-
-	if (is_tuple)
-	  fputs_filtered (")", stream);
-	else
-	  fputs_filtered ("}", stream);
-      }
+      /* 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.  */
+      val_print_struct (type, embedded_offset, address, stream,
+			recurse, val, options);
       break;
 
     case TYPE_CODE_STRUCT:
-      val_print_struct (type, embedded_offset, address, stream,
-			recurse, val, options);
+      if (rust_enum_p (type))
+	rust_print_enum (type, embedded_offset, address, stream,
+			 recurse, val, options);
+      else
+	val_print_struct (type, embedded_offset, address, stream,
+			  recurse, val, options);
       break;
 
     default:
@@ -807,19 +602,18 @@  rust_val_print (struct type *type, int embedded_offset,
 
 
 static void
-rust_print_type (struct type *type, const char *varstring,
-		 struct ui_file *stream, int show, int level,
-		 const struct type_print_options *flags);
+rust_internal_print_type (struct type *type, const char *varstring,
+			  struct ui_file *stream, int show, int level,
+			  const struct type_print_options *flags,
+			  bool for_rust_enum);
 
 /* 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)
+		       const struct type_print_options *flags,
+		       bool for_rust_enum)
 {
-  bool is_tuple_struct;
-  int i;
-
   /* Print a tuple type simply.  */
   if (rust_tuple_type_p (type))
     {
@@ -831,22 +625,57 @@  rust_print_struct_def (struct type *type, const char *varstring,
   if (TYPE_N_BASECLASSES (type) > 0)
     c_print_type (type, varstring, stream, show, level, flags);
 
-  /* This code path is also used by unions.  */
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
-    fputs_filtered ("struct ", stream);
-  else
-    fputs_filtered ("union ", stream);
+  /* Compute properties of TYPE here because, in the enum case, the
+     rest of the code ends up looking only at the variant part.  */
+  const char *tagname = TYPE_TAG_NAME (type);
+  bool is_tuple_struct = rust_tuple_struct_type_p (type);
+  bool is_tuple = rust_tuple_type_p (type);
+  bool is_enum = rust_enum_p (type);
+  bool is_univariant = false;
+
+  int enum_discriminant_index = -1;
 
-  if (TYPE_TAG_NAME (type) != NULL)
-    fputs_filtered (TYPE_TAG_NAME (type), stream);
+  if (for_rust_enum)
+    {
+      /* Already printing an outer enum, so nothing to print here.  */
+    }
+  else
+    {
+      /* This code path is also used by unions and enums.  */
+      if (is_enum)
+	{
+	  fputs_filtered ("enum ", stream);
+	  type = TYPE_FIELD_TYPE (type, 0);
+
+	  struct dynamic_prop *discriminant_prop
+	    = get_dyn_prop (DYN_PROP_DISCRIMINATED, type);
+	  struct discriminant_info *info
+	    = (struct discriminant_info *) discriminant_prop->data.baton;
+	  enum_discriminant_index = info->discriminant_index;
+	}
+      else if (TYPE_CODE (type) == TYPE_CODE_UNION && TYPE_NFIELDS (type) == 1)
+	{
+	  /* Probably a univariant enum.  */
+	  fputs_filtered ("enum ", stream);
+	  is_univariant = true;
+	}
+      else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+	fputs_filtered ("struct ", stream);
+      else
+	fputs_filtered ("union ", stream);
 
-  is_tuple_struct = rust_tuple_struct_type_p (type);
+      if (tagname != NULL)
+	fputs_filtered (tagname, stream);
+    }
 
-  if (TYPE_NFIELDS (type) == 0 && !rust_tuple_type_p (type))
+  if (TYPE_NFIELDS (type) == 0 && !is_tuple)
     return;
-  fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream);
+  if (for_rust_enum)
+    fputs_filtered (is_tuple_struct ? "(" : "{", stream);
+  else
+    fputs_filtered (is_tuple_struct ? " (\n" : " {\n", stream);
 
-  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+  for (int i = 0; i < TYPE_NFIELDS (type); ++i)
     {
       QUIT;
       if (field_is_static (&TYPE_FIELD (type, i)))
@@ -858,17 +687,36 @@  rust_print_struct_def (struct type *type, const char *varstring,
 
       /* For a tuple struct we print the type but nothing
 	 else.  */
-      print_spaces_filtered (level + 2, stream);
-      if (!is_tuple_struct)
+      if (!for_rust_enum)
+	print_spaces_filtered (level + 2, stream);
+      if (is_enum)
+	{
+	  if (i == enum_discriminant_index)
+	    continue;
+	  fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	}
+      else if (is_univariant)
+	{
+	  const char *name
+	    = rust_last_path_segment (TYPE_NAME (TYPE_FIELD_TYPE (type, i)));
+	  fputs_filtered (name, stream);
+	}
+      else 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);
+      rust_internal_print_type (TYPE_FIELD_TYPE (type, i), NULL,
+				stream,
+				((is_enum || is_univariant) ? show : show - 1),
+				level + 2, flags, is_enum || is_univariant);
+      if (!for_rust_enum)
+	fputs_filtered (",\n", stream);
+      else if (i + 1 < TYPE_NFIELDS (type))
+	fputs_filtered (", ", stream);
     }
 
-  fprintfi_filtered (level, stream, is_tuple_struct ? ")" : "}");
+  if (!for_rust_enum)
+    print_spaces_filtered (level, stream);
+  fputs_filtered (is_tuple_struct ? ")" : "}", stream);
 }
 
 /* la_print_typedef implementation for Rust.  */
@@ -887,9 +735,10 @@  rust_print_typedef (struct type *type,
 /* la_print_type implementation for Rust.  */
 
 static void
-rust_print_type (struct type *type, const char *varstring,
-		 struct ui_file *stream, int show, int level,
-		 const struct type_print_options *flags)
+rust_internal_print_type (struct type *type, const char *varstring,
+			  struct ui_file *stream, int show, int level,
+			  const struct type_print_options *flags,
+			  bool for_rust_enum)
 {
   int i;
 
@@ -910,7 +759,11 @@  rust_print_type (struct type *type, const char *varstring,
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_VOID:
-      fputs_filtered ("()", stream);
+      /* If we have an enum, we've already printed the type's
+	 unqualified name, and there is nothing else to print
+	 here.  */
+      if (!for_rust_enum)
+	fputs_filtered ("()", stream);
       break;
 
     case TYPE_CODE_FUNC:
@@ -927,15 +780,16 @@  rust_print_type (struct type *type, const char *varstring,
 	  QUIT;
 	  if (i > 0)
 	    fputs_filtered (", ", stream);
-	  rust_print_type (TYPE_FIELD_TYPE (type, i), "", stream, -1, 0,
-			   flags);
+	  rust_internal_print_type (TYPE_FIELD_TYPE (type, i), "", stream,
+				    -1, 0, flags, false);
 	}
       fputs_filtered (")", stream);
       /* If it returns unit, we can omit the return type.  */
       if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_VOID)
         {
           fputs_filtered (" -> ", stream);
-          rust_print_type (TYPE_TARGET_TYPE (type), "", stream, -1, 0, flags);
+          rust_internal_print_type (TYPE_TARGET_TYPE (type), "", stream,
+				    -1, 0, flags, false);
         }
       break;
 
@@ -944,8 +798,8 @@  rust_print_type (struct type *type, const char *varstring,
 	LONGEST low_bound, high_bound;
 
 	fputs_filtered ("[", stream);
-	rust_print_type (TYPE_TARGET_TYPE (type), NULL,
-			 stream, show - 1, level, flags);
+	rust_internal_print_type (TYPE_TARGET_TYPE (type), NULL,
+				  stream, show - 1, level, flags, false);
 
 	if (TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == PROP_LOCEXPR
 	    || TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == PROP_LOCLIST)
@@ -957,8 +811,10 @@  rust_print_type (struct type *type, const char *varstring,
       }
       break;
 
+    case TYPE_CODE_UNION:
     case TYPE_CODE_STRUCT:
-      rust_print_struct_def (type, varstring, stream, show, level, flags);
+      rust_print_struct_def (type, varstring, stream, show, level, flags,
+			     for_rust_enum);
       break;
 
     case TYPE_CODE_ENUM:
@@ -992,93 +848,21 @@  rust_print_type (struct type *type, const char *varstring,
       }
       break;
 
-    case TYPE_CODE_UNION:
-      {
-	/* ADT enums.  */
-	int i;
-	/* 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)
-	  {
-	    fputs_filtered (TYPE_TAG_NAME (type), stream);
-	    fputs_filtered (" ", stream);
-	  }
-	fputs_filtered ("{\n", stream);
-
-	if (strncmp (TYPE_FIELD_NAME (type, 0), RUST_ENUM_PREFIX,
-		     strlen (RUST_ENUM_PREFIX)) == 0)
-	  {
-	    const char *zero_field = strrchr (TYPE_FIELD_NAME (type, 0), '$');
-	    if (zero_field != NULL && strlen (zero_field) > 1)
-	      {
-		fprintfi_filtered (level + 2, stream, "%s,\n", zero_field + 1);
-		/* There is no explicit discriminant field, skip nothing.  */
-		skip_to = 0;
-	      }
-	  }
-	else if (TYPE_NFIELDS (type) == 1)
-	  skip_to = 0;
-
-	for (i = 0; i < TYPE_NFIELDS (type); ++i)
-	  {
-	    struct type *variant_type = TYPE_FIELD_TYPE (type, i);
-	    const char *name
-	      = rust_last_path_segment (TYPE_NAME (variant_type));
-
-	    fprintfi_filtered (level + 2, stream, "%s", name);
-
-	    if (TYPE_NFIELDS (variant_type) > skip_to)
-	      {
-		int first = 1;
-		bool is_tuple = (TYPE_NFIELDS (type) == 1
-				 ? rust_tuple_struct_type_p (variant_type)
-				 : rust_tuple_variant_type_p (variant_type));
-		int j;
-
-		fputs_filtered (is_tuple ? "(" : "{", stream);
-		for (j = skip_to; j < TYPE_NFIELDS (variant_type); j++)
-		  {
-		    if (first)
-		      first = 0;
-		    else
-		      fputs_filtered (", ", stream);
-
-		    if (!is_tuple)
-		      fprintf_filtered (stream, "%s: ",
-					TYPE_FIELD_NAME (variant_type, j));
-
-		    rust_print_type (TYPE_FIELD_TYPE (variant_type, j), NULL,
-				     stream, show - 1, level + 2,
-				     flags);
-		  }
-		fputs_filtered (is_tuple ? ")" : "}", stream);
-	      }
-
-	    fputs_filtered (",\n", stream);
-	  }
-
-	fputs_filtered ("}", stream);
-      }
-      break;
-
     default:
     c_printer:
       c_print_type (type, varstring, stream, show, level, flags);
     }
 }
 
+static void
+rust_print_type (struct type *type, const char *varstring,
+		 struct ui_file *stream, int show, int level,
+		 const struct type_print_options *flags)
+{
+  rust_internal_print_type (type, varstring, stream, show, level,
+			    flags, false);
+}
+
 
 
 /* Compute the alignment of the type T.  */
@@ -1800,7 +1584,6 @@  rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
         struct value *lhs;
         int pc, field_number, nfields;
         struct type *type, *variant_type;
-        struct disr_info disr;
 
         pc = (*pos)++;
         field_number = longest_to_int (exp->elts[pc + 1].longconst);
@@ -1808,58 +1591,64 @@  rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
         lhs = evaluate_subexp (NULL_TYPE, exp, pos, noside);
 
         type = value_type (lhs);
-        /* 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))
+
+	/* Treat a univariant union as if it were an enum.  */
+	if (TYPE_CODE (type) == TYPE_CODE_UNION && TYPE_NFIELDS (type) == 1)
 	  {
-	    disr = rust_get_disr_info (type, value_contents (lhs),
-				       value_embedded_offset (lhs),
-				       value_address (lhs), lhs);
+	    lhs = value_primitive_field (lhs, 0, 0, type);
+	    type = value_type (lhs);
+	  }
 
-	    if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN)
-	      {
-		variant_type = NULL;
-		nfields = 0;
-	      }
-	    else
+	if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+	  {
+	    struct type *outer_type = NULL;
+
+	    if (rust_enum_p (type))
 	      {
-		variant_type = TYPE_FIELD_TYPE (type, disr.field_no);
-		nfields = TYPE_NFIELDS (variant_type);
+		const gdb_byte *valaddr = value_contents (lhs);
+		struct field *variant_field = rust_enum_variant (type, valaddr);
+
+		struct value *union_value = value_primitive_field (lhs, 0, 0,
+								   type);
+
+		int fieldno = (variant_field
+			       - &TYPE_FIELD (value_type (union_value), 0));
+		lhs = value_primitive_field (union_value, 0, fieldno,
+					     value_type (union_value));
+		outer_type = type;
+		type = value_type (lhs);
 	      }
 
-	    if (!disr.is_encoded)
-	      ++field_number;
-
-	    if (field_number >= nfields || field_number < 0)
-	      error(_("Cannot access field %d of variant %s, \
-there are only %d fields"),
-		    disr.is_encoded ? field_number : field_number - 1,
-		    disr.name.c_str (),
-		    disr.is_encoded ? nfields : nfields - 1);
-
-	    if (!(disr.is_encoded
-		  ? rust_tuple_struct_type_p (variant_type)
-		  : rust_tuple_variant_type_p (variant_type)))
-	      error(_("Variant %s is not a tuple variant"), disr.name.c_str ());
-
-	    result = value_primitive_field (lhs, 0, field_number,
-					    variant_type);
-	  }
-	else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
-	  {
 	    /* Tuples and tuple structs */
-	    nfields = TYPE_NFIELDS(type);
+	    nfields = TYPE_NFIELDS (type);
 
 	    if (field_number >= nfields || field_number < 0)
-	      error(_("Cannot access field %d of %s, there are only %d fields"),
-		    field_number, TYPE_TAG_NAME (type), nfields);
+	      {
+		if (outer_type != NULL)
+		  error(_("Cannot access field %d of variant %s::%s, "
+			  "there are only %d fields"),
+			field_number, TYPE_TAG_NAME (outer_type),
+			rust_last_path_segment (TYPE_TAG_NAME (type)),
+			nfields);
+		else
+		  error(_("Cannot access field %d of %s, "
+			  "there are only %d fields"),
+			field_number, TYPE_TAG_NAME (type), nfields);
+	      }
 
 	    /* Tuples are tuple structs too.  */
 	    if (!rust_tuple_struct_type_p (type))
-	      error(_("Attempting to access anonymous field %d of %s, which is \
-not a tuple, tuple struct, or tuple-like variant"),
-		    field_number, TYPE_TAG_NAME (type));
+	      {
+		if (outer_type != NULL)
+		  error(_("Variant %s::%s is not a tuple variant"),
+			TYPE_TAG_NAME (outer_type),
+			rust_last_path_segment (TYPE_TAG_NAME (type)));
+		else
+		  error(_("Attempting to access anonymous field %d "
+			  "of %s, which is not a tuple, tuple struct, or "
+			  "tuple-like variant"),
+		      field_number, TYPE_TAG_NAME (type));
+	      }
 
 	    result = value_primitive_field (lhs, 0, field_number, type);
 	  }
@@ -1882,53 +1671,44 @@  tuple structs, and tuple-like enum variants"));
 
 	const char *field_name = &exp->elts[pc + 2].string;
         type = value_type (lhs);
-        if (TYPE_CODE (type) == TYPE_CODE_UNION
-            && !rust_union_is_untagged (type))
+        if (TYPE_CODE (type) == TYPE_CODE_STRUCT && rust_enum_p (type))
 	  {
-	    int i, start;
-	    struct disr_info disr;
-	    struct type *variant_type;
-
-	    disr = rust_get_disr_info (type, value_contents (lhs),
-				       value_embedded_offset (lhs),
-				       value_address (lhs), lhs);
-
-	    if (disr.is_encoded && disr.field_no == RUST_ENCODED_ENUM_HIDDEN)
-	      error(_("Could not find field %s of struct variant %s"),
-		    field_name, disr.name.c_str ());
-
-	    variant_type = TYPE_FIELD_TYPE (type, disr.field_no);
-
-	    if (variant_type == NULL
-	        || (disr.is_encoded
-	            ? rust_tuple_struct_type_p (variant_type)
-	            : rust_tuple_variant_type_p (variant_type)))
-	      error(_("Attempting to access named field %s of tuple variant %s, \
-which has only anonymous fields"),
-		    field_name, disr.name.c_str ());
-
-	    start = disr.is_encoded ? 0 : 1;
-	    for (i = start; i < TYPE_NFIELDS (variant_type); i++)
+	    const gdb_byte *valaddr = value_contents (lhs);
+	    struct field *variant_field = rust_enum_variant (type, valaddr);
+
+	    struct value *union_value = value_primitive_field (lhs, 0, 0,
+							       type);
+
+	    int fieldno = (variant_field
+			   - &TYPE_FIELD (value_type (union_value), 0));
+	    lhs = value_primitive_field (union_value, 0, fieldno,
+					 value_type (union_value));
+
+	    struct type *outer_type = type;
+	    type = value_type (lhs);
+	    if (rust_tuple_type_p (type) || rust_tuple_struct_type_p (type))
+		error (_("Attempting to access named field foo of tuple "
+			 "variant %s::%s, which has only anonymous fields"),
+		       TYPE_TAG_NAME (outer_type),
+		       rust_last_path_segment (TYPE_NAME (type)));
+
+	    TRY
 	      {
-		if (strcmp (TYPE_FIELD_NAME (variant_type, i),
-			    field_name) == 0) {
-		  result = value_primitive_field (lhs, 0, i, variant_type);
-		  break;
-		}
+		result = value_struct_elt (&lhs, NULL, field_name,
+					   NULL, "structure");
 	      }
-
-	    if (i == TYPE_NFIELDS (variant_type))
-	      /* We didn't find it.  */
-	      error(_("Could not find field %s of struct variant %s"),
-		    field_name, disr.name.c_str ());
+	    CATCH (except, RETURN_MASK_ERROR)
+	      {
+		error (_("Could not find field %s of struct variant %s::%s"),
+		       field_name, TYPE_TAG_NAME (outer_type),
+		       rust_last_path_segment (TYPE_NAME (type)));
+	      }
+	    END_CATCH
 	  }
 	else
-	  {
-	    result = value_struct_elt (&lhs, NULL, field_name, NULL,
-				       "structure");
-	    if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	      result = value_zero (value_type (result), VALUE_LVAL (result));
-	  }
+	  result = value_struct_elt (&lhs, NULL, field_name, NULL, "structure");
+	if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	  result = value_zero (value_type (result), VALUE_LVAL (result));
       }
       break;
 
diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
index 554a440d58..ce30e33276 100644
--- a/gdb/rust-lang.h
+++ b/gdb/rust-lang.h
@@ -39,6 +39,11 @@  extern bool rust_tuple_struct_type_p (struct type *type);
    stringif no crate name can be found.  */
 extern std::string rust_crate_for_block (const struct block *block);
 
+/* Returns the last segment of a Rust path like foo::bar::baz.  Will
+   not handle cases where the last segment contains generics.  */
+
+extern const char *rust_last_path_segment (const char *path);
+
 /* Create a new slice type.  NAME is the name of the type.  ELT_TYPE
    is the type of the elements of the slice.  USIZE_TYPE is the Rust
    "usize" type to use.  The new type is allocated whereever ELT_TYPE
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7ee3e0b835..9510dcff39 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@ 
+2018-02-22  Tom Tromey  <tom@tromey.com>
+
+	* gdb.rust/simple.exp: Accept more possible results in enum test.
+
 2018-02-21  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdb.arch/amd64-i386-address.exp: Fix a typo.
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 04750d9eb1..230e6a7a88 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -220,8 +220,8 @@  gdb_test "print .." " = .*::ops::RangeFull"
 gdb_test "print str_some" \
     " = core::option::Option<\[a-z\]+::string::String>::Some\\(\[a-z\]+::string::String .*"
 gdb_test "print str_none" " = core::option::Option<\[a-z\]+::string::String>::None"
-gdb_test "print int_some" " = core::option::Option::Some\\(1\\)"
-gdb_test "print int_none" " = core::option::Option::None"
+gdb_test "print int_some" " = core::option::Option<u8>::Some\\(1\\)"
+gdb_test "print int_none" " = core::option::Option<u8>::None"
 gdb_test "print box_some" " = core::option::Option<\[a-z:\]*Box<u8>>::Some\\(.*\\)"
 gdb_test "print box_none" " = core::option::Option<\[a-z:\]*Box<u8>>::None"
 gdb_test "print custom_some" \