Skip to content

Commit 25789e8

Browse files
davidhewittdavidhewitt
committed
introduce traits to make ffi ptr handling cleaner
1 parent 4229c18 commit 25789e8

File tree

7 files changed

+111
-72
lines changed

7 files changed

+111
-72
lines changed

‎src/ffi_ptr_ext.rs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
use crate::{ffi, instance::Py2, PyAny, PyResult, Python};
2+
3+
mod ffi_ptr_ext_sealed {
4+
use super::*;
5+
6+
pub trait Sealed {}
7+
8+
impl Sealed for *mut ffi::PyObject {}
9+
}
10+
11+
use ffi_ptr_ext_sealed::Sealed;
12+
13+
pub(crate) trait FfiPtrExt: Sealed {
14+
unsafe fn assume_owned_or_ffi_error(self, py: Python<'_>) -> PyResult<Py2<'_, PyAny>>;
15+
unsafe fn assume_owned_and_infallible(self, py: Python<'_>) -> Py2<'_, PyAny>;
16+
}
17+
18+
impl FfiPtrExt for *mut ffi::PyObject {
19+
#[inline]
20+
unsafe fn assume_owned_or_ffi_error(self, py: Python<'_>) -> PyResult<Py2<'_, PyAny>> {
21+
Py2::from_owned_ptr_or_err(py, self)
22+
}
23+
24+
#[inline]
25+
unsafe fn assume_owned_and_infallible(self, py: Python<'_>) -> Py2<'_, PyAny> {
26+
Py2::from_owned_ptr(py, self)
27+
}
28+
}

‎src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,8 @@ pub use crate::version::PythonVersionInfo;
312312

313313
// Expected to become public API in 0.21 under a different name
314314
pub(crate) use crate::instance::Py2;
315+
pub(crate) mod ffi_ptr_ext;
316+
pub(crate) mod py_result_ext;
315317

316318
/// Old module which contained some implementation details of the `#[pyproto]` module.
317319
///

‎src/prelude.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ pub use pyo3_macros::{pyclass, pyfunction, pymethods, pymodule, FromPyObject};
2323
pub use crate::wrap_pyfunction;
2424

2525
// Expected to become public API in 0.21
26-
pub(crate) use crate::instance::Py2; // Will be stabilized with a different name
27-
pub(crate) use crate::types::any::PyAnyMethods;
26+
// pub(crate) use crate::instance::Py2; // Will be stabilized with a different name
27+
// pub(crate) use crate::types::any::PyAnyMethods;
2828
// pub(crate) use crate::types::sequence::PySequenceMethods;

‎src/py_result_ext.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
use crate::{types::any::PyAnyMethods, Py2, PyAny, PyResult};
2+
3+
mod py_result_ext_sealed {
4+
use super::*;
5+
6+
pub trait Sealed {}
7+
8+
impl Sealed for PyResult<Py2<'_, PyAny>> {}
9+
}
10+
11+
use py_result_ext_sealed::Sealed;
12+
13+
pub(crate) trait PyResultExt<'py>: Sealed {
14+
unsafe fn downcast_into_unchecked<T>(self) -> PyResult<Py2<'py, T>>;
15+
}
16+
17+
impl<'py> PyResultExt<'py> for PyResult<Py2<'py, PyAny>> {
18+
#[inline]
19+
unsafe fn downcast_into_unchecked<T>(self) -> PyResult<Py2<'py, T>> {
20+
self.map(|instance| instance.downcast_into_unchecked())
21+
}
22+
}

‎src/types/any.rs‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::class::basic::CompareOp;
22
use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, PyTryFrom, ToPyObject};
33
use crate::err::{PyDowncastError, PyErr, PyResult};
44
use crate::exceptions::{PyAttributeError, PyTypeError};
5+
use crate::ffi_ptr_ext::FfiPtrExt;
56
use crate::instance::Py2;
67
use crate::type_object::PyTypeInfo;
78
#[cfg(not(PyPy))]
@@ -12,6 +13,8 @@ use std::cell::UnsafeCell;
1213
use std::cmp::Ordering;
1314
use std::os::raw::c_int;
1415

16+
use crate::py_result_ext::PyResultExt;
17+
1518
/// Represents any Python object.
1619
///
1720
/// It currently only appears as a *reference*, `&PyAny`,
@@ -1708,10 +1711,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
17081711
attr_name: Py2<'_, PyString>,
17091712
) -> PyResult<Py2<'py, PyAny>> {
17101713
unsafe {
1711-
Py2::from_owned_ptr_or_err(
1712-
any.py(),
1713-
ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr()),
1714-
)
1714+
ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr())
1715+
.assume_owned_or_ffi_error(any.py())
17151716
}
17161717
}
17171718

