Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Borrowed::as_ptr #4520

Merged
merged 1 commit into from
Sep 6, 2024
Merged

add Borrowed::as_ptr #4520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4520.added.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unrelated, but is there a particular reason why we didn't opt for NonNull<PyObject> for these pointer methods?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a historical thing from before I was involved with PyO3. Given that mostly these pointers are just immediately passed to FFI, maybe it was more convenient? We could add non-null variants for cases where these pointers get handled by Rust I guess...

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading