add FromPyObjectBound trait for extracting &str without GIL Refs (#…

…3928)

* add `FromPyObjectBound` adjustment for `&str` without GIL Refs

* review: alex, Icxolu feedback

* add newsfragment

* add newsfragment for `FromPyObject` trait change

* make some examples compatible with abi3 < 3.10

* seal `FromPyObjectBound`

* fixup chrono_tz conversion

guide/src/migration.md

@@ -211,6 +211,8 @@ Interactions with Python objects implemented in Rust no longer need to go though
 
 To minimise breakage of code using the GIL-Refs API, the `Bound<T>` smart pointer has been introduced by adding complements to all functions which accept or return GIL Refs. This allows code to migrate by replacing the deprecated APIs with the new ones.
 
+To identify what to migrate, temporarily switch off the `gil-refs` feature to see deprecation warnings on all uses of APIs accepting and producing GIL Refs. Over one or more PRs it should be possible to follow the deprecation hints to update code. Depending on your development environment, switching off the `gil-refs` feature may introduce [some very targeted breakages](#deactivating-the-gil-refs-feature), so you may need to fixup those first.
+
 For example, the following APIs have gained updated variants:
 - `PyList::new`, `PyTyple::new` and similar constructors have replacements `PyList::new_bound`, `PyTuple::new_bound` etc.
 - `FromPyObject::extract` has a new `FromPyObject::extract_bound` (see the section below)
@@ -272,7 +274,15 @@ 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.
 
+### Deactivating the `gil-refs` feature
+
+As a final step of migration, deactivating the `gil-refs` feature will set up code for best performance and is intended to set up a forward-compatible API for PyO3 0.22.
+
+There is one notable API removed when this feature is disabled. `FromPyObject` trait implementations for types which borrow directly from the input data cannot be implemented by PyO3 without GIL Refs (while the migration is ongoing). These types are `&str`, `Cow<'_, str>`, `&[u8]`, `Cow<'_, u8>`.
+
+To ease pain during migration, these types instead implement a new temporary trait `FromPyObjectBound` which is the expected future form of `FromPyObject`. The new temporary trait ensures is that `obj.extract::<&str>()` continues to work (with the new constraint that the extracted value now depends on the input `obj` lifetime), as well for these types in `#[pyfunction]` arguments.
 
+An unfortunate final point here is that PyO3 cannot offer this new implementation for `&str` on `abi3` builds for Python older than 3.10. On code which needs `abi3` builds for these older Python versions, many cases of `.extract::<&str>()` may need to be replaced with `.extract::<PyBackedStr>()`, which is string data which borrows from the Python `str` object. Alternatively, use `.extract::<Cow<str>>()`, `.extract::<String>()` to copy the data into Rust for these versions.
 
 ## from 0.19.* to 0.20
 

newsfragments/3928.added.md

@@ -0,0 +1 @@
+Implement `FromPyObject` for `Cow<str>`.

newsfragments/3928.changed.md

@@ -0,0 +1 @@
+Move implementations of `FromPyObject` for `&str`, `Cow<str>`, `&[u8]` and `Cow<[u8]>` onto a temporary trait `FromPyObjectBound` when `gil-refs` feature is deactivated.

noxfile.py

@@ -613,8 +613,7 @@ def check_feature_powerset(session: nox.Session):
     if toml is None:
         session.error("requires Python 3.11 or `toml` to be installed")
 
-    with (PYO3_DIR / "Cargo.toml").open("rb") as cargo_toml_file:
-        cargo_toml = toml.load(cargo_toml_file)
+    cargo_toml = toml.loads((PYO3_DIR / "Cargo.toml").read_text())
 
     EXCLUDED_FROM_FULL = {
         "nightly",

src/conversion.rs

@@ -178,12 +178,12 @@ pub trait IntoPy<T>: Sized {
 /// Python::with_gil(|py| {
 ///     // Calling `.extract()` on a `Bound` smart pointer
 ///     let obj: Bound<'_, PyString> = PyString::new_bound(py, "blah");
-///     let s: &str = obj.extract()?;
+///     let s: String = obj.extract()?;
 /// #   assert_eq!(s, "blah");
 ///
 ///     // Calling `.extract(py)` on a `Py` smart pointer
 ///     let obj: Py<PyString> = obj.unbind();
-///     let s: &str = obj.extract(py)?;
+///     let s: String = obj.extract(py)?;
 /// #   assert_eq!(s, "blah");
 /// #   Ok(())
 /// })
@@ -237,6 +237,85 @@ pub trait FromPyObject<'py>: Sized {
     }
 }
 
+mod from_py_object_bound_sealed {
+    /// Private seal for the `FromPyObjectBound` trait.
+    ///
+    /// This prevents downstream types from implementing the trait before
+    /// PyO3 is ready to declare the trait as public API.
+    pub trait Sealed {}
+
+    // This generic implementation is why the seal is separate from
+    // `crate::sealed::Sealed`.
+    impl<'py, T> Sealed for T where T: super::FromPyObject<'py> {}
+    #[cfg(not(feature = "gil-refs"))]
+    impl Sealed for &'_ str {}
+    #[cfg(not(feature = "gil-refs"))]
+    impl Sealed for std::borrow::Cow<'_, str> {}
+    #[cfg(not(feature = "gil-refs"))]
+    impl Sealed for &'_ [u8] {}
+    #[cfg(not(feature = "gil-refs"))]
+    impl Sealed for std::borrow::Cow<'_, [u8]> {}
+}
+
+/// 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.
+///
+/// # Usage
+///
+/// Users are prevented from implementing this trait, instead they should implement
+/// the normal `FromPyObject` trait. This trait has a blanket implementation
+/// for `T: FromPyObject`.
+///
+/// The only case where this trait may have a use case to be implemented is when the
+/// lifetime of the extracted value is tied to the lifetime `'a` of the input `Bound`
+/// instead of the GIL lifetime `py`, as is the case for the `&'a str` implementation.
+///
+/// Please contact the PyO3 maintainers if you believe you have a use case for implementing
+/// this trait before PyO3 is ready to change the main `FromPyObject` trait to take an
+/// additional lifetime.
+///
+/// Similarly, users should typically not call these trait methods and should instead
+/// use this via the `extract` method on `Bound` and `Py`.
+pub trait FromPyObjectBound<'a, 'py>: Sized + from_py_object_bound_sealed::Sealed {
+    /// Extracts `Self` from the bound smart pointer `obj`.
+    ///
+    /// 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>;
+
+    /// 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/chrono_tz.rs

@@ -34,6 +34,7 @@
 //! }
 //! ```
 use crate::exceptions::PyValueError;
+use crate::pybacked::PyBackedStr;
 use crate::sync::GILOnceCell;
 use crate::types::{any::PyAnyMethods, PyType};
 use crate::{
@@ -62,8 +63,11 @@ impl IntoPy<PyObject> for Tz {
 
 impl FromPyObject<'_> for Tz {
     fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Tz> {
-        Tz::from_str(ob.getattr(intern!(ob.py(), "key"))?.extract()?)
-            .map_err(|e| PyValueError::new_err(e.to_string()))
+        Tz::from_str(
+            &ob.getattr(intern!(ob.py(), "key"))?
+                .extract::<PyBackedStr>()?,
+        )
+        .map_err(|e| PyValueError::new_err(e.to_string()))
     }
 }
 

src/conversions/either.rs

@@ -113,6 +113,8 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::borrow::Cow;
+
     use crate::exceptions::PyTypeError;
     use crate::{Python, ToPyObject};
 
@@ -132,7 +134,7 @@ mod tests {
 
             let r = E::Right("foo".to_owned());
             let obj_r = r.to_object(py);
-            assert_eq!(obj_r.extract::<&str>(py).unwrap(), "foo");
+            assert_eq!(obj_r.extract::<Cow<'_, str>>(py).unwrap(), "foo");
             assert_eq!(obj_r.extract::<E>(py).unwrap(), r);
 
             let obj_s = "foo".to_object(py);

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 {
+        <String as crate::FromPyObject>::type_input()
+    }
+}
+
+#[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()
             );
         })