From 5c86dc35c1108591e553ff3829bca88678d3ccf8 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 15 Mar 2024 08:53:52 +0100 Subject: [PATCH] allow borrowed extracts with `gil-refs` disabled (#3959) --- src/conversion.rs | 10 ++++++---- src/conversions/std/slice.rs | 13 ++++++------- src/conversions/std/string.rs | 4 ++-- src/err/mod.rs | 23 ++++++++++++++++------- src/instance.rs | 14 ++++++++++++++ src/types/any.rs | 14 ++++++++------ src/types/bytes.rs | 2 +- src/types/string.rs | 2 +- 8 files changed, 54 insertions(+), 28 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index 8d4ad776..dfa53eac 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -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. @@ -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; + fn from_py_object_bound(ob: Borrowed<'a, 'py, PyAny>) -> PyResult; /// Extracts the type hint information for this type when it appears as an argument. /// @@ -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::extract_bound(ob) + fn from_py_object_bound(ob: Borrowed<'_, 'py, PyAny>) -> PyResult { + Self::extract_bound(&ob) } #[cfg(feature = "experimental-inspect")] diff --git a/src/conversions/std/slice.rs b/src/conversions/std/slice.rs index 18bb52cd..b3932302 100644 --- a/src/conversions/std/slice.rs +++ b/src/conversions/std/slice.rs @@ -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 for &'a [u8] { @@ -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 { + fn from_py_object_bound(obj: crate::Borrowed<'a, '_, PyAny>) -> PyResult { Ok(obj.downcast::()?.as_bytes()) } @@ -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 { + fn extract_bound(ob: &crate::Bound<'py, PyAny>) -> PyResult { + use crate::types::PyAnyMethods; if let Ok(bytes) = ob.downcast::() { return Ok(Cow::Borrowed(bytes.clone().into_gil_ref().as_bytes())); } @@ -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 { + fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult { if let Ok(bytes) = ob.downcast::() { return Ok(Cow::Borrowed(bytes.as_bytes())); } diff --git a/src/conversions/std/string.rs b/src/conversions/std/string.rs index d013890a..9c276d1d 100644 --- a/src/conversions/std/string.rs +++ b/src/conversions/std/string.rs @@ -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 { + fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult { ob.downcast::()?.to_str() } @@ -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 { + fn from_py_object_bound(ob: crate::Borrowed<'a, '_, PyAny>) -> PyResult { ob.downcast::()?.to_cow() } diff --git a/src/err/mod.rs b/src/err/mod.rs index 7610f499..de1e621f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -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; @@ -65,17 +65,16 @@ 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>, } @@ -83,6 +82,16 @@ 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>) -> 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>, + ) -> Self { DowncastError { from, to: to.into(), @@ -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) } } diff --git a/src/instance.rs b/src/instance.rs index 069bea69..83775708 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -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(self) -> Result, 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 diff --git a/src/types/any.rs b/src/types/any.rs index e4dbaf80..19855abb 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -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; @@ -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 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. @@ -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 where T: FromPyObjectBound<'a, 'py>; @@ -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 { diff --git a/src/types/bytes.rs b/src/types/bytes.rs index 55c295c4..44c87560 100644 --- a/src/types/bytes.rs +++ b/src/types/bytes.rs @@ -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; diff --git a/src/types/string.rs b/src/types/string.rs index 93f3682c..81c41adb 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -369,7 +369,7 @@ impl<'a> Borrowed<'a, '_, PyString> { } #[allow(clippy::wrong_self_convention)] - fn to_cow(self) -> PyResult> { + pub(crate) fn to_cow(self) -> PyResult> { // 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)))]