Skip to content

Commit

Permalink
finish migrations to FromPyObject::extract_bound
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Feb 12, 2024
1 parent 16225f8 commit ec7598f
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 32 deletions.
14 changes: 8 additions & 6 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
use crate::err::{self, PyDowncastError, PyResult};
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::instance::Bound;
use crate::pyclass::boolean_struct::False;
use crate::type_object::PyTypeInfo;
use crate::types::any::PyAnyMethods;
use crate::types::PyTuple;
use crate::{
ffi, gil, Bound, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python,
ffi, gil, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python,
};
use std::cell::Cell;
use std::ptr::NonNull;
Expand Down Expand Up @@ -309,8 +311,8 @@ impl<T: Copy + IntoPy<PyObject>> IntoPy<PyObject> for Cell<T> {
}

impl<'py, T: FromPyObject<'py>> FromPyObject<'py> for Cell<T> {
fn extract(ob: &'py PyAny) -> PyResult<Self> {
T::extract(ob).map(Cell::new)
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
obj.extract().map(Cell::new)
}
}

Expand Down Expand Up @@ -357,11 +359,11 @@ impl<'py, T> FromPyObject<'py> for Option<T>
where
T: FromPyObject<'py>,
{
fn extract(obj: &'py PyAny) -> PyResult<Self> {
if obj.as_ptr() == unsafe { ffi::Py_None() } {
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
if obj.is_none() {
Ok(None)
} else {
T::extract(obj).map(Some)
obj.extract().map(Some)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/conversions/num_complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ macro_rules! complex_conversion {
unsafe {
let complex;
let obj = if obj.is_instance_of::<PyComplex>() {
obj
obj.clone()
} else if let Some(method) =
obj.lookup_special(crate::intern!(obj.py(), "__complex__"))?
{
Expand All @@ -161,7 +161,7 @@ macro_rules! complex_conversion {
// `obj` might still implement `__float__` or `__index__`, which will be
// handled by `PyComplex_{Real,Imag}AsDouble`, including propagating any
// errors if those methods don't exist / raise exceptions.
obj
obj.clone()
};
let ptr = obj.as_ptr();
let real = ffi::PyComplex_RealAsDouble(ptr);
Expand Down
14 changes: 13 additions & 1 deletion src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ impl<'py> Bound<'py, PyAny> {
) -> PyResult<Self> {
Py::from_owned_ptr_or_err(py, ptr).map(|obj| Self(py, ManuallyDrop::new(obj)))
}

/// Constructs a new Bound from a borrowed pointer, incrementing the reference count.
/// Returns None if ptr is null.
///
/// # Safety
/// ptr must be a valid pointer to a Python object, or NULL.
pub unsafe fn from_borrowed_ptr_or_opt(
py: Python<'py>,
ptr: *mut ffi::PyObject,
) -> Option<Self> {
Py::from_borrowed_ptr_or_opt(py, ptr).map(|obj| Self(py, ManuallyDrop::new(obj)))
}
}

impl<'py, T> Bound<'py, T>
Expand Down Expand Up @@ -1194,7 +1206,7 @@ impl<T> Py<T> {
where
D: FromPyObject<'py>,
{
FromPyObject::extract(unsafe { py.from_borrowed_ptr(self.as_ptr()) })
self.bind(py).as_any().extract()
}

/// Retrieves an attribute value.
Expand Down
38 changes: 28 additions & 10 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl<T: PyClass> PyCell<T> {
/// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
/// [`try_borrow`](#method.try_borrow).
pub fn borrow(&self) -> PyRef<'_, T> {
self.try_borrow().expect("Already mutably borrowed")
PyRef::borrow(&self.as_borrowed())
}

/// Mutably borrows the value `T`. This borrow lasts as long as the returned `PyRefMut` exists.
Expand All @@ -317,7 +317,7 @@ impl<T: PyClass> PyCell<T> {
where
T: PyClass<Frozen = False>,
{
self.try_borrow_mut().expect("Already borrowed")
PyRefMut::borrow_mut(&self.as_borrowed())
}

/// Immutably borrows the value `T`, returning an error if the value is currently
Expand Down Expand Up @@ -348,10 +348,7 @@ impl<T: PyClass> PyCell<T> {
/// });
/// ```
pub fn try_borrow(&self) -> Result<PyRef<'_, T>, PyBorrowError> {
self.ensure_threadsafe();
self.borrow_checker().try_borrow().map(|_| PyRef {
inner: self.as_borrowed().to_owned(),
})
PyRef::try_borrow(&self.as_borrowed())
}

/// Variant of [`try_borrow`][Self::try_borrow] which fails instead of panicking if called from the wrong thread
Expand Down Expand Up @@ -387,10 +384,7 @@ impl<T: PyClass> PyCell<T> {
where
T: PyClass<Frozen = False>,
{
self.ensure_threadsafe();
self.borrow_checker().try_borrow_mut().map(|_| PyRefMut {
inner: self.as_borrowed().to_owned(),
})
PyRefMut::try_borrow_mut(&self.as_borrowed())
}

/// Immutably borrows the value `T`, returning an error if the value is
Expand Down Expand Up @@ -694,6 +688,18 @@ impl<'p, T: PyClass> PyRef<'p, T> {
pub fn into_ptr(self) -> *mut ffi::PyObject {
self.inner.clone().into_ptr()
}

pub(crate) fn borrow(obj: &Bound<'p, T>) -> Self {
Self::try_borrow(obj).expect("Already mutably borrowed")
}

pub(crate) fn try_borrow(obj: &Bound<'p, T>) -> Result<Self, PyBorrowError> {
obj.as_gil_ref().ensure_threadsafe();
obj.as_gil_ref()
.borrow_checker()
.try_borrow()
.map(|_| Self { inner: obj.clone() })
}
}

impl<'p, T, U> PyRef<'p, T>
Expand Down Expand Up @@ -863,6 +869,18 @@ impl<'p, T: PyClass<Frozen = False>> PyRefMut<'p, T> {
pub fn into_ptr(self) -> *mut ffi::PyObject {
self.inner.clone().into_ptr()
}

pub(crate) fn borrow_mut(obj: &Bound<'p, T>) -> Self {
Self::try_borrow_mut(obj).expect("Already borrowed")
}

pub(crate) fn try_borrow_mut(obj: &Bound<'p, T>) -> Result<Self, PyBorrowMutError> {
obj.as_gil_ref().ensure_threadsafe();
obj.as_gil_ref()
.borrow_checker()
.try_borrow_mut()
.map(|_| Self { inner: obj.clone() })
}
}

impl<'p, T, U> PyRefMut<'p, T>
Expand Down
8 changes: 2 additions & 6 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2286,7 +2286,7 @@ impl<'py> Bound<'py, PyAny> {
N: IntoPy<Py<PyString>>,
{
let py = self.py();
let self_type = self.get_type().as_borrowed();
let self_type = self.get_type();
let attr = if let Ok(attr) = self_type.getattr(attr_name) {
attr
} else {
Expand All @@ -2308,11 +2308,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
4 changes: 2 additions & 2 deletions src/types/boolobject.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::types::any::PyAnyMethods;
use crate::types::typeobject::PyTypeMethods;
use crate::{
exceptions::PyTypeError, ffi, ffi_ptr_ext::FfiPtrExt, instance::Bound, Borrowed, FromPyObject,
IntoPy, PyAny, PyNativeType, PyObject, PyResult, Python, ToPyObject,
};

use super::any::PyAnyMethods;

/// Represents a Python `bool`.
#[repr(transparent)]
pub struct PyBool(PyAny);
Expand Down
2 changes: 1 addition & 1 deletion src/types/typeobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl PyType {
p.cast::<ffi::PyObject>()
.assume_borrowed(py)
.downcast_unchecked()
.clone()
.to_owned()
}

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl Mapping {
if let Some(pylist) = elements {
let mut elems = HashMap::with_capacity(pylist.len());
for (i, pyelem) in pylist.into_iter().enumerate() {
let elem = String::extract(pyelem)?;
let elem = pyelem.extract()?;
elems.insert(elem, i);
}
Ok(Self { index: elems })
Expand Down
2 changes: 1 addition & 1 deletion tests/test_pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ struct ValueClass {
fn conversion_error(
str_arg: &str,
int_arg: i64,
tuple_arg: (&str, f64),
tuple_arg: (String, f64),
option_arg: Option<i64>,
struct_arg: Option<ValueClass>,
) {
Expand Down
4 changes: 2 additions & 2 deletions tests/test_sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl ByteSequence {
if let Some(pylist) = elements {
let mut elems = Vec::with_capacity(pylist.len());
for pyelem in pylist {
let elem = u8::extract(pyelem)?;
let elem = pyelem.extract()?;
elems.push(elem);
}
Ok(Self { elements: elems })
Expand Down Expand Up @@ -61,7 +61,7 @@ impl ByteSequence {
}

fn __contains__(&self, other: &PyAny) -> bool {
match u8::extract(other) {
match other.extract::<u8>() {
Ok(x) => self.elements.contains(&x),
Err(_) => false,
}
Expand Down

0 comments on commit ec7598f

Please sign in to comment.