From c47565666d4b6431259994ea4ad72e8e48965d86 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 21 Nov 2023 11:41:43 +0000 Subject: [PATCH] add `PyString::new_bound` --- guide/src/conversions/traits.md | 4 +-- pyo3-benches/benches/bench_extract.rs | 15 ++++---- pyo3-benches/benches/bench_frompyobject.rs | 21 ++++++----- src/conversion.rs | 2 +- src/conversions/std/ipaddr.rs | 4 +-- src/conversions/std/string.rs | 16 ++++----- src/err/mod.rs | 2 +- src/ffi/tests.rs | 8 ++--- src/instance.rs | 2 +- src/marker.rs | 15 ++++---- src/types/any.rs | 4 +-- src/types/string.rs | 21 +++++++++-- tests/test_pyself.rs | 2 +- tests/ui/not_send2.rs | 4 +-- tests/ui/not_send2.stderr | 41 +++++++++++++++------- 15 files changed, 96 insertions(+), 65 deletions(-) diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md index 746f098b..f657fe07 100644 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -236,7 +236,7 @@ struct RustyTransparentStruct { # use pyo3::types::PyString; # fn main() -> PyResult<()> { # Python::with_gil(|py| -> PyResult<()> { -# let s = PyString::new(py, "test"); +# let s = PyString::new_bound(py, "test"); # # let tup: RustyTransparentTupleStruct = s.extract()?; # assert_eq!(tup.0, "test"); @@ -303,7 +303,7 @@ enum RustyEnum<'a> { # ); # } # { -# let thing = PyString::new(py, "text"); +# let thing = PyString::new_bound(py, "text"); # let rust_thing: RustyEnum<'_> = thing.extract()?; # # assert_eq!( diff --git a/pyo3-benches/benches/bench_extract.rs b/pyo3-benches/benches/bench_extract.rs index 6ff22d38..479bd0fd 100644 --- a/pyo3-benches/benches/bench_extract.rs +++ b/pyo3-benches/benches/bench_extract.rs @@ -1,18 +1,16 @@ use codspeed_criterion_compat::{black_box, criterion_group, criterion_main, Bencher, Criterion}; use pyo3::{ + prelude::*, types::{PyDict, PyFloat, PyInt, PyString}, IntoPy, PyAny, PyObject, Python, }; fn extract_str_extract_success(bench: &mut Bencher<'_>) { Python::with_gil(|py| { - let s = PyString::new(py, "Hello, World!") as &PyAny; + let s = &PyString::new_bound(py, "Hello, World!"); - bench.iter(|| { - let v = black_box(s).extract::<&str>().unwrap(); - black_box(v); - }); + bench.iter(|| black_box(s).extract::<&str>().unwrap()); }); } @@ -27,14 +25,14 @@ fn extract_str_extract_fail(bench: &mut Bencher<'_>) { }); } +#[cfg(any(Py_3_10, not(Py_LIMITED_API)))] fn extract_str_downcast_success(bench: &mut Bencher<'_>) { Python::with_gil(|py| { - let s = PyString::new(py, "Hello, World!") as &PyAny; + let s = &PyString::new_bound(py, "Hello, World!"); bench.iter(|| { let py_str = black_box(s).downcast::().unwrap(); - let v = py_str.to_str().unwrap(); - black_box(v); + py_str.to_str().unwrap() }); }); } @@ -147,6 +145,7 @@ fn extract_float_downcast_fail(bench: &mut Bencher<'_>) { fn criterion_benchmark(c: &mut Criterion) { c.bench_function("extract_str_extract_success", extract_str_extract_success); c.bench_function("extract_str_extract_fail", extract_str_extract_fail); + #[cfg(any(Py_3_10, not(Py_LIMITED_API)))] c.bench_function("extract_str_downcast_success", extract_str_downcast_success); c.bench_function("extract_str_downcast_fail", extract_str_downcast_fail); c.bench_function("extract_int_extract_success", extract_int_extract_success); diff --git a/pyo3-benches/benches/bench_frompyobject.rs b/pyo3-benches/benches/bench_frompyobject.rs index 5a6cbcf7..8114ee5a 100644 --- a/pyo3-benches/benches/bench_frompyobject.rs +++ b/pyo3-benches/benches/bench_frompyobject.rs @@ -14,10 +14,9 @@ enum ManyTypes { fn enum_from_pyobject(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let obj = PyString::new(py, "hello world"); - b.iter(|| { - let _: ManyTypes = obj.extract().unwrap(); - }); + let any: &Bound<'_, PyAny> = &PyString::new_bound(py, "hello world"); + + b.iter(|| any.extract::().unwrap()); }) } @@ -39,7 +38,7 @@ fn list_via_extract(b: &mut Bencher<'_>) { fn not_a_list_via_downcast(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let any: &PyAny = PyString::new(py, "foobar").into(); + let any: &Bound<'_, PyAny> = &PyString::new_bound(py, "foobar"); b.iter(|| black_box(any).downcast::().unwrap_err()); }) @@ -47,25 +46,25 @@ fn not_a_list_via_downcast(b: &mut Bencher<'_>) { fn not_a_list_via_extract(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let any: &PyAny = PyString::new(py, "foobar").into(); + let any: &Bound<'_, PyAny> = &PyString::new_bound(py, "foobar"); - b.iter(|| black_box(any).extract::<&PyList>().unwrap_err()); + b.iter(|| black_box(any).extract::>().unwrap_err()); }) } #[derive(FromPyObject)] enum ListOrNotList<'a> { - List(&'a PyList), - NotList(&'a PyAny), + List(Bound<'a, PyList>), + NotList(Bound<'a, PyAny>), } fn not_a_list_via_extract_enum(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let any: &PyAny = PyString::new(py, "foobar").into(); + let any: &Bound<'_, PyAny> = &PyString::new_bound(py, "foobar"); b.iter(|| match black_box(any).extract::>() { Ok(ListOrNotList::List(_list)) => panic!(), - Ok(ListOrNotList::NotList(_any)) => (), + Ok(ListOrNotList::NotList(any)) => any, Err(_) => panic!(), }); }) diff --git a/src/conversion.rs b/src/conversion.rs index 46b39436..98ef9831 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -190,7 +190,7 @@ pub trait IntoPy: Sized { /// /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { -/// let obj: Py = PyString::new(py, "blah").into(); +/// let obj: Py = PyString::new_bound(py, "blah").into(); /// /// // Straight from an owned reference /// let s: &str = obj.extract(py)?; diff --git a/src/conversions/std/ipaddr.rs b/src/conversions/std/ipaddr.rs index ca3c8728..713de0af 100755 --- a/src/conversions/std/ipaddr.rs +++ b/src/conversions/std/ipaddr.rs @@ -99,11 +99,11 @@ mod test_ipaddr { #[test] fn test_from_pystring() { Python::with_gil(|py| { - let py_str = PyString::new(py, "0:0:0:0:0:0:0:1"); + let py_str = PyString::new_bound(py, "0:0:0:0:0:0:0:1"); let ip: IpAddr = py_str.to_object(py).extract(py).unwrap(); assert_eq!(ip, IpAddr::from_str("::1").unwrap()); - let py_str = PyString::new(py, "invalid"); + let py_str = PyString::new_bound(py, "invalid"); assert!(py_str.to_object(py).extract::(py).is_err()); }); } diff --git a/src/conversions/std/string.rs b/src/conversions/std/string.rs index b4bc8c26..b43e5422 100644 --- a/src/conversions/std/string.rs +++ b/src/conversions/std/string.rs @@ -11,14 +11,14 @@ use crate::{ impl ToPyObject for str { #[inline] fn to_object(&self, py: Python<'_>) -> PyObject { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } } impl<'a> IntoPy for &'a str { #[inline] fn into_py(self, py: Python<'_>) -> PyObject { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } #[cfg(feature = "experimental-inspect")] @@ -30,7 +30,7 @@ impl<'a> IntoPy for &'a str { impl<'a> IntoPy> for &'a str { #[inline] fn into_py(self, py: Python<'_>) -> Py { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } #[cfg(feature = "experimental-inspect")] @@ -44,7 +44,7 @@ impl<'a> IntoPy> for &'a str { impl ToPyObject for Cow<'_, str> { #[inline] fn to_object(&self, py: Python<'_>) -> PyObject { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } } @@ -65,7 +65,7 @@ impl IntoPy for Cow<'_, str> { impl ToPyObject for String { #[inline] fn to_object(&self, py: Python<'_>) -> PyObject { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } } @@ -78,7 +78,7 @@ impl ToPyObject for char { impl IntoPy for char { fn into_py(self, py: Python<'_>) -> PyObject { let mut bytes = [0u8; 4]; - PyString::new(py, self.encode_utf8(&mut bytes)).into() + PyString::new_bound(py, self.encode_utf8(&mut bytes)).into() } #[cfg(feature = "experimental-inspect")] @@ -89,7 +89,7 @@ impl IntoPy for char { impl IntoPy for String { fn into_py(self, py: Python<'_>) -> PyObject { - PyString::new(py, &self).into() + PyString::new_bound(py, &self).into() } #[cfg(feature = "experimental-inspect")] @@ -101,7 +101,7 @@ impl IntoPy for String { impl<'a> IntoPy for &'a String { #[inline] fn into_py(self, py: Python<'_>) -> PyObject { - PyString::new(py, self).into() + PyString::new_bound(py, self).into() } #[cfg(feature = "experimental-inspect")] diff --git a/src/err/mod.rs b/src/err/mod.rs index e6bcd521..13e154b3 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -206,7 +206,7 @@ impl PyErr { /// assert_eq!(err.to_string(), "TypeError: "); /// /// // Case #3: Invalid exception value - /// let err = PyErr::from_value(PyString::new(py, "foo").into()); + /// let err = PyErr::from_value(PyString::new_bound(py, "foo").as_gil_ref()); /// assert_eq!( /// err.to_string(), /// "TypeError: exceptions must derive from BaseException" diff --git a/src/ffi/tests.rs b/src/ffi/tests.rs index 2cbd84dc..cb097259 100644 --- a/src/ffi/tests.rs +++ b/src/ffi/tests.rs @@ -3,7 +3,7 @@ use crate::Python; #[cfg(not(Py_LIMITED_API))] use crate::{ - types::{PyDict, PyString}, + types::{any::PyAnyMethods, PyDict, PyString}, IntoPy, Py, PyAny, }; #[cfg(not(any(Py_3_12, Py_LIMITED_API)))] @@ -98,7 +98,7 @@ fn test_timezone_from_offset_and_name() { Python::with_gil(|py| { let delta = PyDelta::new(py, 0, 100, 0, false).unwrap(); - let tzname = PyString::new(py, "testtz"); + let tzname = PyString::new_bound(py, "testtz"); let tz: &PyAny = unsafe { py.from_borrowed_ptr(PyTimeZone_FromOffsetAndName( delta.as_ptr(), @@ -167,7 +167,7 @@ fn ascii_object_bitfield() { fn ascii() { Python::with_gil(|py| { // This test relies on implementation details of PyString. - let s = PyString::new(py, "hello, world"); + let s = PyString::new_bound(py, "hello, world"); let ptr = s.as_ptr(); unsafe { @@ -209,7 +209,7 @@ fn ascii() { fn ucs4() { Python::with_gil(|py| { let s = "哈哈🐈"; - let py_string = PyString::new(py, s); + let py_string = PyString::new_bound(py, s); let ptr = py_string.as_ptr(); unsafe { diff --git a/src/instance.rs b/src/instance.rs index d7edfe06..c1ab4592 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1624,7 +1624,7 @@ a = A() assert!(instance .getattr(py, "foo")? .as_ref(py) - .eq(PyString::new(py, "bar"))?); + .eq(PyString::new_bound(py, "bar"))?); instance.getattr(py, "foo")?; Ok(()) diff --git a/src/marker.rs b/src/marker.rs index 74c7095f..836d0109 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -72,7 +72,7 @@ //! use send_wrapper::SendWrapper; //! //! Python::with_gil(|py| { -//! let string = PyString::new(py, "foo"); +//! let string = PyString::new_bound(py, "foo"); //! //! let wrapped = SendWrapper::new(string); //! @@ -80,7 +80,7 @@ //! # #[cfg(not(feature = "nightly"))] //! # { //! // 💥 Unsound! 💥 -//! let smuggled: &PyString = *wrapped; +//! let smuggled: &Bound<'_, PyString> = &*wrapped; //! println!("{:?}", smuggled); //! # } //! }); @@ -164,12 +164,12 @@ use std::os::raw::c_int; /// use send_wrapper::SendWrapper; /// /// Python::with_gil(|py| { -/// let string = PyString::new(py, "foo"); +/// let string = PyString::new_bound(py, "foo"); /// /// let wrapped = SendWrapper::new(string); /// /// py.allow_threads(|| { -/// let sneaky: &PyString = *wrapped; +/// let sneaky: &Bound<'_, PyString> = &*wrapped; /// /// println!("{:?}", sneaky); /// }); @@ -210,7 +210,7 @@ mod nightly { /// # use pyo3::prelude::*; /// # use pyo3::types::PyString; /// Python::with_gil(|py| { - /// let string = PyString::new(py, "foo"); + /// let string = PyString::new_bound(py, "foo"); /// /// py.allow_threads(|| { /// println!("{:?}", string); @@ -238,7 +238,7 @@ mod nightly { /// use send_wrapper::SendWrapper; /// /// Python::with_gil(|py| { - /// let string = PyString::new(py, "foo"); + /// let string = PyString::new_bound(py, "foo"); /// /// let wrapped = SendWrapper::new(string); /// @@ -521,7 +521,7 @@ impl<'py> Python<'py> { /// use pyo3::types::PyString; /// /// fn parallel_print(py: Python<'_>) { - /// let s = PyString::new(py, "This object cannot be accessed without holding the GIL >_<"); + /// let s = PyString::new_bound(py, "This object cannot be accessed without holding the GIL >_<"); /// py.allow_threads(move || { /// println!("{:?}", s); // This causes a compile error. /// }); @@ -1004,6 +1004,7 @@ impl Python<'_> { /// The `Ungil` bound on the closure does prevent hanging on to existing GIL-bound references /// /// ```compile_fail + /// # #![allow(deprecated)] /// # use pyo3::prelude::*; /// # use pyo3::types::PyString; /// diff --git a/src/types/any.rs b/src/types/any.rs index cd19e847..14102d80 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -219,7 +219,7 @@ impl PyAny { /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { /// let a = PyFloat::new_bound(py, 0_f64); - /// let b = PyString::new(py, "zero"); + /// let b = PyString::new_bound(py, "zero"); /// assert!(a.compare(b).is_err()); /// Ok(()) /// })?; @@ -1075,7 +1075,7 @@ pub trait PyAnyMethods<'py> { /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { /// let a = PyFloat::new_bound(py, 0_f64); - /// let b = PyString::new(py, "zero"); + /// let b = PyString::new_bound(py, "zero"); /// assert!(a.compare(b).is_err()); /// Ok(()) /// })?; diff --git a/src/types/string.rs b/src/types/string.rs index 0ee64be5..fd20aab8 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -134,13 +134,29 @@ pub struct PyString(PyAny); pyobject_native_type_core!(PyString, pyobject_native_static_type_object!(ffi::PyUnicode_Type), #checkfunction=ffi::PyUnicode_Check); impl PyString { + /// Deprecated form of [`PyString::new_bound`]. + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyString::new` will be replaced by `PyString::new_bound` in a future PyO3 version" + ) + )] + pub fn new<'py>(py: Python<'py>, s: &str) -> &'py Self { + Self::new_bound(py, s).into_gil_ref() + } + /// Creates a new Python string object. /// /// Panics if out of memory. - pub fn new<'p>(py: Python<'p>, s: &str) -> &'p PyString { + pub fn new_bound<'py>(py: Python<'py>, s: &str) -> Bound<'py, PyString> { let ptr = s.as_ptr() as *const c_char; let len = s.len() as ffi::Py_ssize_t; - unsafe { py.from_owned_ptr(ffi::PyUnicode_FromStringAndSize(ptr, len)) } + unsafe { + ffi::PyUnicode_FromStringAndSize(ptr, len) + .assume_owned(py) + .downcast_into_unchecked() + } } /// Intern the given string @@ -452,6 +468,7 @@ impl IntoPy> for &'_ Py { } #[cfg(test)] +#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::*; use crate::Python; diff --git a/tests/test_pyself.rs b/tests/test_pyself.rs index ed922d3e..7abec02b 100644 --- a/tests/test_pyself.rs +++ b/tests/test_pyself.rs @@ -73,7 +73,7 @@ impl Iter { let res = reader_ref .inner .get(&b) - .map(|s| PyString::new(py, s).into()); + .map(|s| PyString::new_bound(py, s).into()); Ok(res) } None => Ok(None), diff --git a/tests/ui/not_send2.rs b/tests/ui/not_send2.rs index 4eb0a9f0..fa99e602 100644 --- a/tests/ui/not_send2.rs +++ b/tests/ui/not_send2.rs @@ -3,10 +3,10 @@ use pyo3::types::PyString; fn main() { Python::with_gil(|py| { - let string = PyString::new(py, "foo"); + let string = PyString::new_bound(py, "foo"); py.allow_threads(|| { println!("{:?}", string); }); }); -} \ No newline at end of file +} diff --git a/tests/ui/not_send2.stderr b/tests/ui/not_send2.stderr index d3a60a1f..30189e09 100644 --- a/tests/ui/not_send2.stderr +++ b/tests/ui/not_send2.stderr @@ -1,4 +1,4 @@ -error[E0277]: `UnsafeCell` cannot be shared between threads safely +error[E0277]: `*mut pyo3::Python<'static>` cannot be shared between threads safely --> tests/ui/not_send2.rs:8:26 | 8 | py.allow_threads(|| { @@ -7,21 +7,36 @@ error[E0277]: `UnsafeCell` cannot be shared between threads safely | | required by a bound introduced by this call 9 | | println!("{:?}", string); 10 | | }); - | |_________^ `UnsafeCell` cannot be shared between threads safely + | |_________^ `*mut pyo3::Python<'static>` cannot be shared between threads safely | - = help: within `&PyString`, the trait `Sync` is not implemented for `UnsafeCell` -note: required because it appears within the type `PyAny` - --> src/types/any.rs + = help: within `pyo3::Bound<'_, PyString>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>` +note: required because it appears within the type `PhantomData<*mut Python<'static>>` + --> $RUST/core/src/marker.rs | - | pub struct PyAny(UnsafeCell); + | pub struct PhantomData; + | ^^^^^^^^^^^ +note: required because it appears within the type `NotSend` + --> src/impl_/not_send.rs + | + | pub(crate) struct NotSend(PhantomData<*mut Python<'static>>); + | ^^^^^^^ + = note: required because it appears within the type `(&GILGuard, NotSend)` +note: required because it appears within the type `PhantomData<(&GILGuard, NotSend)>` + --> $RUST/core/src/marker.rs + | + | pub struct PhantomData; + | ^^^^^^^^^^^ +note: required because it appears within the type `Python<'_>` + --> src/marker.rs + | + | pub struct Python<'py>(PhantomData<(&'py GILGuard, NotSend)>); + | ^^^^^^ +note: required because it appears within the type `Bound<'_, PyString>` + --> src/instance.rs + | + | pub struct Bound<'py, T>(Python<'py>, ManuallyDrop>); | ^^^^^ -note: required because it appears within the type `PyString` - --> src/types/string.rs - | - | pub struct PyString(PyAny); - | ^^^^^^^^ - = note: required because it appears within the type `&PyString` - = note: required for `&&PyString` to implement `Send` + = note: required for `&pyo3::Bound<'_, PyString>` to implement `Send` note: required because it's used within this closure --> tests/ui/not_send2.rs:8:26 |