try to resolve &str FromPyObject question

guide/src/migration.md

@@ -272,6 +272,13 @@ impl<'py> FromPyObject<'py> for MyType {
 
 The expectation is that in 0.22 `extract_bound` will have the default implementation removed and in 0.23 `extract` will be removed.
 
+##### `FromPyObject` for `&str`
+
+A particularly sticky point is `FromPyObject` for `&str`. The resulting `&str` slice needs to be borrowed from the underlying Python data.
+
+Due to technical limitations it is not possible to implement this for `abi3` when building for Python versions older than 3.10.
+
+This may lead to some compilation failures. It's recommended to use `Cow<'_, str>` or `String` in these cases.
 
 
 ## from 0.19.* to 0.20

src/conversion.rs

@@ -237,6 +237,45 @@ pub trait FromPyObject<'py>: Sized {
     }
 }
 
+/// Expected form of [`FromPyObject`] to be used in a future PyO3 release.
+///
+/// The difference between this and `FromPyObject` is that this trait takes an
+/// additional lifetime `'a`, which is the lifetime of the input `Bound`.
+///
+/// This allows implementations for `&'a str` and `&'a [u8]`, which could not
+/// be expressed by the existing `FromPyObject` trait once the GIL Refs API was
+/// removed.
+pub trait FromPyObjectBound<'a, 'py>: Sized {
+    /// Extracts `Self` from the bound smart pointer `obj`.
+    fn from_py_object_bound(ob: &'a Bound<'py, PyAny>) -> PyResult<Self>;
+
+    /// Extracts the type hint information for this type when it appears as an argument.
+    ///
+    /// For example, `Vec<u32>` would return `Sequence[int]`.
+    /// The default implementation returns `Any`, which is correct for any type.
+    ///
+    /// For most types, the return value for this method will be identical to that of [`IntoPy::type_output`].
+    /// It may be different for some types, such as `Dict`, to allow duck-typing: functions return `Dict` but take `Mapping` as argument.
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        TypeInfo::Any
+    }
+}
+
+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)
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        <T as FromPyObject>::type_input()
+    }
+}
+
 /// Identity conversion: allows using existing `PyObject` instances where
 /// `T: ToPyObject` is expected.
 impl<T: ?Sized + ToPyObject> ToPyObject for &'_ T {

src/conversions/std/slice.rs

@@ -2,9 +2,11 @@ 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::{PyByteArray, PyBytes},
-    FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
+    types::{PyAnyMethods, PyByteArray, PyByteArrayMethods, PyBytes},
+    Bound, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
 };
 
 impl<'a> IntoPy<PyObject> for &'a [u8] {
@@ -18,7 +20,8 @@ impl<'a> IntoPy<PyObject> for &'a [u8] {
     }
 }
 
-impl<'py> FromPyObject<'py> for &'py [u8] {
+#[cfg(feature = "gil-refs")]
+impl<'py> crate::FromPyObject<'py> for &'py [u8] {
     fn extract(obj: &'py PyAny) -> PyResult<Self> {
         Ok(obj.downcast::<PyBytes>()?.as_bytes())
     }
@@ -29,20 +32,50 @@ impl<'py> 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> {
+        Ok(obj.downcast::<PyBytes>()?.as_bytes())
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        Self::type_output()
+    }
+}
+
 /// Special-purpose trait impl to efficiently handle both `bytes` and `bytearray`
 ///
 /// If the source object is a `bytes` object, the `Cow` will be borrowed and
 /// pointing into the source object, and no copying or heap allocations will happen.
 /// If it is a `bytearray`, its contents will be copied to an owned `Cow`.
-impl<'py> FromPyObject<'py> for Cow<'py, [u8]> {
-    fn extract(ob: &'py PyAny) -> PyResult<Self> {
+#[cfg(feature = "gil-refs")]
+impl<'py> crate::FromPyObject<'py> for Cow<'py, [u8]> {
+    fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
+        if let Ok(bytes) = ob.downcast::<PyBytes>() {
+            return Ok(Cow::Borrowed(bytes.clone().into_gil_ref().as_bytes()));
+        }
+
+        let byte_array = ob.downcast::<PyByteArray>()?;
+        Ok(Cow::Owned(byte_array.to_vec()))
+    }
+}
+
+#[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> {
         if let Ok(bytes) = ob.downcast::<PyBytes>() {
             return Ok(Cow::Borrowed(bytes.as_bytes()));
         }
 
         let byte_array = ob.downcast::<PyByteArray>()?;
         Ok(Cow::Owned(byte_array.to_vec()))
     }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        Self::type_output()
+    }
 }
 
 impl ToPyObject for Cow<'_, [u8]> {

src/conversions/std/string.rs

