[PATCH] Fix handling of discriminantless univariant enums in Rust

[PATCH] Fix handling of discriminantless univariant enums in Rust

[Manish Goregaokar]

Fixes an issue caught by TimNN in https://github.com/rust-lang/rust/pull/37410


From: Manish Goregaokar <manish@mozilla.com>
Date: Thu, 27 Oct 2016 16:46:34 -0700
Subject: Fix handling of discriminantless univariant enums in Rust

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 |  2 ++
 gdb/testsuite/gdb.rust/simple.rs  |  6 ++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 82cd3f9..9d13353 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..dbfc88a 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -103,6 +103,8 @@ gdb_test_sequence "ptype z" "" {
 }
 gdb_test "print z.1" " = 8"

+gdb_test "print univariant" " = simple::Univariant::Foo{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

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

[Manish Goregaokar]

Fixed another enum issue, reported in the same PR:

(I'm not sure if this is the correct fix)

From: Manish Goregaokar <manish@mozilla.com>
Date: Thu, 27 Oct 2016 16:46:34 -0700
Subject: Handle field access on encoded struct-like enums

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

gdb/ChangeLog:
    * rust-lang.c (rust_evaluate_subexp): Handle field access
    on encoded struct-like enums

gdb/testsuite/ChangeLog:
    * simple.rs: Add test for encoded struct-like enums
    * simple.exp: Add test expectations
---
 gdb/rust-lang.c                   |  4 +++-
 gdb/testsuite/gdb.rust/simple.exp |  6 ++++++
 gdb/testsuite/gdb.rust/simple.rs  | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 9d13353..9569584 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1736,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 dbfc88a..075bff7 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -222,3 +222,9 @@ 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\\}"
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
index b2e29ae..e756296 100644
--- a/gdb/testsuite/gdb.rust/simple.rs
+++ b/gdb/testsuite/gdb.rust/simple.rs
@@ -67,6 +67,16 @@ enum Univariant {
     Foo {a: 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] = [];
@@ -123,6 +133,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 Thu, Oct 27, 2016 at 4:51 PM, Manish Goregaokar <manish@mozilla.com> wrote:
> Fixes an issue caught by TimNN in https://github.com/rust-lang/rust/pull/37410
>
>
> From: Manish Goregaokar <manish@mozilla.com>
> Date: Thu, 27 Oct 2016 16:46:34 -0700
> Subject: Fix handling of discriminantless univariant enums in Rust
>
> 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 |  2 ++
>  gdb/testsuite/gdb.rust/simple.rs  |  6 ++++++
>  3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 82cd3f9..9d13353 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..dbfc88a 100644
> --- a/gdb/testsuite/gdb.rust/simple.exp
> +++ b/gdb/testsuite/gdb.rust/simple.exp
> @@ -103,6 +103,8 @@ gdb_test_sequence "ptype z" "" {
>  }
>  gdb_test "print z.1" " = 8"
>
> +gdb_test "print univariant" " = simple::Univariant::Foo{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

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

[Tom Tromey]

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

This needs some small changes to conform to the GNU coding style.

Also, I suspect this will wind up doing the wrong thing in the
STRUCTOP_ANONYMOUS case in rust_evaluate_subexp.  In particular I wonder
if an additional "print univariant.0.a" test will work correctly?

Tom

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

[Tom Tromey]

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

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

[Manish Goregaokar]

`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

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

[Manish Goregaokar]

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

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

[Tom Tromey]

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

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

[Manish Goregaokar]

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

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

[Manish Goregaokar]

Rolled up patch:

From 789f95700e43b29d0c374985f8504eb414c85eec Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manish@mozilla.com>
Date: Thu, 27 Oct 2016 16:46:34 -0700
Subject: [PATCH] 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                   | 17 +++++++++++++++--
 gdb/testsuite/gdb.rust/simple.exp | 11 +++++++++++
 gdb/testsuite/gdb.rust/simple.rs  | 30 ++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 82cd3f9..63c7992 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"));

@@ -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..14eae2e 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,9 @@ 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\\}"
\ No newline at end of file
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 Sat, Oct 29, 2016 at 8:35 PM, Manish Goregaokar <manish@mozilla.com> wrote:
> 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

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

[Tom Tromey]

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

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

[Manish Goregaokar]

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

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

[Tom Tromey]

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

Manish> Updated. Not sure what the style issues are, could you point them out?

Sure.  There's also the GNU coding standards document, plus some docs on
the gdb wiki somewhere.

Manish>      }
Manish> +  else if (TYPE_NFIELDS (type) == 1) {

Brace placement.  Should line up with that "}" just above.

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.  */

Subsequent text in a multi-line comment should line up under the
"Sometimes".

Period followed by two spaces ends a sentence, so one more space before
"In".

Manish> +    const char* field_name = TYPE_NAME (TYPE_FIELD_TYPE (type, 0));
Manish> +    const char* last = rust_last_path_segment (field_name);

"char *"

Manish> +    ret.name = concat (TYPE_NAME (type), "::", last, (char *) NULL);
Manish> +    ret.field_no = RUST_ENCODED_ENUM_REAL;
Manish> +    ret.is_encoded = 1;
Manish> +    return ret;
Manish> +  }

This brace and the whole block will be indented a bit more due to the
brace placement issue.

thanks,
Tom

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

[Manish Goregaokar]

Yeah, have gone through the document, but I still miss things :)

From 7a2296aabb423ade3bc6bb675cf97123ba939eb4 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                   | 16 +++++++++++++++-
 gdb/testsuite/gdb.rust/simple.exp | 12 ++++++++++++
 gdb/testsuite/gdb.rust/simple.rs  | 30 ++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 82cd3f9..774eed9 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -194,6 +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));
+      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 +1737,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:29 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Manish" == Manish Goregaokar <manish@mozilla.com> writes:
>
> Manish> Updated. Not sure what the style issues are, could you point them out?
>
> Sure.  There's also the GNU coding standards document, plus some docs on
> the gdb wiki somewhere.
>
> Manish>      }
> Manish> +  else if (TYPE_NFIELDS (type) == 1) {
>
> Brace placement.  Should line up with that "}" just above.
>
> 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.  */
>
> Subsequent text in a multi-line comment should line up under the
> "Sometimes".
>
> Period followed by two spaces ends a sentence, so one more space before
> "In".
>
> Manish> +    const char* field_name = TYPE_NAME (TYPE_FIELD_TYPE (type, 0));
> Manish> +    const char* last = rust_last_path_segment (field_name);
>
> "char *"
>
> Manish> +    ret.name = concat (TYPE_NAME (type), "::", last, (char *) NULL);
> Manish> +    ret.field_no = RUST_ENCODED_ENUM_REAL;
> Manish> +    ret.is_encoded = 1;
> Manish> +    return ret;
> Manish> +  }
>
> This brace and the whole block will be indented a bit more due to the
> brace placement issue.
>
> thanks,
> Tom

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

[Tom Tromey]

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

Manish> Yeah, have gone through the document, but I still miss things :)

Yeah, I feel your pain.

This version is ok.  Thanks!

Tom