[FYI] Allow empty struct expressions in Rust

Message ID 1468271438-5389-1-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey July 11, 2016, 9:10 p.m. UTC
  I learned recently that empty struct expressions, like "X{}", have been
promoted from experimental to stable in Rust.  This patch changes the
Rust expression parser to allow this case.

New test case included.
Built and regtested on x86-64 Fedora 23, using Rust 1.11 beta.

I plan to check this in after a reasonable interval.

2016-07-11  Tom Tromey  <tom@tromey.com>

	* rust-lang.c (rust_tuple_struct_type_p): Return false for empty
	structs.
	* rust-exp.y (struct_expr_list): Allow empty elements.

2016-07-11  Tom Tromey  <tom@tromey.com>

	* gdb.rust/simple.rs (main): Use empty struct expression.
	* gdb.rust/simple.exp: Add tests for empty struct expression.
---
 gdb/ChangeLog                     |  6 ++++++
 gdb/rust-exp.y                    | 10 +++++++---
 gdb/rust-lang.c                   |  5 ++++-
 gdb/testsuite/ChangeLog           |  5 +++++
 gdb/testsuite/gdb.rust/simple.exp |  3 +++
 gdb/testsuite/gdb.rust/simple.rs  |  1 +
 6 files changed, 26 insertions(+), 4 deletions(-)
  

Comments

Manish Goregaokar July 12, 2016, 12:53 a.m. UTC | #1
This lgtm.

I don't think we should bother trying to differentiate between Unit,
Unit{}, and Unit().
-Manish