@@ -113,7 +113,8 @@ impl<'a> IntoPy<PyObject> for &'a String {
 }
 
 /// Allows extracting strings from Python objects.
-/// Accepts Python `str` and `unicode` objects.
+/// Accepts Python `str` objects.
+#[cfg(feature = "gil-refs")]
 impl<'py> FromPyObject<'py> for &'py str {
     fn extract(ob: &'py PyAny) -> PyResult<Self> {
         ob.downcast::<PyString>()?.to_str()
@@ -125,6 +126,42 @@ 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> {
+        ob.downcast::<PyString>()?.to_str()
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        Self::type_output()
+    }
+}
+
+#[cfg(feature = "gil-refs")]
+impl<'py> FromPyObject<'py> for Cow<'py, str> {
+    fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
+        ob.extract().map(Cow::Owned)
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        <String as crate::FromPyObject>::type_input()
+    }
+}
+
+#[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> {
+        ob.downcast::<PyString>()?.to_cow()
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        <String as crate::FromPyObject>::type_input()
+    }
+}
+
 /// Allows extracting strings from Python objects.
 /// Accepts Python `str` and `unicode` objects.
 impl FromPyObject<'_> for String {
@@ -169,9 +206,9 @@ mod tests {
         Python::with_gil(|py| {
             let s = "Hello Python";
             let py_string: PyObject = Cow::Borrowed(s).into_py(py);
-            assert_eq!(s, py_string.extract::<&str>(py).unwrap());
+            assert_eq!(s, py_string.extract::<Cow<'_, str>>(py).unwrap());
             let py_string: PyObject = Cow::<str>::Owned(s.into()).into_py(py);
-            assert_eq!(s, py_string.extract::<&str>(py).unwrap());
+            assert_eq!(s, py_string.extract::<Cow<'_, str>>(py).unwrap());
         })
     }
 
@@ -180,9 +217,9 @@ mod tests {
         Python::with_gil(|py| {
             let s = "Hello Python";
             let py_string = Cow::Borrowed(s).to_object(py);
-            assert_eq!(s, py_string.extract::<&str>(py).unwrap());
+            assert_eq!(s, py_string.extract::<Cow<'_, str>>(py).unwrap());
             let py_string = Cow::<str>::Owned(s.into()).to_object(py);
-            assert_eq!(s, py_string.extract::<&str>(py).unwrap());
+            assert_eq!(s, py_string.extract::<Cow<'_, str>>(py).unwrap());
         })
     }
 
@@ -201,7 +238,7 @@ mod tests {
             let s = "Hello Python";
             let py_string = s.to_object(py);
 
-            let s2: &str = py_string.bind(py).extract().unwrap();
+            let s2: Cow<'_, str> = py_string.bind(py).extract().unwrap();
             assert_eq!(s, s2);
         })
     }
@@ -238,19 +275,19 @@ mod tests {
             assert_eq!(
                 s,
                 IntoPy::<PyObject>::into_py(s3, py)
-                    .extract::<&str>(py)
+                    .extract::<Cow<'_, str>>(py)
                     .unwrap()
             );
             assert_eq!(
                 s,
                 IntoPy::<PyObject>::into_py(s2, py)
-                    .extract::<&str>(py)
+                    .extract::<Cow<'_, str>>(py)
                     .unwrap()
             );
             assert_eq!(
                 s,
                 IntoPy::<PyObject>::into_py(s, py)
-                    .extract::<&str>(py)
+                    .extract::<Cow<'_, str>>(py)
                     .unwrap()
             );
         })

src/impl_/extract_argument.rs

@@ -1,10 +1,10 @@
 use crate::{
+    conversion::FromPyObjectBound,
     exceptions::PyTypeError,
     ffi,
     pyclass::boolean_struct::False,
     types::{any::PyAnyMethods, dict::PyDictMethods, tuple::PyTupleMethods, PyDict, PyTuple},
-    Borrowed, Bound, FromPyObject, PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, PyTypeCheck,
-    Python,
+    Borrowed, Bound, PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, PyTypeCheck, Python,
 };
 
 /// Helper type used to keep implementation more concise.
@@ -27,7 +27,7 @@ pub trait PyFunctionArgument<'a, 'py>: Sized + 'a {
 
 impl<'a, 'py, T> PyFunctionArgument<'a, 'py> for T
 where
-    T: FromPyObject<'py> + 'a,
+    T: FromPyObjectBound<'a, 'py> + 'a,
 {
     type Holder = ();
 
@@ -52,6 +52,19 @@ where
     }
 }
 
