Re: [PATCH 1/2] gdb/block: remove block_iterator::d::block, remove union

[Guinevere Larsen <guinevere@redhat.com>]

On 2/5/26 11:52 PM, simon.marchi@polymtl.ca wrote:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> The block field is never used.  The block is only used to grab a
> reference to the mdict, which is stored in the mdict_iterator.
>
> Remove it and remove the union.  Leave the compunit_symtab field, whose
> name unfortunately conflicts with the method.
>
> Change-Id: I09dbc42f937eaba6c70598acca8ff355c4e5bdb9

This change looks harmless enough, but since I don't feel like I 
understand it well enough to properly review, best I can say is, no 
issues running the testsuite or rebuilding, so

Tested-By: Guinevere Larsen <guinevere@redhat.com>

> ---
>   gdb/block.c |  9 +++------
>   gdb/block.h | 13 ++++++-------
>   2 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/block.c b/gdb/block.c
> index 64ca59d44a1d..5aa58545393b 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -407,8 +407,6 @@ initialize_block_iterator (const struct block *block,
>       which = STATIC_BLOCK;
>     else
>       {
> -      iter->d.block = block;
> -
>         /* A signal value meaning that we're iterating over a single
>   	 block.  */
>         iter->which = FIRST_LOCAL_BLOCK;
> @@ -428,14 +426,13 @@ initialize_block_iterator (const struct block *block,
>        directly.  */
>     if (cu->includes.empty ())
>       {
> -      iter->d.block = block;
>         /* A signal value meaning that we're iterating over a single
>   	 block.  */
>         iter->which = FIRST_LOCAL_BLOCK;
>       }
>     else
>       {
> -      iter->d.compunit_symtab = cu;
> +      iter->compunit_symtab_ = cu;
>         iter->which = which;
>       }
>   }
> @@ -446,9 +443,9 @@ compunit_symtab *
>   block_iterator::compunit_symtab () const
>   {
>     if (this->idx == -1)
> -    return this->d.compunit_symtab;
> +    return this->compunit_symtab_;
>   
> -  auto &includes = this->d.compunit_symtab->includes;
> +  auto &includes = this->compunit_symtab_->includes;
>   
>     if (this->idx < includes.size ())
>       return includes[this->idx];
> diff --git a/gdb/block.h b/gdb/block.h
> index d9a8c4b6ff89..b84ca12c35a6 100644
> --- a/gdb/block.h
> +++ b/gdb/block.h
> @@ -557,14 +557,13 @@ struct block_iterator
>      iterates.  Return nullptr if the iteration is finished.  */
>     struct compunit_symtab *compunit_symtab () const;
>   
> -  /* If we're iterating over a single block, this holds the block.
> -     Otherwise, it holds the canonical compunit.  */
> +  /* If iterating on a global or static blocks, iteration starts from the
> +     top-level CU and then continues with the global or static blocks of all
> +     the included CUs.  This field holds the compunit of the current block.
>   
> -  union
> -  {
> -    struct compunit_symtab *compunit_symtab;
> -    const struct block *block;
> -  } d;
> +     This field is not private because block_iterator must remain trivial,
> +     but treat it as private.  */
> +  struct compunit_symtab *compunit_symtab_;
>   
>     /* If we're trying to match a name, this will be non-NULL.  */
>     const lookup_name_info *name;
>
> base-commit: 1d688bb5f48a0feddf3ca3c60af2e91f4147dc36


-- 
Cheers,
Guinevere Larsen
It/she