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

[Matthieu Longo <matthieu.longo@arm.com>]

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