From 22a23ffb3150037fc62a5b74e3a0a8cec9cbdf6f Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Thu, 22 Feb 2024 23:06:55 +0000 Subject: [PATCH] Tidy some usage of `py.from_borrowed_ptr` and `py.from_borrowed_ptr_or_opt` (#3877) * Tidy some usage of py.from_borrowed_ptr * Add BoundRef::ref_from_ptr_or_opt --- pyo3-macros-backend/src/pymethod.rs | 6 +++--- src/ffi/tests.rs | 14 ++++++-------- src/impl_/pyclass.rs | 8 +++++--- src/impl_/pymethods.rs | 7 +++++++ src/instance.rs | 12 ++++++++++++ 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index d45d2e12..74072f2a 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -572,12 +572,12 @@ pub fn impl_py_setter_def( _slf: *mut _pyo3::ffi::PyObject, _value: *mut _pyo3::ffi::PyObject, ) -> _pyo3::PyResult<::std::os::raw::c_int> { - let _value = py - .from_borrowed_ptr_or_opt(_value) + use ::std::convert::Into; + let _value = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_value) .ok_or_else(|| { _pyo3::exceptions::PyAttributeError::new_err("can't delete attribute") })?; - let _val = _pyo3::FromPyObject::extract(_value)?; + let _val = _pyo3::FromPyObject::extract_bound(_value.into())?; #( #holders )* _pyo3::callback::convert(py, #setter_impl) } diff --git a/src/ffi/tests.rs b/src/ffi/tests.rs index d67bd848..3532172c 100644 --- a/src/ffi/tests.rs +++ b/src/ffi/tests.rs @@ -57,9 +57,9 @@ fn test_date_fromtimestamp() { #[test] fn test_utc_timezone() { Python::with_gil(|py| { - let utc_timezone: &PyAny = unsafe { + let utc_timezone: Bound<'_, PyAny> = unsafe { PyDateTime_IMPORT(); - py.from_borrowed_ptr(PyDateTime_TimeZone_UTC()) + Bound::from_borrowed_ptr(py, PyDateTime_TimeZone_UTC()) }; let locals = PyDict::new_bound(py); locals.set_item("utc_timezone", utc_timezone).unwrap(); @@ -254,35 +254,33 @@ fn test_get_tzinfo() { crate::Python::with_gil(|py| { use crate::types::{PyDateTime, PyTime}; - use crate::PyAny; let utc = &timezone_utc_bound(py); let dt = PyDateTime::new_bound(py, 2018, 1, 1, 0, 0, 0, 0, Some(utc)).unwrap(); assert!( - unsafe { py.from_borrowed_ptr::(PyDateTime_DATE_GET_TZINFO(dt.as_ptr())) } + unsafe { Bound::from_borrowed_ptr(py, PyDateTime_DATE_GET_TZINFO(dt.as_ptr())) } .is(utc) ); let dt = PyDateTime::new_bound(py, 2018, 1, 1, 0, 0, 0, 0, None).unwrap(); assert!( - unsafe { py.from_borrowed_ptr::(PyDateTime_DATE_GET_TZINFO(dt.as_ptr())) } + unsafe { Bound::from_borrowed_ptr(py, PyDateTime_DATE_GET_TZINFO(dt.as_ptr())) } .is_none() ); let t = PyTime::new_bound(py, 0, 0, 0, 0, Some(utc)).unwrap(); assert!( - unsafe { py.from_borrowed_ptr::(PyDateTime_TIME_GET_TZINFO(t.as_ptr())) } - .is(utc) + unsafe { Bound::from_borrowed_ptr(py, PyDateTime_TIME_GET_TZINFO(t.as_ptr())) }.is(utc) ); let t = PyTime::new_bound(py, 0, 0, 0, 0, None).unwrap(); assert!( - unsafe { py.from_borrowed_ptr::(PyDateTime_TIME_GET_TZINFO(t.as_ptr())) } + unsafe { Bound::from_borrowed_ptr(py, PyDateTime_TIME_GET_TZINFO(t.as_ptr())) } .is_none() ); }) diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 2bd39dda..e2204fab 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -6,8 +6,10 @@ use crate::{ internal_tricks::extract_c_string, pycell::PyCellLayout, pyclass_init::PyObjectInit, + types::any::PyAnyMethods, types::PyBool, - Py, PyAny, PyCell, PyClass, PyErr, PyMethodDefType, PyNativeType, PyResult, PyTypeInfo, Python, + Borrowed, Py, PyAny, PyCell, PyClass, PyErr, PyMethodDefType, PyNativeType, PyResult, + PyTypeInfo, Python, }; use std::{ borrow::Cow, @@ -811,8 +813,8 @@ slot_fragment_trait! { other: *mut ffi::PyObject, ) -> PyResult<*mut ffi::PyObject> { // By default `__ne__` will try `__eq__` and invert the result - let slf: &PyAny = py.from_borrowed_ptr(slf); - let other: &PyAny = py.from_borrowed_ptr(other); + let slf = Borrowed::from_ptr(py, slf); + let other = Borrowed::from_ptr(py, other); slf.eq(other).map(|is_eq| PyBool::new_bound(py, !is_eq).to_owned().into_ptr()) } } diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index eef3b569..196766d0 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -483,6 +483,13 @@ impl<'a, 'py> BoundRef<'a, 'py, PyAny> { BoundRef(Bound::ref_from_ptr(py, ptr)) } + pub unsafe fn ref_from_ptr_or_opt( + py: Python<'py>, + ptr: &'a *mut ffi::PyObject, + ) -> Option { + Bound::ref_from_ptr_or_opt(py, ptr).as_ref().map(BoundRef) + } + pub unsafe fn downcast_unchecked(self) -> BoundRef<'a, 'py, T> { BoundRef(self.0.downcast_unchecked::()) } diff --git a/src/instance.rs b/src/instance.rs index ecf12811..307b341f 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -191,6 +191,18 @@ impl<'py> Bound<'py, PyAny> { ) -> &'a Self { &*(ptr as *const *mut ffi::PyObject).cast::>() } + + /// Variant of the above which returns `None` for null pointers. + /// + /// # Safety + /// - `ptr` must be a valid pointer to a Python object for the lifetime `'a, or null. + #[inline] + pub(crate) unsafe fn ref_from_ptr_or_opt<'a>( + _py: Python<'py>, + ptr: &'a *mut ffi::PyObject, + ) -> &'a Option { + &*(ptr as *const *mut ffi::PyObject).cast::>>() + } } impl<'py, T> Bound<'py, T>