From 770d9b7f014cc70213950065b9b60bb3679464cd Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 8 Mar 2024 07:43:48 +0000 Subject: [PATCH] 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 | 10 ++++ newsfragments/3928.added.md | 1 + newsfragments/3928.changed.md | 1 + noxfile.py | 3 +- src/conversion.rs | 83 ++++++++++++++++++++++++++- src/conversions/chrono_tz.rs | 8 ++- src/conversions/either.rs | 4 +- src/conversions/std/slice.rs | 43 ++++++++++++-- src/conversions/std/string.rs | 55 +++++++++++++++--- src/impl_/extract_argument.rs | 19 +++++- src/impl_/pymodule.rs | 9 ++- src/inspect/types.rs | 5 +- src/instance.rs | 7 ++- src/types/any.rs | 32 +++++------ src/types/bytes.rs | 9 ++- src/types/module.rs | 10 +++- tests/test_class_conversion.rs | 2 +- tests/test_compile_error.rs | 1 + tests/test_pyfunction.rs | 2 +- tests/ui/invalid_cancel_handle.stderr | 2 + 20 files changed, 251 insertions(+), 55 deletions(-) create mode 100644 newsfragments/3928.added.md create mode 100644 newsfragments/3928.changed.md diff --git a/guide/src/migration.md b/guide/src/migration.md index 8537f9a1..fa4c23cc 100644 --- a/guide/src/migration.md +++ b/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` 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::()`, which is string data which borrows from the Python `str` object. Alternatively, use `.extract::>()`, `.extract::()` to copy the data into Rust for these versions. ## from 0.19.* to 0.20 diff --git a/newsfragments/3928.added.md b/newsfragments/3928.added.md new file mode 100644 index 00000000..e768e6b0 --- /dev/null +++ b/newsfragments/3928.added.md @@ -0,0 +1 @@ +Implement `FromPyObject` for `Cow`. diff --git a/newsfragments/3928.changed.md b/newsfragments/3928.changed.md new file mode 100644 index 00000000..a4734c41 --- /dev/null +++ b/newsfragments/3928.changed.md @@ -0,0 +1 @@ +Move implementations of `FromPyObject` for `&str`, `Cow`, `&[u8]` and `Cow<[u8]>` onto a temporary trait `FromPyObjectBound` when `gil-refs` feature is deactivated. diff --git a/noxfile.py b/noxfile.py index 2aa0f464..3bba2574 100644 --- a/noxfile.py +++ b/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", diff --git a/src/conversion.rs b/src/conversion.rs index 68633d19..efb09b6b 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -178,12 +178,12 @@ pub trait IntoPy: 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 = 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; + + /// Extracts the type hint information for this type when it appears as an argument. + /// + /// For example, `Vec` 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::extract_bound(ob) + } + + #[cfg(feature = "experimental-inspect")] + fn type_input() -> TypeInfo { + ::type_input() + } +} + /// Identity conversion: allows using existing `PyObject` instances where /// `T: ToPyObject` is expected. impl ToPyObject for &'_ T { diff --git a/src/conversions/chrono_tz.rs b/src/conversions/chrono_tz.rs index b4d0e7ed..845814c4 100644 --- a/src/conversions/chrono_tz.rs +++ b/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 for Tz { impl FromPyObject<'_> for Tz { fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult { - 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::()?, + ) + .map_err(|e| PyValueError::new_err(e.to_string())) } } diff --git a/src/conversions/either.rs b/src/conversions/either.rs index 16d5ad49..84ec88ea 100644 --- a/src/conversions/either.rs +++ b/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::>(py).unwrap(), "foo"); assert_eq!(obj_r.extract::(py).unwrap(), r); let obj_s = "foo".to_object(py); diff --git a/src/conversions/std/slice.rs b/src/conversions/std/slice.rs index dad27f08..18bb52cd 100644 --- a/src/conversions/std/slice.rs +++ b/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 for &'a [u8] { @@ -18,7 +20,8 @@ impl<'a> IntoPy 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 { Ok(obj.downcast::()?.as_bytes()) } @@ -29,13 +32,38 @@ 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 { + Ok(obj.downcast::()?.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 { +#[cfg(feature = "gil-refs")] +impl<'py> crate::FromPyObject<'py> for Cow<'py, [u8]> { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + if let Ok(bytes) = ob.downcast::() { + return Ok(Cow::Borrowed(bytes.clone().into_gil_ref().as_bytes())); + } + + let byte_array = ob.downcast::()?; + 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 { if let Ok(bytes) = ob.downcast::() { return Ok(Cow::Borrowed(bytes.as_bytes())); } @@ -43,6 +71,11 @@ impl<'py> FromPyObject<'py> for Cow<'py, [u8]> { let byte_array = ob.downcast::()?; Ok(Cow::Owned(byte_array.to_vec())) } + + #[cfg(feature = "experimental-inspect")] + fn type_input() -> TypeInfo { + Self::type_output() + } } impl ToPyObject for Cow<'_, [u8]> { diff --git a/src/conversions/std/string.rs b/src/conversions/std/string.rs index 60e8ff7e..d013890a 100644 --- a/src/conversions/std/string.rs +++ b/src/conversions/std/string.rs @@ -113,7 +113,8 @@ impl<'a> IntoPy 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 { ob.downcast::()?.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 { + ob.downcast::()?.to_str() + } + + #[cfg(feature = "experimental-inspect")] + fn type_input() -> TypeInfo { + ::type_input() + } +} + +#[cfg(feature = "gil-refs")] +impl<'py> FromPyObject<'py> for Cow<'py, str> { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + ob.extract().map(Cow::Owned) + } + + #[cfg(feature = "experimental-inspect")] + fn type_input() -> TypeInfo { + ::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 { + ob.downcast::()?.to_cow() + } + + #[cfg(feature = "experimental-inspect")] + fn type_input() -> TypeInfo { + ::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::>(py).unwrap()); let py_string: PyObject = Cow::::Owned(s.into()).into_py(py); - assert_eq!(s, py_string.extract::<&str>(py).unwrap()); + assert_eq!(s, py_string.extract::>(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::>(py).unwrap()); let py_string = Cow::::Owned(s.into()).to_object(py); - assert_eq!(s, py_string.extract::<&str>(py).unwrap()); + assert_eq!(s, py_string.extract::>(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::::into_py(s3, py) - .extract::<&str>(py) + .extract::>(py) .unwrap() ); assert_eq!( s, IntoPy::::into_py(s2, py) - .extract::<&str>(py) + .extract::>(py) .unwrap() ); assert_eq!( s, IntoPy::::into_py(s, py) - .extract::<&str>(py) + .extract::>(py) .unwrap() ); }) diff --git a/src/impl_/extract_argument.rs b/src/impl_/extract_argument.rs index d2e2b18c..82285b3d 100644 --- a/src/impl_/extract_argument.rs +++ b/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>; + + #[inline] + fn extract( + obj: &'a Bound<'_, PyAny>, + holder: &'a mut Option>, + ) -> PyResult { + 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 { diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index a19cbda5..9ca80e39 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -142,7 +142,10 @@ pub trait PyAddToModule { #[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}, @@ -169,7 +172,7 @@ mod tests { module .getattr("__name__") .unwrap() - .extract::<&str>() + .extract::>() .unwrap(), "test_module", ); @@ -177,7 +180,7 @@ mod tests { module .getattr("__doc__") .unwrap() - .extract::<&str>() + .extract::>() .unwrap(), "some doc", ); diff --git a/src/inspect/types.rs b/src/inspect/types.rs index d8f8e6a1..13ce6258 100644 --- a/src/inspect/types.rs +++ b/src/inspect/types.rs @@ -464,7 +464,10 @@ mod conversion { assert_display(&String::type_input(), "str"); assert_display(&<&[u8]>::type_output(), "bytes"); - assert_display(&<&[u8]>::type_input(), "bytes"); + assert_display( + &<&[u8] as crate::conversion::FromPyObjectBound>::type_input(), + "bytes", + ); } #[test] diff --git a/src/instance.rs b/src/instance.rs index cef06062..069bea69 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1315,9 +1315,12 @@ impl Py { /// 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 + pub fn extract<'a, 'py, D>(&'a self, py: Python<'py>) -> PyResult 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() } diff --git a/src/types/any.rs b/src/types/any.rs index 0207217b..e4dbaf80 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, 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; @@ -391,7 +391,7 @@ impl PyAny { /// let kwargs = PyDict::new_bound(py); /// kwargs.set_item("cruel", "world")?; /// let result = fun.call(args, Some(&kwargs))?; - /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); + /// assert_eq!(result.extract::()?, "called with args and kwargs"); /// Ok(()) /// }) /// # } @@ -452,7 +452,7 @@ impl PyAny { /// let fun = module.getattr("function")?; /// let args = ("hello",); /// let result = fun.call1(args)?; - /// assert_eq!(result.extract::<&str>()?, "called with args"); + /// assert_eq!(result.extract::()?, "called with args"); /// Ok(()) /// }) /// # } @@ -491,7 +491,7 @@ impl PyAny { /// let kwargs = PyDict::new_bound(py); /// kwargs.set_item("cruel", "world")?; /// let result = instance.call_method("method", args, Some(&kwargs))?; - /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); + /// assert_eq!(result.extract::()?, "called with args and kwargs"); /// Ok(()) /// }) /// # } @@ -532,7 +532,7 @@ impl PyAny { /// let module = PyModule::from_code_bound(py, CODE, "", "")?; /// let instance = module.getattr("a")?; /// let result = instance.call_method0("method")?; - /// assert_eq!(result.extract::<&str>()?, "called with no arguments"); + /// assert_eq!(result.extract::()?, "called with no arguments"); /// Ok(()) /// }) /// # } @@ -573,7 +573,7 @@ impl PyAny { /// let instance = module.getattr("a")?; /// let args = ("hello",); /// let result = instance.call_method1("method", args)?; - /// assert_eq!(result.extract::<&str>()?, "called with args"); + /// assert_eq!(result.extract::()?, "called with args"); /// Ok(()) /// }) /// # } @@ -1271,7 +1271,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// let kwargs = PyDict::new_bound(py); /// kwargs.set_item("cruel", "world")?; /// let result = fun.call(args, Some(&kwargs))?; - /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); + /// assert_eq!(result.extract::()?, "called with args and kwargs"); /// Ok(()) /// }) /// # } @@ -1326,7 +1326,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// let fun = module.getattr("function")?; /// let args = ("hello",); /// let result = fun.call1(args)?; - /// assert_eq!(result.extract::<&str>()?, "called with args"); + /// assert_eq!(result.extract::()?, "called with args"); /// Ok(()) /// }) /// # } @@ -1363,7 +1363,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// let kwargs = PyDict::new_bound(py); /// kwargs.set_item("cruel", "world")?; /// let result = instance.call_method("method", args, Some(&kwargs))?; - /// assert_eq!(result.extract::<&str>()?, "called with args and kwargs"); + /// assert_eq!(result.extract::()?, "called with args and kwargs"); /// Ok(()) /// }) /// # } @@ -1404,7 +1404,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// let module = PyModule::from_code_bound(py, CODE, "", "")?; /// let instance = module.getattr("a")?; /// let result = instance.call_method0("method")?; - /// assert_eq!(result.extract::<&str>()?, "called with no arguments"); + /// assert_eq!(result.extract::()?, "called with no arguments"); /// Ok(()) /// }) /// # } @@ -1440,7 +1440,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// let instance = module.getattr("a")?; /// let args = ("hello",); /// let result = instance.call_method1("method", args)?; - /// assert_eq!(result.extract::<&str>()?, "called with args"); + /// assert_eq!(result.extract::()?, "called with args"); /// Ok(()) /// }) /// # } @@ -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(&self) -> PyResult + fn extract<'a, T>(&'a self) -> PyResult 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(&self) -> PyResult + fn extract<'a, T>(&'a self) -> PyResult where - D: FromPyObject<'py>, + T: FromPyObjectBound<'a, 'py>, { - FromPyObject::extract_bound(self) + FromPyObjectBound::from_py_object_bound(self) } fn get_refcnt(&self) -> isize { diff --git a/src/types/bytes.rs b/src/types/bytes.rs index 2a54c172..55c295c4 100644 --- a/src/types/bytes.rs +++ b/src/types/bytes.rs @@ -196,14 +196,13 @@ impl> Index for Bound<'_, PyBytes> { } #[cfg(test)] -#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::*; #[test] fn test_bytes_index() { Python::with_gil(|py| { - let bytes = PyBytes::new(py, b"Hello World"); + let bytes = PyBytes::new_bound(py, b"Hello World"); assert_eq!(bytes[1], b'e'); }); } @@ -222,7 +221,7 @@ mod tests { #[test] fn test_bytes_new_with() -> super::PyResult<()> { Python::with_gil(|py| -> super::PyResult<()> { - let py_bytes = PyBytes::new_with(py, 10, |b: &mut [u8]| { + let py_bytes = PyBytes::new_bound_with(py, 10, |b: &mut [u8]| { b.copy_from_slice(b"Hello Rust"); Ok(()) })?; @@ -235,7 +234,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_bound_with(py, 10, |_b: &mut [u8]| Ok(()))?; let bytes: &[u8] = py_bytes.extract()?; assert_eq!(bytes, &[0; 10]); Ok(()) @@ -246,7 +245,7 @@ mod tests { fn test_bytes_new_with_error() { use crate::exceptions::PyValueError; Python::with_gil(|py| { - let py_bytes_result = PyBytes::new_with(py, 10, |_b: &mut [u8]| { + let py_bytes_result = PyBytes::new_bound_with(py, 10, |_b: &mut [u8]| { Err(PyValueError::new_err("Hello Crustaceans!")) }); assert!(py_bytes_result.is_err()); diff --git a/src/types/module.rs b/src/types/module.rs index 99306070..11afcf76 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -10,6 +10,8 @@ use crate::{exceptions, ffi, Bound, IntoPy, Py, PyNativeType, PyObject, Python}; use std::ffi::CString; use std::str; +use super::PyStringMethods; + /// Represents a Python [`module`][1] object. /// /// As with all other Python objects, modules are first class citizens. @@ -399,8 +401,12 @@ impl PyModule { /// [1]: crate::prelude::pyfunction /// [2]: crate::wrap_pyfunction pub fn add_function<'a>(&'a self, fun: &'a PyCFunction) -> PyResult<()> { - let name = fun.getattr(__name__(self.py()))?.extract()?; - self.add(name, fun) + let name = fun + .as_borrowed() + .getattr(__name__(self.py()))? + .downcast_into::()?; + let name = name.to_cow()?; + self.add(&name, fun) } } diff --git a/tests/test_class_conversion.rs b/tests/test_class_conversion.rs index a0919527..ede8928f 100644 --- a/tests/test_class_conversion.rs +++ b/tests/test_class_conversion.rs @@ -151,7 +151,7 @@ fn test_pycell_deref() { // Should be able to deref as PyAny assert_eq!( obj.call_method0("foo") - .and_then(|e| e.extract::<&str>()) + .and_then(|e| e.extract::()) .unwrap(), "SubClass" ); diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 3f1052c3..38b5c472 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -51,5 +51,6 @@ fn test_compile_errors() { #[cfg(feature = "experimental-declarative-modules")] t.compile_fail("tests/ui/invalid_pymodule_two_pymodule_init.rs"); #[cfg(feature = "experimental-async")] + #[cfg(any(not(Py_LIMITED_API), Py_3_10))] // to avoid PyFunctionArgument for &str t.compile_fail("tests/ui/invalid_cancel_handle.rs"); } diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 838f7353..32c3ede6 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -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, struct_arg: Option, ) { diff --git a/tests/ui/invalid_cancel_handle.stderr b/tests/ui/invalid_cancel_handle.stderr index 6dc3ff3c..9e617bca 100644 --- a/tests/ui/invalid_cancel_handle.stderr +++ b/tests/ui/invalid_cancel_handle.stderr @@ -40,6 +40,7 @@ error[E0277]: the trait bound `CancelHandle: PyClass` is not satisfied | = help: the trait `PyClass` is implemented for `pyo3::coroutine::Coroutine` = note: required for `CancelHandle` to implement `FromPyObject<'_>` + = note: required for `CancelHandle` to implement `FromPyObjectBound<'_, '_>` = note: required for `CancelHandle` to implement `PyFunctionArgument<'_, '_>` note: required by a bound in `extract_argument` --> src/impl_/extract_argument.rs @@ -61,6 +62,7 @@ error[E0277]: the trait bound `CancelHandle: Clone` is not satisfied &'a pyo3::coroutine::Coroutine &'a mut pyo3::coroutine::Coroutine = note: required for `CancelHandle` to implement `FromPyObject<'_>` + = note: required for `CancelHandle` to implement `FromPyObjectBound<'_, '_>` = note: required for `CancelHandle` to implement `PyFunctionArgument<'_, '_>` note: required by a bound in `extract_argument` --> src/impl_/extract_argument.rs