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

[Simon Marchi <simark@simark.ca>]

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