On Tue, Jul 12, 2016 at 2:40 AM, Tom Tromey <tom@tromey.com> wrote:
> I learned recently that empty struct expressions, like "X{}", have been
> promoted from experimental to stable in Rust.  This patch changes the
> Rust expression parser to allow this case.
>
> New test case included.
> Built and regtested on x86-64 Fedora 23, using Rust 1.11 beta.
>
> I plan to check this in after a reasonable interval.
>
> 2016-07-11  Tom Tromey  <tom@tromey.com>
>
>         * rust-lang.c (rust_tuple_struct_type_p): Return false for empty
>         structs.
>         * rust-exp.y (struct_expr_list): Allow empty elements.
>
> 2016-07-11  Tom Tromey  <tom@tromey.com>
>
>         * gdb.rust/simple.rs (main): Use empty struct expression.
>         * gdb.rust/simple.exp: Add tests for empty struct expression.
> ---
>  gdb/ChangeLog                     |  6 ++++++
>  gdb/rust-exp.y                    | 10 +++++++---
>  gdb/rust-lang.c                   |  5 ++++-
>  gdb/testsuite/ChangeLog           |  5 +++++
>  gdb/testsuite/gdb.rust/simple.exp |  3 +++
>  gdb/testsuite/gdb.rust/simple.rs  |  1 +
>  6 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 92c1337..dd678f5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-07-11  Tom Tromey  <tom@tromey.com>
> +
> +       * rust-lang.c (rust_tuple_struct_type_p): Return false for empty
> +       structs.
> +       * rust-exp.y (struct_expr_list): Allow empty elements.
> +
>  2016-07-07  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>
>         * cp-namespace.c (cp_lookup_bare_symbol): Initialize
> diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
> index aeb6058..443c356 100644
> --- a/gdb/rust-exp.y
> +++ b/gdb/rust-exp.y
> @@ -428,10 +428,14 @@ struct_expr_tail:
>                 }
>  ;
>
> -/* S{} is documented as valid but seems to be an unstable feature, so
> -   it is left out here.  */
>  struct_expr_list:
> -       struct_expr_tail
> +       /* %empty */
> +               {
> +                 VEC (set_field) **result
> +                   = OBSTACK_ZALLOC (&work_obstack, VEC (set_field) *);
> +                 $$ = result;
> +               }
> +|      struct_expr_tail
>                 {
>                   VEC (set_field) **result
>                     = OBSTACK_ZALLOC (&work_obstack, VEC (set_field) *);
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 17b20c6..6e317fb 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -294,7 +294,10 @@ rust_underscore_fields (struct type *type, int offset)
>  int
>  rust_tuple_struct_type_p (struct type *type)
>  {
> -  return rust_underscore_fields (type, 0);
> +  /* 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.  */
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index b6f21d7..2eba208 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-07-11  Tom Tromey  <tom@tromey.com>
> +
> +       * gdb.rust/simple.rs (main): Use empty struct expression.
> +       * gdb.rust/simple.exp: Add tests for empty struct expression.
> +
>  2016-07-07  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>
>         * gdb.fortran/derived-types.exp (result_line, result_line_2):
> diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
> index 32b3785..5e00b03 100644
> --- a/gdb/testsuite/gdb.rust/simple.exp
> +++ b/gdb/testsuite/gdb.rust/simple.exp
> @@ -55,7 +55,10 @@ gdb_test "print *(&c as *mut i32)" " = 0"
>
>  gdb_test "print j" " = simple::Unit"
>  gdb_test "ptype j" " = struct simple::Unit"
> +gdb_test "print j2" " = simple::Unit"
> +gdb_test "ptype j2" " = struct simple::Unit"
>  gdb_test "print simple::Unit" " = simple::Unit"
> +gdb_test "print simple::Unit{}" " = simple::Unit"
>
>  gdb_test "print g" " = \\(u8 \\(\\*\\)\\\[6\\\]\\) $hex b\"hi bob\""
>  gdb_test "ptype g" " = u8 \\(\\*\\)\\\[6\\\]"
> diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
> index 4980826..eeff3d7 100644
> --- a/gdb/testsuite/gdb.rust/simple.rs
> +++ b/gdb/testsuite/gdb.rust/simple.rs
> @@ -81,6 +81,7 @@ fn main () {
>      let i = ["whatever"; 8];
>
>      let j = Unit;
> +    let j2 = Unit{};
>
>      let k = SpaceSaver::Nothing;
>      let l = SpaceSaver::Thebox(9, Box::new(1729));
> --
> 2.5.5
>
  
Tom Tromey July 21, 2016, 9:14 p.m. UTC | #2
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I learned recently that empty struct expressions, like "X{}", have been
Tom> promoted from experimental to stable in Rust.  This patch changes the
Tom> Rust expression parser to allow this case.

Tom> New test case included.
Tom> Built and regtested on x86-64 Fedora 23, using Rust 1.11 beta.

Tom> I plan to check this in after a reasonable interval.

Tom> 2016-07-11  Tom Tromey  <tom@tromey.com>

Tom> 	* rust-lang.c (rust_tuple_struct_type_p): Return false for empty
Tom> 	structs.
Tom> 	* rust-exp.y (struct_expr_list): Allow empty elements.

I'm checking this in now.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 92c1337..dd678f5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-07-11  Tom Tromey  <tom@tromey.com>
+
+	* rust-lang.c (rust_tuple_struct_type_p): Return false for empty
+	structs.
+	* rust-exp.y (struct_expr_list): Allow empty elements.
+
 2016-07-07  Walfred Tedeschi  <walfred.tedeschi@intel.com>
 
 	* cp-namespace.c (cp_lookup_bare_symbol): Initialize 
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index aeb6058..443c356 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -428,10 +428,14 @@  struct_expr_tail:
 		}
 ;
 
-/* S{} is documented as valid but seems to be an unstable feature, so
-   it is left out here.  */
 struct_expr_list:
-	struct_expr_tail
+	/* %empty */
+		{
+		  VEC (set_field) **result
+		    = OBSTACK_ZALLOC (&work_obstack, VEC (set_field) *);
+		  $$ = result;
+		}
+|	struct_expr_tail
 		{
 		  VEC (set_field) **result
 		    = OBSTACK_ZALLOC (&work_obstack, VEC (set_field) *);
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 17b20c6..6e317fb 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -294,7 +294,10 @@  rust_underscore_fields (struct type *type, int offset)
 int
 rust_tuple_struct_type_p (struct type *type)
 {
-  return rust_underscore_fields (type, 0);
+  /* 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.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b6f21d7..2eba208 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-07-11  Tom Tromey  <tom@tromey.com>
+
+	* gdb.rust/simple.rs (main): Use empty struct expression.
+	* gdb.rust/simple.exp: Add tests for empty struct expression.
+
 2016-07-07  Walfred Tedeschi  <walfred.tedeschi@intel.com>
 
 	* gdb.fortran/derived-types.exp (result_line, result_line_2):
diff --git a/gdb/testsuite/gdb.rust/simple.exp b/gdb/testsuite/gdb.rust/simple.exp
index 32b3785..5e00b03 100644
--- a/gdb/testsuite/gdb.rust/simple.exp
+++ b/gdb/testsuite/gdb.rust/simple.exp
@@ -55,7 +55,10 @@  gdb_test "print *(&c as *mut i32)" " = 0"
 
 gdb_test "print j" " = simple::Unit"
 gdb_test "ptype j" " = struct simple::Unit"
+gdb_test "print j2" " = simple::Unit"
+gdb_test "ptype j2" " = struct simple::Unit"
 gdb_test "print simple::Unit" " = simple::Unit"
+gdb_test "print simple::Unit{}" " = simple::Unit"
 
 gdb_test "print g" " = \\(u8 \\(\\*\\)\\\[6\\\]\\) $hex b\"hi bob\""
 gdb_test "ptype g" " = u8 \\(\\*\\)\\\[6\\\]"
diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
index 4980826..eeff3d7 100644
--- a/gdb/testsuite/gdb.rust/simple.rs
+++ b/gdb/testsuite/gdb.rust/simple.rs
@@ -81,6 +81,7 @@  fn main () {
     let i = ["whatever"; 8];
 
     let j = Unit;
+    let j2 = Unit{};
 
     let k = SpaceSaver::Nothing;
     let l = SpaceSaver::Thebox(9, Box::new(1729));