Re: [PATCH 08/10] [gdb] Use block::block_and_superblocks

[Tom Tromey <tom@tromey.com>]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> diff --git a/gdb/blockframe.c b/gdb/blockframe.c
Tom> index 8aafa6ffe57..5c981748cbc 100644
Tom> --- a/gdb/blockframe.c
Tom> +++ b/gdb/blockframe.c
Tom> @@ -69,13 +69,14 @@ get_frame_block (const frame_info_ptr &frame, CORE_ADDR *addr_in_block)
 
Tom>    inline_count = frame_inlined_callees (frame);
 
Tom> -  while (inline_count > 0)
Tom> +  auto range = bl->block_and_superblocks ();
Tom> +  for (auto b = range.begin (); b != range.end (); ++b, bl = *b)
Tom>      {
Tom> -      if (bl->inlined_p ())
Tom> -	inline_count--;
Tom> +      if (inline_count <= 0)
Tom> +	break;
 
Tom> -      bl = bl->superblock ();
Tom> -      gdb_assert (bl != NULL);
Tom> +      if ((*b)->inlined_p ())
Tom> +	inline_count--;
Tom>      }

Tom>    return bl;

I think this particular change is less clear than the code it replaces.

Tom> diff --git a/gdb/dwarf2/line-program.c b/gdb/dwarf2/line-program.c
Tom> index b9da1b4799f..227ec01aec9 100644
Tom> --- a/gdb/dwarf2/line-program.c
Tom> +++ b/gdb/dwarf2/line-program.c
Tom> @@ -373,21 +373,15 @@ dwarf_find_and_extend_inline_block_range (dwarf2_cu *cu,
Tom>       the child of a non-inline block.  This is new inline block is our
Tom>       candidate for extending.  */
Tom>    struct block *block = nullptr;
Tom> -  for (const struct block *b = it->second;
Tom> -       b != nullptr;
Tom> -       b = b->superblock ())
Tom> -    {
Tom> -      if (b->function () != nullptr && b->inlined_p ())
Tom> -	{
Tom> -	  if (b->superblock () != nullptr
Tom> -	      && b->superblock ()->function () != nullptr
Tom> -	      && !b->superblock ()->inlined_p ())
Tom> -	    {
Tom> -	      block = const_cast<struct block *> (b);
Tom> -	      break;
Tom> -	    }
Tom> -	}
Tom> -    }
Tom> +  for (auto b : block::block_and_superblocks (it->second))
Tom> +    if (b->inlined_p ()
Tom> +	&& b->superblock () != nullptr
Tom> +	&& b->superblock ()->function () != nullptr
Tom> +	&& !b->superblock ()->inlined_p ())
Tom> +      {
Tom> +	block = const_cast<struct block *> (b);
Tom> +	break;
Tom> +      }

It took me a while to figure out why you removed the check of
'function', but it's because inlined_p already does this check.

It's helpful to call out this kind of change or to put it in a separate
patch.

Tom> --- a/gdb/infrun.c
Tom> +++ b/gdb/infrun.c
Tom> @@ -8261,13 +8261,15 @@ process_event_stop_test (struct execution_control_state *ecs)
Tom>        const struct block *prev
Tom>  	= block_for_pc (ecs->event_thread->control.step_frame_id.code_addr);
Tom>        const struct block *curr = block_for_pc (ecs->event_thread->stop_pc ());
Tom> -      while (curr != nullptr && !curr->contains (prev))
Tom> +      for (auto b : block::block_and_superblocks (curr))
Tom>  	{

This code would be clearer if 'curr' were eliminated entirely.
In this case I had to go see if it was used after the loop.

Tom> +  auto range = block::block_and_superblocks (cur_block);
Tom> +  for (auto b = range.begin (); b != range.end (); ++b, cur_block = *b)

This is another case where the result is harder to reason about that the
earlier code.

Maybe it could be salvaged somehow by assigning at the breaks, not sure.

Tom> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
Tom> index 89374bd8a2f..f882f3add17 100644
Tom> --- a/gdb/symmisc.c
Tom> +++ b/gdb/symmisc.c
Tom> @@ -961,11 +961,10 @@ block_depth (const struct block *block)
Tom>  {
Tom>    int i = 0;
 
Tom> -  while ((block = block->superblock ()) != NULL)
Tom> -    {
Tom> -      i++;
Tom> -    }
Tom> -  return i;
Tom> +  for (auto b [[maybe_unused]]: block::block_and_superblocks (block))
Tom> +    i++;
Tom> +
Tom> +  return i - 1;

Can this just be std::count?

Also this doesn't have to handle the case where the initial block is
nullptr.

Tom> +	  auto range = block::block_and_superblocks (bl);
Tom> +	  for (auto b = range.begin (); b != range.end (); ++b, bl = *b)

Like the earlier ones.  I don't think multiple assignments like this
make it clearer or easier to work with.

Tom