@@ -1761,12 +1762,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
17611762
O: ToPyObject,
17621763
{
17631764
fn inner(any: &Py2<'_, PyAny>, other: Py2<'_, PyAny>) -> PyResult<Ordering> {
1764-
let py = any.py();
17651765
let other = other.as_ptr();
17661766
// Almost the same as ffi::PyObject_RichCompareBool, but this one doesn't try self == other.
17671767
// See https://github.com/PyO3/pyo3/issues/985 for more.
17681768
let do_compare = |other, op| unsafe {
1769-
Py2::from_owned_ptr_or_err(py, ffi::PyObject_RichCompare(any.as_ptr(), other, op))
1769+
ffi::PyObject_RichCompare(any.as_ptr(), other, op)
1770+
.assume_owned_or_ffi_error(any.py())
17701771
.and_then(|obj| obj.is_true())
17711772
};
17721773
if do_compare(other, ffi::Py_EQ)? {
@@ -1796,10 +1797,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
17961797
compare_op: CompareOp,
17971798
) -> PyResult<Py2<'py, PyAny>> {
17981799
unsafe {
1799-
Py2::from_owned_ptr_or_err(
1800-
any.py(),
1801-
ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int),
1802-
)
1800+
ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int)
1801+
.assume_owned_or_ffi_error(any.py())
18031802
}
18041803
}
18051804

@@ -1870,14 +1869,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
18701869
kwargs: Option<&PyDict>,
18711870
) -> PyResult<Py2<'py, PyAny>> {
18721871
unsafe {
1873-
Py2::from_owned_ptr_or_err(
1874-
any.py(),
1875-
ffi::PyObject_Call(
1876-
any.as_ptr(),
1877-
args.as_ptr(),
1878-
kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
1879-
),
1872+
ffi::PyObject_Call(
1873+
any.as_ptr(),
1874+
args.as_ptr(),
1875+
kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
18801876
)
1877+
.assume_owned_or_ffi_error(any.py())
18811878
}
18821879
}
18831880

@@ -1893,7 +1890,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
18931890
))] {
18941891
// Optimized path on python 3.9+
18951892
unsafe {
1896-
Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_CallNoArgs(self.as_ptr()))
1893+
ffi::PyObject_CallNoArgs(self.as_ptr()).assume_owned_or_ffi_error(self.py())
18971894
}
18981895
} else {
18991896
self.call((), None)
@@ -1930,7 +1927,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
19301927
// Optimized path on python 3.9+
19311928
unsafe {
19321929
let name = name.into_py(py).attach_into(py);
1933-
Py2::from_owned_ptr_or_err(py, ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr()))
1930+
ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr()).assume_owned_or_ffi_error(py)
19341931
}
19351932
} else {
19361933
self.call_method(name, (), None)
@@ -1971,10 +1968,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
19711968
{
19721969
fn inner<'py>(any: &Py2<'py, PyAny>, key: Py2<'_, PyAny>) -> PyResult<Py2<'py, PyAny>> {
19731970
unsafe {
1974-
Py2::from_owned_ptr_or_err(
1975-
any.py(),
1976-
ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()),
1977-
)
1971+
ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr())
1972+
.assume_owned_or_ffi_error(any.py())
19781973
}
19791974
}
19801975

@@ -2103,15 +2098,17 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
21032098

21042099
fn repr(&self) -> PyResult<Py2<'py, PyString>> {
21052100
unsafe {
2106-
Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_Repr(self.as_ptr()))
2107-
.map(|any| any.downcast_into_unchecked())
2101+
ffi::PyObject_Repr(self.as_ptr())
2102+
.assume_owned_or_ffi_error(self.py())
2103+
.downcast_into_unchecked()
21082104
}
21092105
}
21102106

21112107
fn str(&self) -> PyResult<Py2<'py, PyString>> {
21122108
unsafe {
2113-
Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_Str(self.as_ptr()))
2114-
.map(|any| any.downcast_into_unchecked())
2109+
ffi::PyObject_Str(self.as_ptr())
2110+
.assume_owned_or_ffi_error(self.py())
2111+
.downcast_into_unchecked()
21152112
}
21162113
}
21172114

@@ -2129,7 +2126,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
21292126

21302127
fn dir(&self) -> Py2<'py, PyList> {
21312128
unsafe {
2132-
Py2::from_owned_ptr(self.py(), ffi::PyObject_Dir(self.as_ptr()))
2129+
ffi::PyObject_Dir(self.as_ptr())
2130+
.assume_owned_and_infallible(self.py())
21332131
.downcast_into_unchecked()
21342132
}
21352133
}

‎src/types/iterator.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1+
use crate::ffi_ptr_ext::FfiPtrExt;
2+
use crate::py_result_ext::PyResultExt;
13
use crate::{ffi, AsPyPointer, Py, Py2, PyAny, PyErr, PyNativeType, PyResult, Python};
24
use crate::{PyDowncastError, PyTryFrom};
35

4-
use super::any::PyAnyMethods;
5-
66
/// A Python iterator object.
77
///
88
/// # Examples
@@ -42,8 +42,9 @@ impl PyIterator {
4242

4343
pub(crate) fn from_object2<'py>(obj: &Py2<'py, PyAny>) -> PyResult<Py2<'py, PyIterator>> {
4444
unsafe {
45-
Py2::from_owned_ptr_or_err(obj.py(), ffi::PyObject_GetIter(obj.as_ptr()))
46-
.map(|any| any.downcast_into_unchecked())
45+
ffi::PyObject_GetIter(obj.as_ptr())
46+
.assume_owned_or_ffi_error(obj.py())
47+
.downcast_into_unchecked()
4748
}
4849
}
4950
}

