Skip to content

Commit

Permalink
add PyString::new_bound
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jan 29, 2024
1 parent c54d897 commit b399737
Show file tree
Hide file tree
Showing 15 changed files with 96 additions and 65 deletions.
4 changes: 2 additions & 2 deletions guide/src/conversions/traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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!(
Expand Down
15 changes: 7 additions & 8 deletions pyo3-benches/benches/bench_extract.rs
Original file line number Diff line number Diff line change
@@ -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());
});
}

Expand All @@ -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::<PyString>().unwrap();
let v = py_str.to_str().unwrap();
black_box(v);
py_str.to_str().unwrap()
});
});
}
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 10 additions & 11 deletions pyo3-benches/benches/bench_frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<ManyTypes>().unwrap());
})
}

Expand All @@ -39,33 +38,33 @@ 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::<PyList>().unwrap_err());
})
}

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::<Bound<'_, PyList>>().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::<ListOrNotList<'_>>() {
Ok(ListOrNotList::List(_list)) => panic!(),
Ok(ListOrNotList::NotList(_any)) => (),
Ok(ListOrNotList::NotList(any)) => any,
Err(_) => panic!(),
});
})
Expand Down
2 changes: 1 addition & 1 deletion src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ pub trait IntoPy<T>: Sized {
///
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| {
/// let obj: Py<PyString> = PyString::new(py, "blah").into();
/// let obj: Py<PyString> = PyString::new_bound(py, "blah").into();
///
/// // Straight from an owned reference
/// let s: &str = obj.extract(py)?;
Expand Down
4 changes: 2 additions & 2 deletions src/conversions/std/ipaddr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<IpAddr>(py).is_err());
});
}
Expand Down
16 changes: 8 additions & 8 deletions src/conversions/std/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PyObject> 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")]
Expand All @@ -30,7 +30,7 @@ impl<'a> IntoPy<PyObject> for &'a str {
impl<'a> IntoPy<Py<PyString>> for &'a str {
#[inline]
fn into_py(self, py: Python<'_>) -> Py<PyString> {
PyString::new(py, self).into()
PyString::new_bound(py, self).into()
}

#[cfg(feature = "experimental-inspect")]
Expand All @@ -44,7 +44,7 @@ impl<'a> IntoPy<Py<PyString>> 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()
}
}

Expand All @@ -65,7 +65,7 @@ impl IntoPy<PyObject> 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()
}
}

Expand All @@ -78,7 +78,7 @@ impl ToPyObject for char {
impl IntoPy<PyObject> 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")]
Expand All @@ -89,7 +89,7 @@ impl IntoPy<PyObject> for char {

impl IntoPy<PyObject> for String {
fn into_py(self, py: Python<'_>) -> PyObject {
PyString::new(py, &self).into()
PyString::new_bound(py, &self).into()
}

#[cfg(feature = "experimental-inspect")]
Expand All @@ -101,7 +101,7 @@ impl IntoPy<PyObject> for String {
impl<'a> IntoPy<PyObject> 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")]
Expand Down
2 changes: 1 addition & 1 deletion src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 4 additions & 4 deletions src/ffi/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))]
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
15 changes: 8 additions & 7 deletions src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@
//! 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(|| {
//! # #[cfg(not(feature = "nightly"))]
//! # {
//! // 💥 Unsound! 💥
//! let smuggled: &PyString = *wrapped;
//! let smuggled: &Bound<'_, PyString> = &*wrapped;
//! println!("{:?}", smuggled);
//! # }
//! });
Expand Down Expand Up @@ -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);
/// });
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
///
Expand Down Expand Up @@ -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.
/// });
Expand Down Expand Up @@ -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;
///
Expand Down
4 changes: 2 additions & 2 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl PyAny {
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| -> PyResult<()> {
/// let a = PyFloat::new(py, 0_f64);
/// let b = PyString::new(py, "zero");
/// let b = PyString::new_bound(py, "zero");
/// assert!(a.compare(b).is_err());
/// Ok(())
/// })?;
Expand Down Expand Up @@ -1075,7 +1075,7 @@ pub trait PyAnyMethods<'py> {
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| -> PyResult<()> {
/// let a = PyFloat::new(py, 0_f64);
/// let b = PyString::new(py, "zero");
/// let b = PyString::new_bound(py, "zero");
/// assert!(a.compare(b).is_err());
/// Ok(())
/// })?;
Expand Down
21 changes: 19 additions & 2 deletions src/types/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -452,6 +468,7 @@ impl IntoPy<Py<PyString>> for &'_ Py<PyString> {
}

#[cfg(test)]
#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))]
mod tests {
use super::*;
use crate::Python;
Expand Down
Loading

0 comments on commit b399737

Please sign in to comment.