Re: [PATCH 05/20] Remove some uses of DW_STRING_IS_CANONICAL

[Simon Marchi <simark@simark.ca>]

On 2020-03-28 3:21 p.m., Tom Tromey wrote:
> This removes the rvalue uses of DW_STRING_IS_CANONICAL, replacing them
> with an accessor method.
> 
> 2020-03-28  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.c (anonymous_struct_prefix, dwarf2_name)
> 	(dump_die_shallow): Use canonical_p.
> 	* dwarf2/attribute.h (struct attribute) <canonical_p>: New
> 	method.
> ---
>  gdb/ChangeLog          | 7 +++++++
>  gdb/dwarf2/attribute.h | 6 ++++++
>  gdb/dwarf2/read.c      | 8 ++++----
>  3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
> index cefd3c5541e..f20540559aa 100644
> --- a/gdb/dwarf2/attribute.h
> +++ b/gdb/dwarf2/attribute.h
> @@ -100,6 +100,12 @@ struct attribute
>  
>    LONGEST constant_value (int default_value) const;
>  
> +  /* Return true if this attribute holds a canonical string.  */
> +  bool canonical_p () const
> +  {
> +    return string_is_canonical;
> +  }

Would you be able to improve the documentation to say what is
a "canonical string"?  If this is a DWARF concept and described
in the DWARF spec, you could just refer to the relevant section
of the standard too.

Is this method only relevant if the attribute is of a string
form?  If so, the method name should perhaps still have "string"
in its name, like "is_canonical_string" or "canonical_string_p".

I presume it doesn't make sense to call this method when the
attribute has a non-string form, so should there be a gdb_assert
that checks that the attribute has a string form?

Simon