Skip to content

Commit

Permalink
allow borrowed extracts with gil-refs disabled (#3959)
Browse files Browse the repository at this point in the history
  • Loading branch information
Icxolu committed Mar 15, 2024
1 parent 989d2c5 commit 5c86dc3
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 28 deletions.
10 changes: 6 additions & 4 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ 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, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python};
use crate::{
ffi, gil, Borrowed, Bound, Py, PyAny, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python,
};
use std::ptr::NonNull;

/// Returns a borrowed pointer to a Python object.
Expand Down Expand Up @@ -288,7 +290,7 @@ pub trait FromPyObjectBound<'a, 'py>: Sized + from_py_object_bound_sealed::Seale
///
/// Users are advised against calling this method directly: instead, use this via
/// [`Bound<'_, PyAny>::extract`] or [`Py::extract`].
fn from_py_object_bound(ob: &'a Bound<'py, PyAny>) -> PyResult<Self>;
fn from_py_object_bound(ob: Borrowed<'a, 'py, PyAny>) -> PyResult<Self>;

/// Extracts the type hint information for this type when it appears as an argument.
///
Expand All @@ -307,8 +309,8 @@ impl<'py, T> FromPyObjectBound<'_, 'py> for T
where
T: FromPyObject<'py>,
{
fn from_py_object_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
Self::extract_bound(ob)
fn from_py_object_bound(ob: Borrowed<'_, 'py, PyAny>) -> PyResult<Self> {
Self::extract_bound(&ob)
}

#[cfg(feature = "experimental-inspect")]
Expand Down
13 changes: 6 additions & 7 deletions src/conversions/std/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ use std::borrow::Cow;

#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
#[cfg(not(feature = "gil-refs"))]
use crate::types::PyBytesMethods;
use crate::{
types::{PyAnyMethods, PyByteArray, PyByteArrayMethods, PyBytes},
Bound, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
types::{PyByteArray, PyByteArrayMethods, PyBytes},
IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
};

impl<'a> IntoPy<PyObject> for &'a [u8] {
Expand Down Expand Up @@ -34,7 +32,7 @@ impl<'py> crate::FromPyObject<'py> for &'py [u8] {

#[cfg(not(feature = "gil-refs"))]
impl<'a> crate::conversion::FromPyObjectBound<'a, '_> for &'a [u8] {
fn from_py_object_bound(obj: &'a Bound<'_, PyAny>) -> PyResult<Self> {
fn from_py_object_bound(obj: crate::Borrowed<'a, '_, PyAny>) -> PyResult<Self> {
Ok(obj.downcast::<PyBytes>()?.as_bytes())
}

Expand All @@ -51,7 +49,8 @@ impl<'a> crate::conversion::FromPyObjectBound<'a, '_> for &'a [u8] {
/// If it is a `bytearray`, its contents will be copied to an owned `Cow`.
#[cfg(feature = "gil-refs")]
impl<'py> crate::FromPyObject<'py> for Cow<'py, [u8]> {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
fn extract_bound(ob: &crate::Bound<'py, PyAny>) -> PyResult<Self> {
use crate::types::PyAnyMethods;
if let Ok(bytes) = ob.downcast::<PyBytes>() {
return Ok(Cow::Borrowed(bytes.clone().into_gil_ref().as_bytes()));
}
Expand All @@ -63,7 +62,7 @@ impl<'py> crate::FromPyObject<'py> for Cow<'py, [u8]> {

#[cfg(not(feature = "gil-refs"))]
impl<'a> crate::conversion::FromPyObjectBound<'a, '_> for Cow<'a, [u8]> {
fn from_py_object_bound(ob: &'a Bound<'_, PyAny>) -> PyResult<Self> {
fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult<Self> {
if let Ok(bytes) = ob.downcast::<PyBytes>() {
return Ok(Cow::Borrowed(bytes.as_bytes()));
}
Expand Down
4 changes: 2 additions & 2 deletions src/conversions/std/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl<'py> FromPyObject<'py> for &'py str {

#[cfg(all(not(feature = "gil-refs"), any(Py_3_10, not(Py_LIMITED_API))))]
impl<'a> crate::conversion::FromPyObjectBound<'a, '_> for &'a str {
fn from_py_object_bound(ob: &'a Bound<'_, PyAny>) -> PyResult<Self> {
fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult<Self> {
ob.downcast::<PyString>()?.to_str()
}

Expand All @@ -152,7 +152,7 @@ impl<'py> FromPyObject<'py> for Cow<'py, str> {

#[cfg(not(feature = "gil-refs"))]
impl<'a> crate::conversion::FromPyObjectBound<'a, '_> for Cow<'a, str> {
fn from_py_object_bound(ob: &'a Bound<'_, PyAny>) -> PyResult<Self> {
fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult<Self> {
ob.downcast::<PyString>()?.to_cow()
}

Expand Down
23 changes: 16 additions & 7 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
exceptions::{self, PyBaseException},
ffi,
};
use crate::{IntoPy, Py, PyAny, PyNativeType, PyObject, Python, ToPyObject};
use crate::{Borrowed, IntoPy, Py, PyAny, PyNativeType, PyObject, Python, ToPyObject};
use std::borrow::Cow;
use std::cell::UnsafeCell;
use std::ffi::CString;
Expand Down Expand Up @@ -65,24 +65,33 @@ impl<'a> PyDowncastError<'a> {
/// Compatibility API to convert the Bound variant `DowncastError` into the
/// gil-ref variant
pub(crate) fn from_downcast_err(DowncastError { from, to }: DowncastError<'a, 'a>) -> Self {
Self {
from: from.as_gil_ref(),
to,
}
#[allow(deprecated)]
let from = unsafe { from.py().from_borrowed_ptr(from.as_ptr()) };
Self { from, to }
}
}

/// Error that indicates a failure to convert a PyAny to a more specific Python type.
#[derive(Debug)]
pub struct DowncastError<'a, 'py> {
from: &'a Bound<'py, PyAny>,
from: Borrowed<'a, 'py, PyAny>,
to: Cow<'static, str>,
}

impl<'a, 'py> DowncastError<'a, 'py> {
/// Create a new `PyDowncastError` representing a failure to convert the object
/// `from` into the type named in `to`.
pub fn new(from: &'a Bound<'py, PyAny>, to: impl Into<Cow<'static, str>>) -> Self {
DowncastError {
from: from.as_borrowed(),
to: to.into(),
}
}
#[cfg(not(feature = "gil-refs"))]
pub(crate) fn new_from_borrowed(
from: Borrowed<'a, 'py, PyAny>,
to: impl Into<Cow<'static, str>>,
) -> Self {
DowncastError {
from,
to: to.into(),
Expand Down Expand Up @@ -1036,7 +1045,7 @@ impl std::error::Error for DowncastError<'_, '_> {}

impl std::fmt::Display for DowncastError<'_, '_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
display_downcast_error(f, self.from, &self.to)
display_downcast_error(f, &self.from, &self.to)
}
}

Expand Down
14 changes: 14 additions & 0 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,20 @@ impl<'a, 'py> Borrowed<'a, 'py, PyAny> {
Self(NonNull::new_unchecked(ptr), PhantomData, py)
}

#[inline]
#[cfg(not(feature = "gil-refs"))]
pub(crate) fn downcast<T>(self) -> Result<Borrowed<'a, 'py, T>, DowncastError<'a, 'py>>
where
T: PyTypeCheck,
{
if T::type_check(&self) {
// Safety: type_check is responsible for ensuring that the type is correct
Ok(unsafe { self.downcast_unchecked() })
} else {
Err(DowncastError::new_from_borrowed(self, T::NAME))
}
}

/// Converts this `PyAny` to a concrete Python type without checking validity.
///
/// # Safety
Expand Down
14 changes: 8 additions & 6 deletions src/types/any.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::class::basic::CompareOp;
use crate::conversion::{AsPyPointer, FromPyObject, FromPyObjectBound, IntoPy, ToPyObject};
use crate::conversion::{AsPyPointer, FromPyObjectBound, IntoPy, ToPyObject};
use crate::err::{DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyResult};
use crate::exceptions::{PyAttributeError, PyTypeError};
use crate::ffi_ptr_ext::FfiPtrExt;
Expand Down Expand Up @@ -799,13 +799,14 @@ impl PyAny {

/// Extracts some type from the Python object.
///
/// This is a wrapper function around [`FromPyObject::extract()`].
/// This is a wrapper function around
/// [`FromPyObject::extract()`](crate::FromPyObject::extract).
#[inline]
pub fn extract<'py, D>(&'py self) -> PyResult<D>
where
D: FromPyObject<'py>,
D: FromPyObjectBound<'py, 'py>,
{
FromPyObject::extract(self)
FromPyObjectBound::from_py_object_bound(self.as_borrowed())
}

/// Returns the reference count for the Python object.
Expand Down Expand Up @@ -1641,7 +1642,8 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed {

/// Extracts some type from the Python object.
///
/// This is a wrapper function around [`FromPyObject::extract()`].
/// This is a wrapper function around
/// [`FromPyObject::extract()`](crate::FromPyObject::extract).
fn extract<'a, T>(&'a self) -> PyResult<T>
where
T: FromPyObjectBound<'a, 'py>;
Expand Down Expand Up @@ -2182,7 +2184,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
where
T: FromPyObjectBound<'a, 'py>,
{
FromPyObjectBound::from_py_object_bound(self)
FromPyObjectBound::from_py_object_bound(self.as_borrowed())
}

fn get_refcnt(&self) -> isize {
Expand Down
2 changes: 1 addition & 1 deletion src/types/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl<'py> PyBytesMethods<'py> for Bound<'py, PyBytes> {
impl<'a> Borrowed<'a, '_, PyBytes> {
/// Gets the Python string as a byte slice.
#[allow(clippy::wrong_self_convention)]
fn as_bytes(self) -> &'a [u8] {
pub(crate) fn as_bytes(self) -> &'a [u8] {
unsafe {
let buffer = ffi::PyBytes_AsString(self.as_ptr()) as *const u8;
let length = ffi::PyBytes_Size(self.as_ptr()) as usize;
Expand Down
2 changes: 1 addition & 1 deletion src/types/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl<'a> Borrowed<'a, '_, PyString> {
}

#[allow(clippy::wrong_self_convention)]
fn to_cow(self) -> PyResult<Cow<'a, str>> {
pub(crate) fn to_cow(self) -> PyResult<Cow<'a, str>> {
// TODO: this method can probably be deprecated once Python 3.9 support is dropped,
// because all versions then support the more efficient `to_str`.
#[cfg(any(Py_3_10, not(Py_LIMITED_API)))]
Expand Down

0 comments on commit 5c86dc3

Please sign in to comment.