Re: [PATCH 02/13] [gdb/symtab] Check effect in parent_map::set_parent

[Tom de Vries <tdevries@suse.de>]

On 10/20/23 21:51, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Set_parent uses m_die_range_map.set_empty to set the parent of a range.
> Tom> If part of the range is already set, it remains the same.
> 
> Tom> If the entire range is already set, the set_empty has no effect, silently.
> 
> Tom> Fix this by verifying that calling set_empty has the desired effect on the
> Tom> start and end points of the range.
> 
> This seems like it might be better as a unit test.

We're trying to check the uses of set_parent, not the implementation, so 
a unit test doesn't help there.

Consider the following sequence:
- set_parent (3, 6, b)
- set_parent (1, 10, a)

Now the parent map looks like:
- [1-2] : a
- [3-6] : b
- [7-10]: a

Now say we accidentally execute this in the opposite order:
- set_parent (1, 10, a)
- set_parent (3, 6, b)

Now the parent map looks like this instead:
- [1-10]: a

The second set_parent has no effect, and this happens silently.  The 
assert tries to make this mistake visible.

It could be fitting to add some kind of gdb_checking_assert scheme for 
this, which is off by default in releases, and can be switched on by 
some means, if this is considered too expensive.

Anyway, since it raises questions, I'll drop this patch for now, but I'd 
like to note that it was useful for me in development of this patch series.

Thanks,
- Tom