Re: [PATCH 11/20] Add reprocessing flag to struct attribute

Simon Marchi
2020-03-30T15:32:37Z

On 2020-03-28 3:21 p.m., Tom Tromey wrote:
> diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
> index 72ec13c11f9..73c1ef9f792 100644
> --- a/gdb/dwarf2/attribute.c
> +++ b/gdb/dwarf2/attribute.c
> @@ -206,3 +206,17 @@ attribute::form_is_unsigned () const
> || form == DW_FORM_ref8
> || form == DW_FORM_ref_udata);
> }
> +
> +/* See attribute.h. */
> +
> +bool
> +attribute::form_is_reprocessed () const

The name is odd, the "form" isn't reprocessed. What would you think of
"form_requires_processing"? The "requires_reprocessing" field implicitly
means "attribute_requires_reprocessing". But if we want to avoid confusion
between the two, the field could be renamed "reprocessing_done", and its
logic inverted.

> @@ -179,8 +193,22 @@ struct attribute
> u.unsnd = unsnd;
> }
>
> + /* Temporarily this attribute to an unsigned integer. This is used

Missing a word here?

> + only for those forms that require reprocessing. */
> + void set_unsigned_reprocess (ULONGEST unsnd)
> + {
> + gdb_assert (form_is_reprocessed ());
> + u.unsnd = unsnd;
> + requires_reprocessing = 1;
> + }
> +
> +
> + ENUM_BITFIELD(dwarf_attribute) name : 15;
> +
> + /* If this requires reprocessing, is it in its final form, or is it
> + still stored as an unsigned? */
> + unsigned int requires_reprocessing : 1;

It would be good to explain what we mean by "reprocessing", here would be a good
place. It would be good to give the example of the DW_FORM_strx form, where we
could encounter this form before having seen the corresponding
DW_AT_str_offsets_base attribute. So we first store the offset as an unsigned
integer in the attribute, then "reprocess" it later to fetch the actual string.

Simon