Fix handling of discriminantless univariant enums in Rust

Message ID CAFOnWknxbPRntPJNJmFyz1f5ss8GQ900zjE_ssVptUEWLrF0Cg@mail.gmail.com
State New, archived
Headers

Commit Message

Manish Goregaokar Oct. 30, 2016, 2:58 a.m. UTC
  `print univariant.a` works for me.

New patch (fixed style, added univariant.a test)


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

gdb/testsuite/ChangeLog:
    * simple.rs: Add test for univariant enums without discriminants
    * simple.exp: Add test expectations
---
 gdb/rust-lang.c                   | 13 ++++++++++++-
 gdb/testsuite/gdb.rust/simple.exp |  3 +++
 gdb/testsuite/gdb.rust/simple.rs  |  6 ++++++
 3 files changed, 21 insertions(+), 1 deletion(-)
  

Comments

Manish Goregaokar Oct. 30, 2016, 3:05 a.m. UTC | #1
The same test works fine with a univariant tuple enum and
STRUCTOP_ANONYMOUS fwiw. (added a univariant_anon test to my local
patch)
-Manish


On Sat, Oct 29, 2016 at 7:58 PM, Manish Goregaokar <manish@mozilla.com> wrote:
> `print univariant.a` works for me.
>
> New patch (fixed style, added univariant.a test)
>
>
> 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
>
> gdb/testsuite/ChangeLog:
>     * simple.rs: Add test for univariant enums without discriminants
>     * simple.exp: Add test expectations
> ---
>  gdb/rust-lang.c                   | 13 ++++++++++++-
>  gdb/testsuite/gdb.rust/simple.exp |  3 +++
>  gdb/testsuite/gdb.rust/simple.rs  |  6 ++++++
>  3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 82cd3f9..7b1ff8a 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -194,7 +194,18 @@ 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));
> +    ret.name = concat (TYPE_NAME (type), "::",
> +                       rust_last_path_segment (field_name),
> +                       (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"));
>
> diff --git a/gdb/testsuite/gdb.rust/simple.exp
> b/gdb/testsuite/gdb.rust/simple.exp
> index 5e00b03..c9e8e57 100644
> --- a/gdb/testsuite/gdb.rust/simple.exp
> +++ b/gdb/testsuite/gdb.rust/simple.exp
> @@ -103,6 +103,9 @@ 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_sequence "ptype simple::ByeBob" "" {
>      " = struct simple::ByeBob \\("
>      "  i32,"
> diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
> index eeff3d7..b2e29ae 100644
> --- a/gdb/testsuite/gdb.rust/simple.rs
> +++ b/gdb/testsuite/gdb.rust/simple.rs
> @@ -63,6 +63,10 @@ enum SpaceSaver {
>      Nothing,
>  }
>
> +enum Univariant {
> +    Foo {a: u8}
> +}
> +
>  fn main () {
>      let a = ();
>      let b : [i32; 0] = [];
> @@ -93,6 +97,8 @@ fn main () {
>      let y = HiBob {field1: 7, field2: 8};
>      let z = ByeBob(7, 8);
>
> +    let univariant = Univariant::Foo {a : 1};
> +
>      let slice = &w[2..3];
>      let fromslice = slice[0];
>      let slice2 = &slice[0..1];
> --
> 2.10.1
>
> -Manish
>
>
> On Sat, Oct 29, 2016 at 7:52 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>
>> Tom> Also, I suspect this will wind up doing the wrong thing in the
>> Tom> STRUCTOP_ANONYMOUS case in rust_evaluate_subexp.  In particular I wonder
>> Tom> if an additional "print univariant.0.a" test will work correctly?
>>
>> Oh duh, I see that this isn't a correct counter-example.
>> What about just "print univariant.a"?
>> It seems to me that this will hit this:
>>
>>             start = disr.is_encoded ? 0 : 1;
>>
>> ... choosing 1 here, but:
>>
>>             for (i = start; i < TYPE_NFIELDS (variant_type); i++)
>>
>> ... failing this because TYPE_NFIELDS == 1; and then:
>>
>>             if (i == TYPE_NFIELDS (variant_type))
>>               /* We didn't find it.  */
>>               error(_("Could not find field %s of struct variant %s"),
>>
>> Tom
  
Tom Tromey Oct. 30, 2016, 3:34 a.m. UTC | #2
>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:

Manish> `print univariant.a` works for me.

I tried this patch and it failed the way I thought it would:

print univariant.a
Attempting to access named field a of tuple variant simple::Univariant::Foo, which has only anonymous fields
(gdb) FAIL: gdb.rust/simple.exp: print univariant.a


Maybe you're trying it with the follow-up patch installed?
In that case I think the patches should probably be combined.

Tom
  
Manish Goregaokar Oct. 30, 2016, 3:35 a.m. UTC | #3
Yes, I'm trying it with the followup patch. I kept them separate since
that's an independent bug that just affects univariant ones too. Will
roll them together.
-Manish


On Sat, Oct 29, 2016 at 8:34 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:
>
> Manish> `print univariant.a` works for me.
>
> I tried this patch and it failed the way I thought it would:
>
> print univariant.a
> Attempting to access named field a of tuple variant simple::Univariant::Foo, which has only anonymous fields
> (gdb) FAIL: gdb.rust/simple.exp: print univariant.a
>
>
> Maybe you're trying it with the follow-up patch installed?
> In that case I think the patches should probably be combined.
>
> Tom
  

Patch

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 82cd3f9..7b1ff8a 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -194,7 +194,18 @@  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));
+    ret.name = concat (TYPE_NAME (type), "::",
+                       rust_last_path_segment (field_name),
+                       (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"));

diff --git a/gdb/testsuite/gdb.rust/simple.exp
b/gdb/testsuite/gdb.rust/simple.exp
index 5e00b03..c9e8e57 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -103,6 +103,9 @@  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_sequence "ptype simple::ByeBob" "" {
     " = struct simple::ByeBob \\("
     "  i32,"
diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
index eeff3d7..b2e29ae 100644
--- a/gdb/testsuite/gdb.rust/simple.rs
+++ b/gdb/testsuite/gdb.rust/simple.rs
@@ -63,6 +63,10 @@  enum SpaceSaver {
     Nothing,
 }

+enum Univariant {
+    Foo {a: u8}
+}
+
 fn main () {
     let a = ();
     let b : [i32; 0] = [];
@@ -93,6 +97,8 @@  fn main () {
     let y = HiBob {field1: 7, field2: 8};
     let z = ByeBob(7, 8);

+    let univariant = Univariant::Foo {a : 1};
+
     let slice = &w[2..3];
     let fromslice = slice[0];
     let slice2 = &slice[0..1];