+#[cfg(all(Py_LIMITED_API, not(any(feature = "gil-refs", Py_3_10))))]
+impl<'a> PyFunctionArgument<'a, '_> for &'a str {
+    type Holder = Option<std::borrow::Cow<'a, str>>;
+
+    #[inline]
+    fn extract(
+        obj: &'a Bound<'_, PyAny>,
+        holder: &'a mut Option<std::borrow::Cow<'a, str>>,
+    ) -> PyResult<Self> {
+        Ok(holder.insert(obj.extract()?))
+    }
+}
+
 /// Trait for types which can be a function argument holder - they should
 /// to be able to const-initialize to an empty value.
 pub trait FunctionArgumentHolder: Sized {

src/impl_/pymodule.rs

@@ -149,7 +149,10 @@ impl<T: PyTypeInfo> PyAddToModule for T {
 
 #[cfg(test)]
 mod tests {
-    use std::sync::atomic::{AtomicBool, Ordering};
+    use std::{
+        borrow::Cow,
+        sync::atomic::{AtomicBool, Ordering},
+    };
 
     use crate::{
         types::{any::PyAnyMethods, module::PyModuleMethods, PyModule},
@@ -176,15 +179,15 @@ mod tests {
                 module
                     .getattr("__name__")
                     .unwrap()
-                    .extract::<&str>()
+                    .extract::<Cow<'_, str>>()
                     .unwrap(),
                 "test_module",
             );
             assert_eq!(
                 module
                     .getattr("__doc__")
                     .unwrap()
-                    .extract::<&str>()
+                    .extract::<Cow<'_, str>>()
                     .unwrap(),
                 "some doc",
             );

src/instance.rs

@@ -1315,9 +1315,12 @@ impl<T> Py<T> {
     /// Extracts some type from the Python object.
     ///
     /// This is a wrapper function around `FromPyObject::extract()`.
-    pub fn extract<'py, D>(&self, py: Python<'py>) -> PyResult<D>
+    pub fn extract<'a, 'py, D>(&'a self, py: Python<'py>) -> PyResult<D>
     where
-        D: FromPyObject<'py>,
+        D: crate::conversion::FromPyObjectBound<'a, 'py>,
+        // TODO it might be possible to relax this bound in future, to allow
+        // e.g. `.extract::<&str>(py)` where `py` is short-lived.
+        'py: 'a,
     {
         self.bind(py).as_any().extract()
     }

src/types/any.rs

@@ -1,5 +1,5 @@
 use crate::class::basic::CompareOp;
-use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject};
+use crate::conversion::{AsPyPointer, FromPyObject, FromPyObjectBound, IntoPy, ToPyObject};
 use crate::err::{DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyResult};
 use crate::exceptions::{PyAttributeError, PyTypeError};
 use crate::ffi_ptr_ext::FfiPtrExt;
@@ -1642,9 +1642,9 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed {
     /// Extracts some type from the Python object.
     ///
     /// This is a wrapper function around [`FromPyObject::extract()`].
-    fn extract<D>(&self) -> PyResult<D>
+    fn extract<'a, T>(&'a self) -> PyResult<T>
     where
-        D: FromPyObject<'py>;
+        T: FromPyObjectBound<'a, 'py>;
 
     /// Returns the reference count for the Python object.
     fn get_refcnt(&self) -> isize;
@@ -2178,11 +2178,11 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
         std::mem::transmute(self)
     }
 
-    fn extract<D>(&self) -> PyResult<D>
+    fn extract<'a, T>(&'a self) -> PyResult<T>
     where
-        D: FromPyObject<'py>,
+        T: FromPyObjectBound<'a, 'py>,
     {
-        FromPyObject::extract_bound(self)
+        FromPyObjectBound::from_py_object_bound(self)
     }
 
     fn get_refcnt(&self) -> isize {

src/types/bytes.rs

@@ -225,7 +225,8 @@ mod tests {
             let py_bytes = PyBytes::new_with(py, 10, |b: &mut [u8]| {
                 b.copy_from_slice(b"Hello Rust");
                 Ok(())
-            })?;
+            })?
+            .as_borrowed();
             let bytes: &[u8] = py_bytes.extract()?;
             assert_eq!(bytes, b"Hello Rust");
             Ok(())
@@ -235,7 +236,7 @@ mod tests {
     #[test]
     fn test_bytes_new_with_zero_initialised() -> super::PyResult<()> {
         Python::with_gil(|py| -> super::PyResult<()> {
-            let py_bytes = PyBytes::new_with(py, 10, |_b: &mut [u8]| Ok(()))?;
+            let py_bytes = PyBytes::new_with(py, 10, |_b: &mut [u8]| Ok(()))?.as_borrowed();
             let bytes: &[u8] = py_bytes.extract()?;
             assert_eq!(bytes, &[0; 10]);
             Ok(())