Pyo3: Support all receiver types for all protocol methods

[davidhewitt]

At the moment the protocol methods are in an inconsistent state: some of them can take PyRef or PyRefMut, and some of them take &self or &mut self.

This is confusing to users and also gets in the way in certain cases like needing to access Python inside a protocol method (which can be obtained from PyRef::py() for example) or wanting to return PyRef<Self>.

The simple solution is to just change all protocol methods to use TryFromPyCell trait. This is however a breaking change.

The _better_ solution is to change all #[pyproto] methods to support any of the five of &PyCell, PyRef<Self>, PyRefMut<Self>, &self and &mut self, just like we support for #[pymethods].

I've had some ideas how to approach this second point so would like to take a shot at it soon.

[kngwyu]

The better solution is to change all #[pyproto] methods to support any of the five of &PyCell, PyRef, PyRefMut, &self and &mut self, just like we support for #[pymethods].

So what would the trait definition would be?
Or we remove __dunder__ methods?

[davidhewitt]

I think I see a way to make a PyMethodReceiver trait which is implemented by all five types.

[davidhewitt]

Quick update from me: over the weekend I had a little time to play with a trait along the lines of:

pub trait PyMethodReceiver<'a>: Sized {
    fn receive<T, U>(slf: &'a PyAny, f: impl FnOnce(Self) -> T) -> PyResult<U>
    where
        T: IntoPyCallbackOutput<U>,
        U: 'static;
}

It _almost_ works, but there's some lifetime challenges about making sure the receiver lifetime is scoped correctly and safely. For the &self and &mut self receivers, I currently have some issues:

// THIS IMPL ISN'T SAFE BECAUSE &'a C IN THE CLOSURE CAN OUTLIVE THE `PYREF` GUARD
impl<'a, C: PyClass + 'a> PyMethodReceiver<'a> for &'a C {
    fn receive<T, U>(slf: &'a PyAny, f: impl FnOnce(Self) -> T) -> PyResult<U>
    where
        T: IntoPyCallbackOutput<U>,
        U: 'static
    {
        let cell: &PyCell<C> = slf.extract()?;

        // Introduce a PyRef to hold the guard. The lifetime of this is not long enough.
        let _ref = cell.try_borrow()?;

        // XXX: Use of unsafe below is not sound; was a hack to get compilation but the lifetime inferred outlives the guard.
        f(unsafe { cell.try_borrow_unguarded()? }).convert(slf.py())
    }
}

I'm hopeful that if I continue experimenting with designs in this space I'll be able to come up with a definition which is sound and gets us what we want.

[kngwyu]

So you mean you're extending the current fn __dunder__(slf: Self::Receiver...) to accept &Self or &mut Self type?
Interesting, but I'm not sure we really need this.
Both fn __dunder__(slf: &Self) and fn __dunder__(slf: PyRef<Self>) are not straightforward to write, so I think the usablity gain is not so much.

[davidhewitt]

I think that the most important thing is that we support fn __dunder__(slf: PyRef<Self>), for a couple of reasons:

• it's possible to return slf: PyRef<Self> from methods, which is _not_ possible with &self.
• it's possible to get Python from slf.py(), which is also not possible with &self.

So for 0.13 we could change all dunder methods to use TryFromPyCell trait, even if we can't support the full set.