Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T

Matthieu Longo
2026-03-06T11:44:53Z

On 06/03/2026 11:37, Matthieu Longo wrote:
> On 03/03/2026 18:18, Tom Tromey wrote:
>>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
>>
>>> When ref_ptr<T,Policy>::new_reference() is specialized for 'PyObject'
>>> (i.e. gdbpy_ref<>), it currently requires the argument type to be exactly
>>> 'PyObject *'. As a result, pointers to subclasses of 'PyObject' must be
>>> explicitly cast before being passed, making call sites unnecessarily
>>> verbose.
>>
>>> This patch makes ref_ptr<T,Policy>::new_reference() a template method
>>> that accepts both T and subclasses of T, performing the cast to 'T *'
>>> internally when needed. This removes redundant casts at call sites
>>> without changing behavior.
>>
>> See this series
>>
>> https://inbox.sourceware.org/gdb-patches/20260221-python-ref-simplif-v1-0-5b06f48d3096@tromey.com/
>>
>> I can land that this week if it's convenient.
>>
>> Tom
>
> After a deeper examination, I am not sure that your series helps much for this patch.
>
> * Case 1: return value from the registry.
>
> PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
>   if (result != nullptr)
>     return gdbpy_ref<>::new_reference (result);
>
> If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
> gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
> if (result != nullptr)
>   return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */
>
>
> * Case 2: registration of the new object in the registry and return the value.
>
> gdbpy_ref<corefile_object> object
>     (PyObject_New (corefile_object, &corefile_object_type));
> ...
> cfpy_inferior_corefile_data_key.set (inf, object.get ());
> return gdbpy_ref<>::new_reference (object.release ());
>
> I could change the code as follows, but I am not sure that it makes the code more readable.
>
> cfpy_inferior_corefile_data_key.set (inf, decltype (object) (object).release ());
> return object;
>
> If the registry was storing gdbpy_ref<corefile_object> instead of 'corefile_object *', I guess that it could be simplified even further to:
>
> cfpy_inferior_corefile_data_key.set (inf, object);
> return object;
>
> What do you think ?
>
> However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
> All those improvements seem to me out of scope of my original patch.
>
> Matthieu

I realized that my response might be unclear.
I forgot to mention that the code I am referencing here is part of the next patch.

Matthieu