Re: [PATCH 02/11] gdb: introduce intrusive_list, make thread_info use it

[Pedro Alves <pedro@palves.net>]

On 2021-07-06 10:45 p.m., Simon Marchi wrote:
> On 2021-07-06 5:02 p.m., Pedro Alves wrote:
>> On 2021-07-06 8:38 p.m., Simon Marchi wrote:
>>> On 2021-07-05 11:44 a.m., Pedro Alves wrote:
>>>>>>  $1 = intrusive list of thread_info = {
>>>>    {id = 1.1, ptid = 1000.1000.0, state = THREAD_RUNNING}, 
>>>>    {id = 1.3, ptid = 1000.1002.0, state = THREAD_STOPPED},
>>>>    {id = 1.5, ptid = 1000.3672.0, state = THREAD_STOPPED}
>>>>  }
>>>
>>> How would you accomplish this, with a struct thread_info
>>> pretty-printer?  This means that printing a single thread like this:
>>>
>>>   (gdb) print *tp
>>>
>>> would also produce the short output, I don't think we want that.  When
>>> we print a single thread_info structure, I think it's good to have all
>>> the fields shown.
>>
>> I don't have a great answer.  It feels to me like the pretty printer code
>> should ask the container if it has a custom printer for the element, and only
>> if it doesn't would it look up the printer for the element's type.
> 
> That would be a new feature that doesn't exist today, I think: a
> different printer for a type when it's printed as a container element
> than when it's printed standalone.

Yeah.

> I think these are all good ideas for improvements, but I'd rather keep
> them for later (if someone wants to implement them, I'm not sure I
> will).  We could bike-shed for a while on how to display a thread_info,
> what to include / what to exclude, etc.  I think that my original
> proposal is strictly better than what we have today, in the sense that
> today you just can't print the whole list of threads, so we don't lose
> anything.

The discussion about the thread_info pretty printer, yes, agreed.

However, the discussion on the list printer itself, one point that we should
settle discuss a bit more is whether it is really the right approach to make it
show children as pointers.  

Showing pointers really looks not useful to me.  The only thing I think I get
out of it is that there are elements in the list.

I mean, AFAICT, even the std::list printer shows objects, not pointers,
for instance:

 (gdb) p my_list
 $1 = std::__cxx11::list = {[0] = 1, [1] = 2, [2] = 3}

Hmm, funny, it shows the indexes, even though I don't have
array-indexes off.  Guess it must be using "map", but I haven't checked.

Imagine we were using std::list for thread_info objects.

But I don't think that that discussion should block the main change
from going in.  I'd support moving the printer out to a separate patch,
even.