Re: [PATCH 16/20] Change is_valid_DW_AT_defaulted to a method on attribute

Simon Marchi
2020-03-30T16:00:30Z

On 2020-03-28 3:22 p.m., Tom Tromey wrote:
> This changes is_valid_DW_AT_defaulted to be a method on struct attribute.
> Now it correctly respects the form of the attribute.
>
> 2020-03-28 Tom Tromey <tom@tromey.com>
>
> * dwarf2/read.c (is_valid_DW_AT_defaulted): Move to attribute.c.
> (dwarf2_add_member_fn): Update.
> * dwarf2/attribute.h (struct attribute) <defaulted>: Declare.
> * dwarf2/attribute.c (attribute::defaulted): New method, from
> is_valid_DW_AT_defaulted.
> ---
> gdb/ChangeLog | 8 ++++++++
> gdb/dwarf2/attribute.c | 21 +++++++++++++++++++++
> gdb/dwarf2/attribute.h | 8 ++++++++
> gdb/dwarf2/read.c | 23 ++---------------------
> 4 files changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
> index 94bedf6dbd3..ee5e6321bde 100644
> --- a/gdb/dwarf2/attribute.c
> +++ b/gdb/dwarf2/attribute.c
> @@ -219,3 +219,24 @@ attribute::form_is_reprocessed () const
> || form == DW_FORM_addrx
> || form == DW_FORM_GNU_addr_index);
> }
> +
> +/* See attribute.h. */
> +
> +dwarf_defaulted_attribute
> +attribute::defaulted () const
> +{
> + LONGEST value = constant_value (-1);
> +
> + switch (value)
> + {
> + case DW_DEFAULTED_no:
> + case DW_DEFAULTED_in_class:
> + case DW_DEFAULTED_out_of_class:
> + return (dwarf_defaulted_attribute) value;
> + }
> +
> + if (form_is_constant ())
> + complaint (_("unrecognized DW_AT_defaulted value (%s)"),
> + plongest (value));
> + return DW_DEFAULTED_no;

I don't quite understand the logic here, we check form_is_constant after having called
constant_value? If the form is not a constant, shouldn't we emit a warning to that
effect (and return _no)?

> +}
> diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
> index 546156283b3..dce93b154a8 100644
> --- a/gdb/dwarf2/attribute.h
> +++ b/gdb/dwarf2/attribute.h
> @@ -29,6 +29,7 @@
>
> #include "dwarf2.h"
> #include "gdbtypes.h"
> +#include "gdbsupport/gdb_optional.h"
>
> /* Blocks are a bunch of untyped bytes. */
> struct dwarf_block
> @@ -229,6 +230,13 @@ struct attribute
> return requires_reprocessing;
> }
>
> + /* Return the value as one of the recognized enum
> + dwarf_defaulted_attribute constants according to DWARF5 spec,
> + Table 7.24. If the value is incorrect, or if this attribute has
> + the wrong form, then a complaint is issued and DW_DEFAULTED_no is
> + returned. */
> + dwarf_defaulted_attribute defaulted () const;
> +
>

There's an extra empty line here, might as well remove it.

Simon