Re: [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper

Matthieu Longo
2026-03-06T12:05:57Z

On 03/03/2026 18:30, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
>
>> Python extension objects that support __dict__ must inherit from
>> gdbpy_dict_wrapper, a wrapper class that stores the PyObject
>> corresponding to the __dict__ attribute.
>
>> Currently, management of this dictionary is not centralized, and
>> each Python extension object implements its own logic to create,
>> access, and destroy it.
>
>> This patch focuses on the allocation of the dictionary, introduces
>> a new helper function, gdbpy_dict_wrapper::allocate_dict(), and
>> adapts the existing code to use the helper.
>
>> + if (! gdbpy_dict_wrapper::allocate_dict (object))
>> return nullptr;
>
> gdb style doesn't put a space after '!' in an expression.
> There's a few instances of this.
>

Fixed.

>> +
>> + template <class T>
>> + static bool
>> + allocate_dict (gdbpy_ref<T>& self)
>
> Space before "&"
>
>> + {
>> + static_assert (std::is_base_of_v<gdbpy_dict_wrapper, T>
>> + || std::is_same_v<PyObject, T>);
>> + auto *obj = static_cast<gdbpy_dict_wrapper *> (self.get ());
>> + obj->dict = PyDict_New ();
>> + return obj->dict != nullptr;
>
> It seems to me that if this were a non-static method, then it also would
> not have to be a template method, like
>
> bool allocate_dict ()
> {
> // maybe? gdb_assert (dict == nullptr);
> dict = PyDict_New ();
> return dict != nullptr;
> }
>
> The idea being that inheritance takes care of the details, so this
> works:
>
> gdbpy_ref<field_object> blah;
> if (blah->allocate_dict ()) ...
>
> Unless we have a case where this is needed for plain gdbpy_ref<>?
> But based on an earlier patch I would think that's not possible.
>
> Tom

You are right, I needlessly overcomplicated things.
There is no case where this is needed for plain gdbpy_ref<>.
The approach with the inheritance is simpler and cleaner.

Regarding the assertion "gdb_assert (dict == nullptr);", it seems that PyObject_New() does not zero the memory so the assertion can be randomly triggered by garbage. I won't keep it.

Fixed in the next revision.

Matthieu