Print void types correctly in Rust

  Rust prefers to not specify the return type of a function when it is unit
(`()`). The type is also referred to as "void" in debuginfo but not in actual
usage, so we should never be printing "void" when the language is Rust.

gdb/ChangeLog:
    * rust-lang.c: Print unit types as "()"
    * rust-lang.c: Omit return type for functions returning unit

gdb/testsuite/ChangeLog:
2016-06-27  Manish Goregaokar  <manish@mozilla.com>
    * gdb.rust/simple.rs: Add test for returning unit in a function
    * gdb.rust/simple.exp: Add expectation for functions returning unit
---
 gdb/rust-lang.c                   | 22 ++++++++++++++++++----
 gdb/testsuite/gdb.rust/simple.exp |  1 +
 gdb/testsuite/gdb.rust/simple.rs  |  7 +++++++
 3 files changed, 26 insertions(+), 4 deletions(-)
  

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

Manish> Rust prefers to not specify the return type of a function when it is unit
Manish> (`()`). The type is also referred to as "void" in debuginfo but not in actual
Manish> usage, so we should never be printing "void" when the language is Rust.

Thanks for finding and fixing this.

The patch looks good overall but I have a few questions and some nits.

Manish>     * rust-lang.c: Print unit types as "()"
Manish>     * rust-lang.c: Omit return type for functions returning unit

A ChangeLog entry is supposed to refer to function and variable names,
so more like:

	* rust-lang.c (rust_decorations): Spell "void" as "()".
	(rust_print_type): Likewise.
	...

Manish> +      /* Rust calls the unit type "void" in its debuginfo,
Manish> +         but we don't want to print it as that.  */
Manish> +      if (TYPE_CODE (type) == TYPE_CODE_VOID)
Manish> +        fputs_filtered ("()", stream);
Manish> +      else
Manish> +        fputs_filtered (TYPE_NAME (type), stream);

This is fine but there is another case in rust_val_print:

    case TYPE_CODE_INT:
      /* Recognize the unit type.  */
      if (TYPE_UNSIGNED (type) && TYPE_LENGTH (type) == 0
	  && TYPE_NAME (type) != NULL && strcmp (TYPE_NAME (type), "()") == 0)
	{
	  fputs_filtered ("()", stream);
	  break;
	}
      goto generic_print;


... I wonder if that is something that changed after 1.8, or if it's the
case that the unit type can be represented in multiple ways.  (Or maybe
this only handles the unit type constructed by rust_language_arch_info?)

Anyway I wonder if a case like this is needed in rust_print_type, or if
the one in rust_val_print can be removed or changed.

