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

Simon Marchi
2020-03-30T15:02:34Z

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