‎src/types/sequence.rs‎

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
use crate::err::{self, PyDowncastError, PyErr, PyResult};
22
use crate::exceptions::PyTypeError;
3+
use crate::ffi_ptr_ext::FfiPtrExt;
34
#[cfg(feature = "experimental-inspect")]
45
use crate::inspect::types::TypeInfo;
6+
use crate::instance::Py2;
57
use crate::internal_tricks::get_ssize_index;
6-
use crate::prelude::*;
8+
use crate::py_result_ext::PyResultExt;
79
use crate::sync::GILOnceCell;
810
use crate::type_object::PyTypeInfo;
911
use crate::types::{PyAny, PyList, PyString, PyTuple, PyType};
@@ -327,69 +329,53 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> {
327329
#[inline]
328330
fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
329331
unsafe {
330-
Py2::from_owned_ptr_or_err(
331-
self.py(),
332-
ffi::PySequence_Concat(self.as_ptr(), other.as_ptr()),
333-
)
334-
.map(|py2| py2.downcast_into_unchecked())
332+
ffi::PySequence_Concat(self.as_ptr(), other.as_ptr())
333+
.assume_owned_or_ffi_error(self.py())
334+
.downcast_into_unchecked()
335335
}
336336
}
337337

338338
#[inline]
339339
fn repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
340340
unsafe {
341-
Py2::from_owned_ptr_or_err(
342-
self.py(),
343-
ffi::PySequence_Repeat(self.as_ptr(), get_ssize_index(count)),
344-
)
345-
.map(|py2| py2.downcast_into_unchecked())
341+
ffi::PySequence_Repeat(self.as_ptr(), get_ssize_index(count))
342+
.assume_owned_or_ffi_error(self.py())
343+
.downcast_into_unchecked()
346344
}
347345
}
348346

349347
#[inline]
350348
fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
351349
unsafe {
352-
Py2::from_owned_ptr_or_err(
353-
self.py(),
354-
ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr()),
355-
)
356-
.map(|py2| py2.downcast_into_unchecked())
350+
ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr())
351+
.assume_owned_or_ffi_error(self.py())
352+
.downcast_into_unchecked()
357353
}
358354
}
359355

360356
#[inline]
361357
fn in_place_repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
362358
unsafe {
363-
Py2::from_owned_ptr_or_err(
364-
self.py(),
365-
ffi::PySequence_InPlaceRepeat(self.as_ptr(), get_ssize_index(count)),
366-
)
367-
.map(|py2| py2.downcast_into_unchecked())
359+
ffi::PySequence_InPlaceRepeat(self.as_ptr(), get_ssize_index(count))
360+
.assume_owned_or_ffi_error(self.py())
361+
.downcast_into_unchecked()
368362
}
369363
}
370364

371365
#[inline]
372366
fn get_item(&self, index: usize) -> PyResult<Py2<'py, PyAny>> {
373367
unsafe {
374-
Py2::from_owned_ptr_or_err(
375-
self.py(),
376-
ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index)),
377-
)
368+
ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index))
369+
.assume_owned_or_ffi_error(self.py())
378370
}
379371
}
380372

381373
#[inline]
382374
fn get_slice(&self, begin: usize, end: usize) -> PyResult<Py2<'py, PySequence>> {
383375
unsafe {
384-
Py2::from_owned_ptr_or_err(
385-
self.py(),
386-
ffi::PySequence_GetSlice(
387-
self.as_ptr(),
388-
get_ssize_index(begin),
389-
get_ssize_index(end),
390-
),
391-
)
392-
.map(|py2| py2.downcast_into_unchecked())
376+
ffi::PySequence_GetSlice(self.as_ptr(), get_ssize_index(begin), get_ssize_index(end))
377+
.assume_owned_or_ffi_error(self.py())
378+
.downcast_into_unchecked()
393379
}
394380
}
395381

@@ -486,16 +472,18 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> {
486472
#[inline]
487473
fn to_list(&self) -> PyResult<Py2<'py, PyList>> {
488474
unsafe {
489-
Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_List(self.as_ptr()))
490-
.map(|py2| py2.downcast_into_unchecked())
475+
ffi::PySequence_List(self.as_ptr())
476+
.assume_owned_or_ffi_error(self.py())
477+
.downcast_into_unchecked()
491478
}
492479
}
493480

494481
#[inline]
495482
fn to_tuple(&self) -> PyResult<Py2<'py, PyTuple>> {
496483
unsafe {
497-
Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_Tuple(self.as_ptr()))
498-
.map(|py2| py2.downcast_into_unchecked())
484+
ffi::PySequence_Tuple(self.as_ptr())
485+
.assume_owned_or_ffi_error(self.py())
486+
.downcast_into_unchecked()
499487
}
500488
}
501489
}

0 commit comments

Comments
 (0)