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

[Tom Tromey <tom@tromey.com>]

>>>>> 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.

On my branch I ended up giving this a gdbpy_opt_borrowed_ref base class.

> +/* A class representing a borrowed reference.
> +
> +   This is a simple wrapper for a PyObject*.  Aside from documenting
> +   what the code does, the main advantage of using this is that
> +   conversion to a gdbpy_ref<> is guaranteed to make a new
> +   reference.  */
> +template <class T = PyObject>
> +class gdbpy_borrowed_ref

Also mine isn't templated like this though I guess this makes some
sense.

IIRC I felt like this was overkill.  I didn't read your subsequent
patches yet but I assume there's some use for it.

> +  /* Allow a (checked) conversion to any subclass of PyObject.  */
> +  template<typename TObj,
> +	   typename = std::is_convertible<TObj *, T *>>
> +  operator TObj * ()
> +  {
> +    //gdb_assert (PyObject_TypeCheck (m_obj, T::corresponding_object_type));

No commented-out code.  This is unsafe though.  I would prefer a check.

I think a name other than "TObj" would be good.
Even "U" would be better.  "T" and "TObj" are too close.

> +  /* When converting a borrowed reference to a gdbpy_ref<>, a new
> +     reference is acquired.  */
> +  template <typename TObj,
> +	    typename = std::is_convertible<TObj *, T *>>
> +  operator gdbpy_ref<TObj> ()
> +  {
> +    gdb_assert (m_obj != nullptr);
> +    return gdbpy_ref<TObj>::new_reference (m_obj);
> +  }

The naming thing here too.  And also safety if we can get it.

> +
> +  gdbpy_ref<T> strong_ref () const noexcept
> +  {
> +    return static_cast<gdbpy_ref<T>> (*this);
> +  }

I think it would be clearer to directly write this in terms of
new_reference.

Tom