Re: [PATCH] Fix handling of discriminantless univariant enums in Rust

[Manish Goregaokar <manish@mozilla.com>]

Updated. Not sure what the style issues are, could you point them out?
Did some editing to remove the need for wrapping the function call.
gmail auto wraps to 80 characters in plain text mode, so I uploaded
the same patch at https://manishearth.pastebin.mozilla.org/8923535

From 940d50a6c4be92ad871f2c8e1f7589d5fa703f24 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manish@mozilla.com>
Date: Thu, 27 Oct 2016 16:46:34 -0700
Subject: [PATCH 1/3] Fix handling of discriminantless univariant enums in
 Rust; fix bug with encoded enums

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

gdb/ChangeLog:
    * rust-lang.c (rust_get_disr_info): Treat univariant enums
    without discriminants as encoded enums with a real field
    * rust-lang.c (rust_evaluate_subexp): Handle field access
    on encoded struct-like enums

gdb/testsuite/ChangeLog:
    * simple.rs: Add test for univariant enums without discriminants
    and for encoded struct-like enums
    * simple.exp: Add test expectations
---
 gdb/rust-lang.c                   | 15 ++++++++++++++-
 gdb/testsuite/gdb.rust/simple.exp | 12 ++++++++++++
 gdb/testsuite/gdb.rust/simple.rs  | 30 ++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 82cd3f9..0ced4bb 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -194,6 +194,17 @@ rust_get_disr_info (struct type *type, const
gdb_byte *valaddr,
      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 = concat (TYPE_NAME (type), "::", last, (char *) NULL);
+    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"));
@@ -1725,7 +1736,9 @@ tuple structs, and tuple-like enum variants"));
         variant_type = TYPE_FIELD_TYPE (type, disr.field_no);

         if (variant_type == NULL
-        || rust_tuple_variant_type_p (variant_type))
+            || (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);
diff --git a/gdb/testsuite/gdb.rust/simple.exp
b/gdb/testsuite/gdb.rust/simple.exp
index 5e00b03..8e84daa 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -103,6 +103,11 @@ gdb_test_sequence "ptype z" "" {
 }
 gdb_test "print z.1" " = 8"

+gdb_test "print univariant" " = simple::Univariant::Foo{a: 1}"
+gdb_test "print univariant.a" " = 1"
+gdb_test "print univariant_anon" " = simple::UnivariantAnon::Foo\\(1\\)"
+gdb_test "print univariant_anon.0" " = 1"
+
 gdb_test_sequence "ptype simple::ByeBob" "" {
     " = struct simple::ByeBob \\("
     "  i32,"
@@ -220,3 +225,10 @@ gdb_test "print (1,)" "Tuple expressions not supported yet"
 gdb_test "print (1)" " = 1"

 gdb_test "print 23..97.0" "Range expression with different types"
+
+gdb_test "print (*parametrized.next.val)" \
+    " = simple::ParametrizedStruct<i32> {next:
simple::ParametrizedEnum<Box<simple::ParametrizedStruct<i32>>>::Empty,
value: 1}"
+gdb_test "print parametrized.next.val" \
+    " = \\(simple::ParametrizedStruct<i32> \\*\\) $hex"
+gdb_test "print parametrized" \
+    " = simple::ParametrizedStruct<i32> \\{next:
simple::ParametrizedEnum<Box<simple::ParametrizedStruct<i32>>>::Val\\{val:
$hex\\}, value: 0\\}"
diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
index eeff3d7..670f54e 100644
--- a/gdb/testsuite/gdb.rust/simple.rs
+++ b/gdb/testsuite/gdb.rust/simple.rs
@@ -63,6 +63,23 @@ enum SpaceSaver {
     Nothing,
 }

+enum Univariant {
+    Foo {a: u8}
+}
+enum UnivariantAnon {
+    Foo(u8)
+}
+
+enum ParametrizedEnum<T> {
+    Val { val: T },
+    Empty,
+}
+
+struct ParametrizedStruct<T> {
+    next: ParametrizedEnum<Box<ParametrizedStruct<T>>>,
+    value: T
+}
+
 fn main () {
     let a = ();
     let b : [i32; 0] = [];
@@ -93,6 +110,9 @@ fn main () {
     let y = HiBob {field1: 7, field2: 8};
     let z = ByeBob(7, 8);

+    let univariant = Univariant::Foo {a : 1};
+    let univariant_anon = UnivariantAnon::Foo(1);
+
     let slice = &w[2..3];
     let fromslice = slice[0];
     let slice2 = &slice[0..1];
@@ -117,6 +137,16 @@ fn main () {
     let custom_some = NonZeroOptimized::Value("hi".into());
     let custom_none = NonZeroOptimized::Empty;

+    let parametrized = ParametrizedStruct {
+        next: ParametrizedEnum::Val {
+            val: Box::new(ParametrizedStruct {
+                next: ParametrizedEnum::Empty,
+                value: 1,
+            })
+        },
+        value: 0,
+    };
+
     println!("{}, {}", x.0, x.1);        // set breakpoint here
     println!("{}", diff2(92, 45));
     empty();
-- 
2.10.1
-Manish


On Sun, Oct 30, 2016 at 8:03 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:
>
> Manish> +  else if (TYPE_NFIELDS (type) == 1) {
> Manish> +    /* Sometimes univariant enums are encoded without a
> Manish> +    discriminant. In that case, treating it as an encoded enum
> Manish> +    with the first field being the actual type works.  */
> Manish> +    const char* field_name = TYPE_NAME (TYPE_FIELD_TYPE (type, 0));
> Manish> +    ret.name = concat (TYPE_NAME (type), "::",
> Manish> +                       rust_last_path_segment (field_name),
> Manish> +                       (char *) NULL);
> Manish> +    ret.field_no = RUST_ENCODED_ENUM_REAL;
> Manish> +    ret.is_encoded = 1;
> Manish> +    return ret;
> Manish> +  }
> Manish>    if (strcmp (TYPE_FIELD_NAME (disr_type, 0), "RUST$ENUM$DISR") != 0)
>
> This still should follow GNU style.
> And I think a blank line after the "}".
>
> Manish> +gdb_test "print parametrized.next.val" " =
> Manish> \\(simple::ParametrizedStruct<i32> \\*\\) $hex"
>
> Patch mangled by mail program.
> No biggie but it's a pain if one wants to try it out.
>
> Manish> $hex\\}, value: 0\\}"
> Manish> \ No newline at end of file
>
> Please add a newline.
>
> Tom