Manish> +      if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_VOID)
Manish> +      {
Manish> +        fputs_filtered (" -> ", stream);
Manish> +        rust_print_type (TYPE_TARGET_TYPE (type), "", stream, -1, 0, flags);

This should be formatted like:

+      if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_VOID)
+        {
+          fputs_filtered (" -> ", stream);
+          rust_print_type (TYPE_TARGET_TYPE (type), "", stream, -1, 0, flags);

Manish> +// Empty function, should not have "void"
Manish> +// or "()" in its return type
Manish> +fn empty() {

I'm curious what happens if it does say "-> ()"?

Tom
  

Tom> ... I wonder if that is something that changed after 1.8, or if it's the
Tom> case that the unit type can be represented in multiple ways.  (Or maybe
Tom> this only handles the unit type constructed by rust_language_arch_info?)

[...]

Manish> +// Empty function, should not have "void"
Manish> +// or "()" in its return type
Manish> +fn empty() {

Tom> I'm curious what happens if it does say "-> ()"?

I did the experiment myself and I think I see what's going on now.

For an explicit "()", like "let y = ()", we'll end up with a zero-sized
integer type:

 <1><98>: Abbrev Number: 7 (DW_TAG_base_type)
    <99>   DW_AT_name        : (indirect string, offset: 0x88): ()
    <9d>   DW_AT_encoding    : 7	(unsigned)
    <9e>   DW_AT_byte_size   : 0


However, "fn empty()" (or in my experiment also "fn empty()->()"), the
type is left unspecified in the DWARF, which gdb turns into
TYPE_CODE_VOID (see dwarf2read.c:read_unspecified_type).

I think both have to be handled.  However I think it's fine to do some
cleanup in a follow-up patch, which I'm happy to do.

So your patch is OK with the ChangeLog entry and the indentation fixed.

Tom
  

Landed as https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=921d8f549f9e35d3f83c7b1a381146a7dc1246f4,
with a changelog fix at
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=6763d566a8d30d1ad65dfd060a919c621dad86c4.


Thanks,
-Manish


On Mon, Jun 27, 2016 at 9:22 PM, Manish Goregaokar <manish@mozilla.com> wrote:
> Rust prefers to not specify the return type of a function when it is unit
> (`()`). The type is also referred to as "void" in debuginfo but not in actual
> usage, so we should never be printing "void" when the language is Rust.
>
> gdb/ChangeLog:
>     * rust-lang.c: Print unit types as "()"
>     * rust-lang.c: Omit return type for functions returning unit
>
> gdb/testsuite/ChangeLog:
> 2016-06-27  Manish Goregaokar  <manish@mozilla.com>
>     * gdb.rust/simple.rs: Add test for returning unit in a function
>     * gdb.rust/simple.exp: Add expectation for functions returning unit
> ---
>  gdb/rust-lang.c                   | 22 ++++++++++++++++++----
>  gdb/testsuite/gdb.rust/simple.exp |  1 +
>  gdb/testsuite/gdb.rust/simple.rs  |  7 +++++++
>  3 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 0c56a0f..2b115ee 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -446,7 +446,7 @@ static const struct generic_val_print_decorations
> rust_decorations =
>    " * I",
>    "true",
>    "false",
> -  "void",
> +  "()",
>    "[",
>    "]"
>  };
> @@ -729,13 +729,22 @@ rust_print_type (struct type *type, const char *varstring,
>    if (show <= 0
>        && TYPE_NAME (type) != NULL)
>      {
> -      fputs_filtered (TYPE_NAME (type), stream);
> +      /* Rust calls the unit type "void" in its debuginfo,
> +         but we don't want to print it as that.  */
> +      if (TYPE_CODE (type) == TYPE_CODE_VOID)
> +        fputs_filtered ("()", stream);
> +      else
> +        fputs_filtered (TYPE_NAME (type), stream);
>        return;
>      }
>
>    type = check_typedef (type);
>    switch (TYPE_CODE (type))
>      {
> +    case TYPE_CODE_VOID:
> +      fputs_filtered ("()", stream);
> +      break;
> +
>      case TYPE_CODE_FUNC:
>        /* Delegate varargs to the C printer.  */
>        if (TYPE_VARARGS (type))
> @@ -753,8 +762,13 @@ rust_print_type (struct type *type, const char *varstring,
>        rust_print_type (TYPE_FIELD_TYPE (type, i), "", stream, -1, 0,
>                 flags);
>      }
> -      fputs_filtered (") -> ", stream);
> -      rust_print_type (TYPE_TARGET_TYPE (type), "", stream, -1, 0, flags);
> +      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);
> +      }
>        break;
>
>      case TYPE_CODE_ARRAY:
> diff --git a/gdb/testsuite/gdb.rust/simple.exp
> b/gdb/testsuite/gdb.rust/simple.exp
> index 88f1c89..4622f75 100644
> --- a/gdb/testsuite/gdb.rust/simple.exp
> +++ b/gdb/testsuite/gdb.rust/simple.exp
> @@ -149,6 +149,7 @@ gdb_test "print self::diff2(8, 9)" " = -1"
>  gdb_test "print ::diff2(23, -23)" " = 46"
>
>  gdb_test "ptype diff2" "fn \\(i32, i32\\) -> i32"
> +gdb_test "ptype empty" "fn \\(\\)"
>
>  gdb_test "print (diff2 as fn(i32, i32) -> i32)(19, -2)" " = 21"
>
> diff --git a/gdb/testsuite/gdb.rust/simple.rs b/gdb/testsuite/gdb.rust/simple.rs
> index 32da580..3d28e27 100644
> --- a/gdb/testsuite/gdb.rust/simple.rs
> +++ b/gdb/testsuite/gdb.rust/simple.rs
> @@ -48,6 +48,12 @@ fn diff2(x: i32, y: i32) -> i32 {
>      x - y
>  }
>
> +// Empty function, should not have "void"
> +// or "()" in its return type
> +fn empty() {
> +
> +}
> +
>  pub struct Unit;
>
>  // This triggers the non-zero optimization that yields a different
> @@ -111,4 +117,5 @@ fn main () {
>
>      println!("{}, {}", x.0, x.1);        // set breakpoint here
>      println!("{}", diff2(92, 45));
> +    empty();
>  }
> --
> 2.8.3