Re: [PATCH v2 1/4] gdb/python: add gdbpy_borrowed_ref

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

On 15/05/2026 17:56, Tom Tromey wrote:
>>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> From: Tom Tromey <tom@tromey.com>
>> This adds a new gdbpy_borrowed_ref class.  This class is primarily for
>> code "documentation" purposes -- it makes it clear to the reader that
>> a given reference is borrowed.  However, it also adds a tiny bit of
>> safety, in that conversion to gdbpy_ref<> will acquire a new
>> reference.
> 
>> +template <class T = PyObject>
>> +class gdbpy_borrowed_ref
>> +{
> 
> On my "safety" branch I ended up rewriting this to have a base class
> that allows NULL and a derived class that does not.
> 
> And my initial review -- the one that didn't get sent -- suggested using
> this.
> 
> But I tend to think this patch isn't needed yet.  It's nice for the
> safety series but in this series it is only used a little in patch 2,
> and not even every spot that could/should use it -- like I noticed:
> 
>         /* PyDict_GetItemWithError returns a borrowed reference.  */
> -      PyObject *found = PyDict_GetItemWithError (d, file.get ());
> +      PyObject *found = PyDict_GetItemWithError (globals, file.get ());
> 

I can replace the raw PyObject * by gdbpy_borrowed_ref<>.

> So on the whole I'd prefer to drop this from this series.  That way it
> won't conflict or cause problems with the stuff I'm planning to send.
> 

A lot of upcoming patches are using gdbpy_borrowed_ref<> because it simplified a lot the code in 
some places.
Dropping it is going to make things definitely more difficult on my side.

> Also you've made this into a template class, which seems alright -- but
> not used in the series.  Maybe there's some later series depending on
> this?  That kind of thing is good to mention if so.

Yes, it is used later.

> 
> Tom

Given those information, do you still want to drop it ?

Matthieu