Skip to content

Commit

Permalink
add Borrowed::as_ptr (#4520)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt authored Sep 6, 2024
1 parent 699de42 commit e2a1da0
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 63 deletions.
1 change: 1 addition & 0 deletions newsfragments/4520.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `Borrowed::as_ptr`.
2 changes: 1 addition & 1 deletion src/conversions/chrono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
};
}

Expand Down
5 changes: 1 addition & 4 deletions src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
19 changes: 14 additions & 5 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
Expand Down
72 changes: 34 additions & 38 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -887,15 +887,15 @@ macro_rules! implement_binop {
{
fn inner<'py>(
any: &Bound<'py, PyAny>,
other: &Bound<'_, PyAny>,
other: Borrowed<'_, 'py, PyAny>,
) -> PyResult<Bound<'py, PyAny>> {
unsafe { ffi::$c_api(any.as_ptr(), other.as_ptr()).assume_owned_or_err(any.py()) }
}

let py = self.py();
inner(
self,
&other
other
.into_pyobject(py)
.map_err(Into::into)?
.into_any()
Expand Down Expand Up @@ -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<Bound<'py, PyAny>> {
unsafe {
ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr())
Expand All @@ -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(),
Expand All @@ -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())
Expand All @@ -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()
Expand All @@ -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())
})
Expand All @@ -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(),
Expand All @@ -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<Ordering> {
fn inner(any: &Bound<'_, PyAny>, other: Borrowed<'_, '_, PyAny>) -> PyResult<Ordering> {
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.
Expand All @@ -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()
Expand All @@ -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<Bound<'py, PyAny>> {
unsafe {
Expand All @@ -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()
Expand Down Expand Up @@ -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<Bound<'py, PyAny>> {
unsafe {
ffi::PyNumber_Divmod(any.as_ptr(), other.as_ptr()).assume_owned_or_err(any.py())
Expand All @@ -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()
Expand All @@ -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<Bound<'py, PyAny>> {
unsafe {
ffi::PyNumber_Power(any.as_ptr(), other.as_ptr(), modulus.as_ptr())
Expand All @@ -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()
Expand Down Expand Up @@ -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<Bound<'py, PyAny>> {
unsafe {
ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()).assume_owned_or_err(any.py())
Expand All @@ -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(),
Expand All @@ -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())
Expand All @@ -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()
Expand All @@ -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())
})
Expand All @@ -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(),
Expand Down Expand Up @@ -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<bool> {
fn inner(any: &Bound<'_, PyAny>, value: Borrowed<'_, '_, PyAny>) -> PyResult<bool> {
match unsafe { ffi::PySequence_Contains(any.as_ptr(), value.as_ptr()) } {
0 => Ok(false),
1 => Ok(true),
Expand All @@ -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()
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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();
Expand All @@ -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()
Expand Down
12 changes: 3 additions & 9 deletions src/types/dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<PyDictKeys>().as_borrowed())
.unwrap());
assert!(keys.is_instance(&py.get_type::<PyDictKeys>()).unwrap());
})
}

Expand All @@ -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::<PyDictValues>().as_borrowed())
.unwrap());
assert!(values.is_instance(&py.get_type::<PyDictValues>()).unwrap());
})
}

Expand All @@ -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::<PyDictItems>().as_borrowed())
.unwrap());
assert!(items.is_instance(&py.get_type::<PyDictItems>()).unwrap());
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/types/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down
5 changes: 1 addition & 4 deletions tests/test_coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<MyClass>().as_any()),
]
.into_py_dict(gil);
Expand Down

0 comments on commit e2a1da0

Please sign in to comment.