Re: [PATCH] gdb/rust: Handle printing structures containing strings

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2019-05-02 07:44:04 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> When printing a rust structure that contains a string GDB can
> Andrew> currently fail to read the fields that define the string. This is
> Andrew> because GDB mistakenly treats a value that is the parent structure as
> Andrew> though it is the structure that defines the string, and then fails to
> Andrew> find the fields needed to extract a string.
> 
> Andrew> The solution is to create a new value to represent the string field of
> Andrew> the parent value.
> 
> Thank you for doing this.
> 
> Andrew>    if (rust_slice_type_p (type) && strcmp (TYPE_NAME (type), "&str") == 0)
> Andrew>      {
> Andrew> +      /* If what we are printing here is actually a string within a
> Andrew> +	 structure then VAL will be the original parent value, while TYPE
> Andrew> +	 will be the type of the structure representing the string we want
> Andrew> +	 to print.
> Andrew> +	 However, RUST_VAL_PRINT_STR looks up the fields of the string
> Andrew> +	 inside VAL, assuming that VAL is the string.
> Andrew> +	 So, recreate VAL as a value representing just the string.  */
> Andrew> +      val = value_at_lazy (type, value_address (val) + embedded_offset);
> Andrew>        rust_val_print_str (stream, val, options);
> Andrew>        return;
> 
> It took me a while to understand this, but I get it now.  At first it
> looked like this code was misplaced, but what's going on is that
> rust_val_print_str only takes a value, so in a "val_print" context we
> must reconstruct one.

If you'd like to suggest some new words I'd be happy to include them.
I spent too long trying to write a helpful comment here, and like you
I wasn't really happy with what I ended up with....

> 
> We really ought to get rid of the val_print / value_print distinction
> someday.  It doesn't provide any benefit and it leads to bugs like this.
> 
> Andrew> +gdb_test "print st" " = simple::StringAtOffset {field1: \"hello\", field2: 1, field3: \"world\"}"
> 
> I think this line should probably be split after the first argument.
> Ok with this change.
> 
> Tom