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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

>> +class IntrusiveListPrinter:
>> +    """Print a struct intrusive_list."""
>> +
>> +    def __init__(self, val):
>> +        self._val = val
>> +
>> +        # Type of linked items.
>> +        self._item_type = self._val.type.template_argument(0)
>> +        self._node_ptr_type = gdb.lookup_type(
>> +            f"intrusive_list_node<{self._item_type.tag}>"
> 
> Hi,
> 
> I do not know what are the compatibility constraints / minimum python
> version required, but f-string do require at least python-3.6.  This
> covers all still supported python versions, but there might be distros
> out-there that are not there yet (there are a few of those in this
> file).

Huh... right.  It's not clear what we aim to support, I'd like if it was
clearer.  But we still support build against Python 2.7, so it would
make sense to try to be compatible with it.  I'll switch to using
`"...".format(...)`.

>> +        ).pointer()
>> +
>> +        # Type of value -> node converter.
>> +        self._conv_type = self._val.type.template_argument(1)
>> +
>> +        if self._uses_member_node():
>> +            # The second template argument of intrusive_member_node is a member
>> +            # pointer value.  Its value is the offset of the node member in the
>> +            # enclosing type.
>> +            member_node_ptr = self._conv_type.template_argument(1)
>> +            member_node_ptr = member_node_ptr.cast(gdb.lookup_type("int"))
>> +            self._member_node_offset = int(member_node_ptr)
>> +
>> +            # This is only needed in _as_node_ptr if using a member node.  Look it
>> +            # up here so we only do it once.
>> +            self._char_ptr_type = gdb.lookup_type("char").pointer()
>> +
>> +    def display_hint(self):
>> +        return "array"
>> +
>> +    # Return True if the list items use a node as a member.  Return False if
>> +    # they use a node as a base class.
>> +    def _uses_member_node(self):
> 
> The documentation for the function should probably go as a doc string,
> not a comment before the function:
> 
>     def _uses_member_node(self):
>         """ Return True if the list items use a node as a member.  Return False
> 	if they use a node as a base class.
> 	"""
> 
> The same remark stands for the other method in this file.

Done.

>> +        if self._conv_type.name.startswith("intrusive_member_node<"):
>> +            return True
>> +        elif self._conv_type.name.startswith("intrusive_base_node<"):
>> +            return False
>> +        else:
>> +            raise RuntimeError(
>> +                f"Unexpected intrusive_list value -> node converter type: {self._conv_type.name}"
>> +            )
>> +
>> +    def to_string(self):
>> +        s = f"intrusive list of {self._item_type}"
>> +
>> +        if self._uses_member_node():
>> +            node_member = self._conv_type.template_argument(1)
>> +            s += f", linked through {node_member}"
>> +
>> +        return s
>> +
>> +    # Given ELEM_PTR, a pointer to a list element, return a pointer to the
>> +    # corresponding intrusive_list_node.
>> +    def _as_node_ptr(self, elem_ptr):
>> +        assert elem_ptr.type.code == gdb.TYPE_CODE_PTR
>> +
>> +        if self._uses_member_node():
>> +            # Node as a memer: add the member node offset from to the element's
> 
> s/memer/member/ ?

Fixed.

I integrated the changes in my local tree.

Simon

PS: when you review, don't hesitate to trim the irrelevant parts of the
diff, it will make your comments easier to find and harder to miss.  I
almost missed the last one, especially because it was after your
signature!  Still, thank you very much for looking at the various
patches on the list, it's very appreciated.