From e5756d41b24182852128ecd282b474adf076d696 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 3 Sep 2024 15:24:23 +0200 Subject: [PATCH] add `Borrowed::as_ptr` --- newsfragments/4520.added.md | 1 + src/conversions/chrono.rs | 2 +- src/coroutine.rs | 5 +-- src/instance.rs | 19 +++++++--- src/types/any.rs | 72 ++++++++++++++++++------------------- src/types/dict.rs | 12 ++----- src/types/mapping.rs | 2 +- src/types/sequence.rs | 2 +- tests/test_coroutine.rs | 5 +-- 9 files changed, 57 insertions(+), 63 deletions(-) create mode 100644 newsfragments/4520.added.md diff --git a/newsfragments/4520.added.md b/newsfragments/4520.added.md new file mode 100644 index 00000000000..d9952934de5 --- /dev/null +++ b/newsfragments/4520.added.md @@ -0,0 +1 @@ +Add `Borrowed::as_ptr`. diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs index e33c12b93e7..ddd4e39d9e3 100644 --- a/src/conversions/chrono.rs +++ b/src/conversions/chrono.rs @@ -710,7 +710,7 @@ fn warn_truncated_leap_second(obj: &Bound<'_, PyAny>) { ffi::c_str!("ignored leap-second, `datetime` does not support leap-seconds"), 0, ) { - e.write_unraisable(py, Some(&obj.as_borrowed())) + e.write_unraisable(py, Some(obj)) }; } diff --git a/src/coroutine.rs b/src/coroutine.rs index 22aad47f5c6..82f5460f03e 100644 --- a/src/coroutine.rs +++ b/src/coroutine.rs @@ -107,10 +107,7 @@ impl Coroutine { if let Some(future) = self.waker.as_ref().unwrap().initialize_future(py)? { // `asyncio.Future` must be awaited; fortunately, it implements `__iter__ = __await__` // and will yield itself if its result has not been set in polling above - if let Some(future) = PyIterator::from_object(&future.as_borrowed()) - .unwrap() - .next() - { + if let Some(future) = PyIterator::from_object(future).unwrap().next() { // future has not been leaked into Python for now, and Rust code can only call // `set_result(None)` in `Wake` implementation, so it's safe to unwrap return Ok(future.unwrap().into()); diff --git a/src/instance.rs b/src/instance.rs index c71cf89c02d..26c6b14ffa0 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -659,6 +659,19 @@ impl<'a, 'py, T> Borrowed<'a, 'py, T> { (*self).clone() } + /// Returns the raw FFI pointer represented by self. + /// + /// # Safety + /// + /// Callers are responsible for ensuring that the pointer does not outlive self. + /// + /// The reference is borrowed; callers should not decrease the reference count + /// when they are finished with the pointer. + #[inline] + pub fn as_ptr(self) -> *mut ffi::PyObject { + self.0.as_ptr() + } + pub(crate) fn to_any(self) -> Borrowed<'a, 'py, PyAny> { Borrowed(self.0, PhantomData, self.2) } @@ -2062,11 +2075,7 @@ a = A() #[test] fn test_py2_into_py_object() { Python::with_gil(|py| { - let instance = py - .eval(ffi::c_str!("object()"), None, None) - .unwrap() - .as_borrowed() - .to_owned(); + let instance = py.eval(ffi::c_str!("object()"), None, None).unwrap(); let ptr = instance.as_ptr(); let instance: PyObject = instance.clone().unbind(); assert_eq!(instance.as_ptr(), ptr); diff --git a/src/types/any.rs b/src/types/any.rs index d9aa845106e..5e40a0fdc4b 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -10,7 +10,7 @@ use crate::type_object::{PyTypeCheck, PyTypeInfo}; #[cfg(not(any(PyPy, GraalPy)))] use crate::types::PySuper; use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType}; -use crate::{err, ffi, BoundObject, Py, Python}; +use crate::{err, ffi, Borrowed, BoundObject, Py, Python}; use std::cell::UnsafeCell; use std::cmp::Ordering; use std::os::raw::c_int; @@ -887,7 +887,7 @@ macro_rules! implement_binop { { fn inner<'py>( any: &Bound<'py, PyAny>, - other: &Bound<'_, PyAny>, + other: Borrowed<'_, 'py, PyAny>, ) -> PyResult> { unsafe { ffi::$c_api(any.as_ptr(), other.as_ptr()).assume_owned_or_err(any.py()) } } @@ -895,7 +895,7 @@ macro_rules! implement_binop { let py = self.py(); inner( self, - &other + other .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -934,7 +934,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner<'py>( any: &Bound<'py, PyAny>, - attr_name: &Bound<'_, PyString>, + attr_name: Borrowed<'_, '_, PyString>, ) -> PyResult> { unsafe { ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr()) @@ -944,7 +944,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { inner( self, - &attr_name + attr_name .into_pyobject(self.py()) .map_err(Into::into)? .as_borrowed(), @@ -958,8 +958,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner( any: &Bound<'_, PyAny>, - attr_name: &Bound<'_, PyString>, - value: &Bound<'_, PyAny>, + attr_name: Borrowed<'_, '_, PyString>, + value: Borrowed<'_, '_, PyAny>, ) -> PyResult<()> { err::error_on_minusone(any.py(), unsafe { ffi::PyObject_SetAttr(any.as_ptr(), attr_name.as_ptr(), value.as_ptr()) @@ -969,11 +969,11 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &attr_name + attr_name .into_pyobject(py) .map_err(Into::into)? .as_borrowed(), - &value + value .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -985,7 +985,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { where N: IntoPyObject<'py, Target = PyString>, { - fn inner(any: &Bound<'_, PyAny>, attr_name: &Bound<'_, PyString>) -> PyResult<()> { + fn inner(any: &Bound<'_, PyAny>, attr_name: Borrowed<'_, '_, PyString>) -> PyResult<()> { err::error_on_minusone(any.py(), unsafe { ffi::PyObject_DelAttr(any.as_ptr(), attr_name.as_ptr()) }) @@ -994,7 +994,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &attr_name + attr_name .into_pyobject(py) .map_err(Into::into)? .as_borrowed(), @@ -1005,7 +1005,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { where O: IntoPyObject<'py>, { - fn inner(any: &Bound<'_, PyAny>, other: &Bound<'_, PyAny>) -> PyResult { + fn inner(any: &Bound<'_, PyAny>, other: Borrowed<'_, '_, PyAny>) -> PyResult { let other = other.as_ptr(); // Almost the same as ffi::PyObject_RichCompareBool, but this one doesn't try self == other. // See https://github.com/PyO3/pyo3/issues/985 for more. @@ -1030,7 +1030,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &other + other .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1044,7 +1044,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner<'py>( any: &Bound<'py, PyAny>, - other: &Bound<'_, PyAny>, + other: Borrowed<'_, 'py, PyAny>, compare_op: CompareOp, ) -> PyResult> { unsafe { @@ -1056,7 +1056,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &other + other .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1161,7 +1161,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner<'py>( any: &Bound<'py, PyAny>, - other: &Bound<'_, PyAny>, + other: Borrowed<'_, 'py, PyAny>, ) -> PyResult> { unsafe { ffi::PyNumber_Divmod(any.as_ptr(), other.as_ptr()).assume_owned_or_err(any.py()) @@ -1171,7 +1171,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &other + other .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1188,8 +1188,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner<'py>( any: &Bound<'py, PyAny>, - other: &Bound<'_, PyAny>, - modulus: &Bound<'_, PyAny>, + other: Borrowed<'_, 'py, PyAny>, + modulus: Borrowed<'_, 'py, PyAny>, ) -> PyResult> { unsafe { ffi::PyNumber_Power(any.as_ptr(), other.as_ptr(), modulus.as_ptr()) @@ -1200,12 +1200,12 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &other + other .into_pyobject(py) .map_err(Into::into)? .into_any() .as_borrowed(), - &modulus + modulus .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1314,7 +1314,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner<'py>( any: &Bound<'py, PyAny>, - key: &Bound<'_, PyAny>, + key: Borrowed<'_, 'py, PyAny>, ) -> PyResult> { unsafe { ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()).assume_owned_or_err(any.py()) @@ -1324,7 +1324,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &key.into_pyobject(py) + key.into_pyobject(py) .map_err(Into::into)? .into_any() .as_borrowed(), @@ -1338,8 +1338,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { fn inner( any: &Bound<'_, PyAny>, - key: &Bound<'_, PyAny>, - value: &Bound<'_, PyAny>, + key: Borrowed<'_, '_, PyAny>, + value: Borrowed<'_, '_, PyAny>, ) -> PyResult<()> { err::error_on_minusone(any.py(), unsafe { ffi::PyObject_SetItem(any.as_ptr(), key.as_ptr(), value.as_ptr()) @@ -1349,11 +1349,11 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &key.into_pyobject(py) + key.into_pyobject(py) .map_err(Into::into)? .into_any() .as_borrowed(), - &value + value .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1365,7 +1365,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { where K: IntoPyObject<'py>, { - fn inner(any: &Bound<'_, PyAny>, key: &Bound<'_, PyAny>) -> PyResult<()> { + fn inner(any: &Bound<'_, PyAny>, key: Borrowed<'_, '_, PyAny>) -> PyResult<()> { err::error_on_minusone(any.py(), unsafe { ffi::PyObject_DelItem(any.as_ptr(), key.as_ptr()) }) @@ -1374,7 +1374,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &key.into_pyobject(py) + key.into_pyobject(py) .map_err(Into::into)? .into_any() .as_borrowed(), @@ -1529,7 +1529,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { where V: IntoPyObject<'py>, { - fn inner(any: &Bound<'_, PyAny>, value: &Bound<'_, PyAny>) -> PyResult { + fn inner(any: &Bound<'_, PyAny>, value: Borrowed<'_, '_, PyAny>) -> PyResult { match unsafe { ffi::PySequence_Contains(any.as_ptr(), value.as_ptr()) } { 0 => Ok(false), 1 => Ok(true), @@ -1540,7 +1540,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { let py = self.py(); inner( self, - &value + value .into_pyobject(py) .map_err(Into::into)? .into_any() @@ -1593,11 +1593,7 @@ impl<'py> Bound<'py, PyAny> { let ret = descr_get(attr.as_ptr(), self.as_ptr(), self_type.as_ptr()); ret.assume_owned_or_err(py).map(Some) } - } else if let Ok(descr_get) = attr - .get_type() - .as_borrowed() - .getattr(crate::intern!(py, "__get__")) - { + } else if let Ok(descr_get) = attr.get_type().getattr(crate::intern!(py, "__get__")) { descr_get.call1((attr, self, self_type)).map(Some) } else { Ok(Some(attr)) @@ -1670,7 +1666,7 @@ class NonHeapNonDescriptorInt: let no_descriptor = module.getattr("NoDescriptorInt").unwrap().call0().unwrap(); assert_eq!(eval_int(no_descriptor).unwrap(), 1); let missing = module.getattr("NoInt").unwrap().call0().unwrap(); - assert!(missing.as_borrowed().lookup_special(int).unwrap().is_none()); + assert!(missing.lookup_special(int).unwrap().is_none()); // Note the instance override should _not_ call the instance method that returns 2, // because that's not how special lookups are meant to work. let instance_override = module.getattr("instance_override").unwrap(); @@ -1680,7 +1676,7 @@ class NonHeapNonDescriptorInt: .unwrap() .call0() .unwrap(); - assert!(descriptor_error.as_borrowed().lookup_special(int).is_err()); + assert!(descriptor_error.lookup_special(int).is_err()); let nonheap_nondescriptor = module .getattr("NonHeapNonDescriptorInt") .unwrap() diff --git a/src/types/dict.rs b/src/types/dict.rs index 6e29e6edcaa..1be0ed22362 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -1189,9 +1189,7 @@ mod tests { Python::with_gil(|py| { let dict = abc_dict(py); let keys = dict.call_method0("keys").unwrap(); - assert!(keys - .is_instance(&py.get_type::().as_borrowed()) - .unwrap()); + assert!(keys.is_instance(&py.get_type::()).unwrap()); }) } @@ -1201,9 +1199,7 @@ mod tests { Python::with_gil(|py| { let dict = abc_dict(py); let values = dict.call_method0("values").unwrap(); - assert!(values - .is_instance(&py.get_type::().as_borrowed()) - .unwrap()); + assert!(values.is_instance(&py.get_type::()).unwrap()); }) } @@ -1213,9 +1209,7 @@ mod tests { Python::with_gil(|py| { let dict = abc_dict(py); let items = dict.call_method0("items").unwrap(); - assert!(items - .is_instance(&py.get_type::().as_borrowed()) - .unwrap()); + assert!(items.is_instance(&py.get_type::()).unwrap()); }) } diff --git a/src/types/mapping.rs b/src/types/mapping.rs index 009c5c0e5e2..0d1467c27bf 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -177,7 +177,7 @@ impl PyTypeCheck for PyMapping { || get_mapping_abc(object.py()) .and_then(|abc| object.is_instance(abc)) .unwrap_or_else(|err| { - err.write_unraisable(object.py(), Some(&object.as_borrowed())); + err.write_unraisable(object.py(), Some(object)); false }) } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 4fa72f1219f..4dbfedc378b 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -367,7 +367,7 @@ impl PyTypeCheck for PySequence { || get_sequence_abc(object.py()) .and_then(|abc| object.is_instance(abc)) .unwrap_or_else(|err| { - err.write_unraisable(object.py(), Some(&object.as_borrowed())); + err.write_unraisable(object.py(), Some(object)); false }) } diff --git a/tests/test_coroutine.rs b/tests/test_coroutine.rs index bb6889f4c0d..cdf01b8891e 100644 --- a/tests/test_coroutine.rs +++ b/tests/test_coroutine.rs @@ -69,10 +69,7 @@ fn test_coroutine_qualname() { assert coro.__name__ == name and coro.__qualname__ == qualname "#; let locals = [ - ( - "my_fn", - wrap_pyfunction!(my_fn, gil).unwrap().as_borrowed().as_any(), - ), + ("my_fn", wrap_pyfunction!(my_fn, gil).unwrap().as_any()), ("MyClass", gil.get_type::().as_any()), ] .into_py_dict(gil);