Re: [PATCH v2 2/5] [gdb] Make addrmap_mutable::insert_empty return bool

[Simon Marchi <simark@simark.ca>]

On 2025-08-23 00:20, Tom de Vries wrote:
> On 8/22/25 20:51, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> @@ -234,8 +235,14 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive,
>> Tom>         n && addrmap_node_key (n) <= end_inclusive;
>> Tom>         n = splay_tree_successor (addrmap_node_key (n)))
>> Tom>      {
>> Tom> -      if (! addrmap_node_value (n))
>> Tom> -    addrmap_node_set_value (n, obj);
>> Tom> +      if (addrmap_node_value (n))
>> Tom> +    {
>> Tom> +      /* Already mapped.  */
>> Tom> +      full_range = false;
>> Tom> +      continue;
>> Tom> +    }
>> Tom> +
>> Tom> +      addrmap_node_set_value (n, obj);
>>
>> I think using 'else' here would be better than 'continue', since loop
>> short-circuits are harder to understand.
> 
> Hi Tom,
> 
> thanks for the review.
> 
> I've pushed this using an if/else.
> 
> FWIW, I still think that they way I wrote is is much better.  My opinion is based on this coding standard rule ( https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code ).

I also like early returns and early continue in loops.  It tells me: you
don't need to think about that case for the rest of the function loop.

Simon