Re: [PATCH v6 3/3] gdb/dwarf: add cooked_index_entry::parent_is_deferred

[Andrew Burgess <aburgess@redhat.com>]

This looks good to me.  But because I have to find something to earn my
pay...

Simon Marchi <simon.marchi@efficios.com> writes:

> Add a parent_is_deferred method to cooked_index_entry, mirroring
> name_is_deferred, and use it instead of checkingthe IS_PARENT_DEFERRED

... missing space in 'checkingthe'.

But otherwise, LGTM:

Approved-By: Andrew Burgess <aburgess@redhat.com>

thanks,
Andrew

> flag directly.
>
> Change-Id: Id684b30ec8aa81ac554e5c3edb8f7197c7238469
> ---
>  gdb/dwarf2/cooked-index-entry.h | 12 ++++++++----
>  gdb/dwarf2/cooked-index-shard.c | 10 +++++-----
>  gdb/dwarf2/cooked-index.c       |  2 +-
>  3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/dwarf2/cooked-index-entry.h b/gdb/dwarf2/cooked-index-entry.h
> index 2637b8255258..462ec0640a9b 100644
> --- a/gdb/dwarf2/cooked-index-entry.h
> +++ b/gdb/dwarf2/cooked-index-entry.h
> @@ -224,17 +224,21 @@ struct cooked_index_entry : public allocate_on_obstack<cooked_index_entry>
>      return compare (canonical, other.canonical, SORT) < 0;
>    }
>  
> +  /* Return true if this entry's parent still has to be resolved.  */
> +  bool parent_is_deferred () const
> +  { return (flags & IS_PARENT_DEFERRED) != 0; }
> +
>    /* Set parent entry to PARENT.  */
>    void set_parent (const cooked_index_entry *parent)
>    {
> -    gdb_assert ((flags & IS_PARENT_DEFERRED) == 0);
> +    gdb_assert (!parent_is_deferred ());
>      m_parent_entry.resolved = parent;
>    }
>  
>    /* Resolve deferred parent entry to PARENT.  */
>    void resolve_parent (const cooked_index_entry *parent)
>    {
> -    gdb_assert ((flags & IS_PARENT_DEFERRED) != 0);
> +    gdb_assert (parent_is_deferred ());
>      flags = flags & ~IS_PARENT_DEFERRED;
>      m_parent_entry.resolved = parent;
>    }
> @@ -242,14 +246,14 @@ struct cooked_index_entry : public allocate_on_obstack<cooked_index_entry>
>    /* Return parent entry.  */
>    const cooked_index_entry *get_parent () const
>    {
> -    gdb_assert ((flags & IS_PARENT_DEFERRED) == 0);
> +    gdb_assert (!parent_is_deferred ());
>      return m_parent_entry.resolved;
>    }
>  
>    /* Return deferred parent entry.  */
>    parent_map::addr_type get_deferred_parent () const
>    {
> -    gdb_assert ((flags & IS_PARENT_DEFERRED) != 0);
> +    gdb_assert (parent_is_deferred ());
>      return m_parent_entry.deferred;
>    }
>  
> diff --git a/gdb/dwarf2/cooked-index-shard.c b/gdb/dwarf2/cooked-index-shard.c
> index af8c3e9527ac..206fe4483a40 100644
> --- a/gdb/dwarf2/cooked-index-shard.c
> +++ b/gdb/dwarf2/cooked-index-shard.c
> @@ -85,7 +85,7 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
>  				       parent_entry, per_cu);
>    m_entries.push_back (result);
>  
> -  if ((flags & IS_PARENT_DEFERRED) != 0)
> +  if (result->parent_is_deferred ())
>      m_have_deferred_parents = true;
>  
>    if (result->name_is_deferred ())
> @@ -109,8 +109,8 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
>       by language_may_use_plain_main as well), but it's handy as a spot
>       to document.  */
>    else if (lang != language_unknown
> -	   && (flags & IS_PARENT_DEFERRED) == 0
> -	   && parent_entry.resolved == nullptr
> +	   && !result->parent_is_deferred ()
> +	   && result->get_parent () == nullptr
>  	   && m_main == nullptr
>  	   && language_may_use_plain_main (lang)
>  	   && !result->name_is_deferred ()
> @@ -238,7 +238,7 @@ cooked_index_shard::resolve_deferred_parents
>    gdb_assert (m_have_deferred_parents);
>  
>    for (cooked_index_entry *entry : m_entries)
> -    if ((entry->flags & IS_PARENT_DEFERRED) != 0)
> +    if (entry->parent_is_deferred ())
>        {
>  	const cooked_index_entry *new_parent
>  	  = parent_maps->find (entry->get_deferred_parent ());
> @@ -308,7 +308,7 @@ cooked_index_shard::canonicalize_names ()
>    for (cooked_index_entry *entry : m_entries)
>      {
>        /* Deferred parents should not reach this point.  */
> -      gdb_assert ((entry->flags & IS_PARENT_DEFERRED) == 0);
> +      gdb_assert (!entry->parent_is_deferred ());
>  
>        /* Entries without a name are filtered out during the call to
>  	 prune_nameless_entries.  */
> diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
> index 96996ebc4603..89a93eff6998 100644
> --- a/gdb/dwarf2/cooked-index.c
> +++ b/gdb/dwarf2/cooked-index.c
> @@ -391,7 +391,7 @@ cooked_index::dump (gdbarch *arch)
>        gdb_printf ("    DIE offset: %s\n", sect_offset_str (entry->die_offset));
>        gdb_printf ("    CU index:   %u\n", entry->per_cu->index);
>  
> -      if ((entry->flags & IS_PARENT_DEFERRED) != 0)
> +      if (entry->parent_is_deferred ())
>  	gdb_printf ("    parent:     deferred (%" PRIx64 ")\n",
>  		    entry->get_deferred_parent ());
>        else if (entry->get_parent () != nullptr)
> -- 
> 2.55.0