Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change return type of intern! macro to &Bound<PyString> #3781

Merged
merged 3 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ The following sections are laid out in this order.

To make the transition for the PyO3 ecosystem away from the GIL Refs API as smooth as possible, in PyO3 0.21 no APIs consuming or producing GIL Refs have been altered. Instead, variants using `Bound<T>` smart pointers have been introduced, for example `PyTuple::new_bound` which returns `Bound<PyTuple>` is the replacement form of `PyTuple::new`. The GIL Ref APIs have been deprecated, but to make migration easier it is possible to disable these deprecation warnings by enabling the `gil-refs` feature.

> The one single exception where an existing API was changed in-place is the `pyo3::intern!` macro. Almost all uses of this macro did not need to update code to account it changing to return `&Bound<PyString>` immediately, and adding an `intern_bound!` replacement was perceived as adding more work for users.

It is recommended that users do this as a first step of updating to PyO3 0.21 so that the deprecation warnings do not get in the way of resolving the rest of the migration steps.

Before:
Expand All @@ -40,7 +42,6 @@ After:
pyo3 = { version = "0.21", features = ["gil-refs"] }
```


### `PyTypeInfo` and `PyTryFrom` have been adjusted

The `PyTryFrom` trait has aged poorly, its [`try_from`] method now conflicts with `try_from` in the 2021 edition prelude. A lot of its functionality was also duplicated with `PyTypeInfo`.
Expand Down Expand Up @@ -274,7 +275,6 @@ 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.

## from 0.19.* to 0.20
Expand Down
10 changes: 8 additions & 2 deletions src/impl_/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ use std::{

use crate::{
coroutine::{cancel::ThrowCallback, Coroutine},
instance::Bound,
pyclass::boolean_struct::False,
types::PyString,
IntoPy, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult, Python,
};

pub fn new_coroutine<F, T, E>(
name: &PyString,
name: &Bound<'_, PyString>,
qualname_prefix: Option<&'static str>,
throw_callback: Option<ThrowCallback>,
future: F,
Expand All @@ -22,7 +23,12 @@ where
T: IntoPy<PyObject>,
E: Into<PyErr>,
{
Coroutine::new(Some(name.into()), qualname_prefix, throw_callback, future)
Coroutine::new(
Some(name.clone().into()),
qualname_prefix,
throw_callback,
future,
)
}

fn get_ptr<T: PyClass>(obj: &Py<T>) -> *mut T {
Expand Down
4 changes: 2 additions & 2 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'py> Bound<'py, PyAny> {
///
/// # Safety
///
/// `ptr`` must be a valid pointer to a Python object.
/// `ptr` must be a valid pointer to a Python object.
pub(crate) unsafe fn from_owned_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self {
Self(py, ManuallyDrop::new(Py::from_owned_ptr(py, ptr)))
}
Expand All @@ -75,7 +75,7 @@ impl<'py> Bound<'py, PyAny> {
///
/// # Safety
///
/// `ptr`` must be a valid pointer to a Python object, or NULL.
/// `ptr` must be a valid pointer to a Python object, or NULL.
pub(crate) unsafe fn from_owned_ptr_or_opt(
py: Python<'py>,
ptr: *mut ffi::PyObject,
Expand Down
8 changes: 4 additions & 4 deletions src/sync.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Synchronization mechanisms based on the Python GIL.
use crate::{types::PyString, types::PyType, Py, PyResult, PyVisit, Python};
use crate::{instance::Bound, types::PyString, types::PyType, Py, PyResult, PyVisit, Python};
use std::cell::UnsafeCell;

/// Value with concurrent access protected by the GIL.
Expand Down Expand Up @@ -259,10 +259,10 @@ impl Interned {

/// Gets or creates the interned `str` value.
#[inline]
pub fn get<'py>(&'py self, py: Python<'py>) -> &'py PyString {
pub fn get<'py>(&self, py: Python<'py>) -> &Bound<'py, PyString> {
self.1
.get_or_init(py, || PyString::intern(py, self.0).into())
.as_ref(py)
.get_or_init(py, || PyString::intern_bound(py, self.0).into())
.bind(py)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ pub(crate) mod float;
mod frame;
pub(crate) mod frozenset;
mod function;
mod iterator;
pub(crate) mod iterator;
pub(crate) mod list;
pub(crate) mod mapping;
mod memoryview;
Expand Down
4 changes: 2 additions & 2 deletions src/types/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,11 +660,11 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> {
}
}

fn __all__(py: Python<'_>) -> &PyString {
fn __all__(py: Python<'_>) -> &Bound<'_, PyString> {
intern!(py, "__all__")
}

fn __name__(py: Python<'_>) -> &PyString {
fn __name__(py: Python<'_>) -> &Bound<'_, PyString> {
intern!(py, "__name__")
}

Expand Down
48 changes: 39 additions & 9 deletions src/types/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use crate::exceptions::PyUnicodeDecodeError;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::instance::Borrowed;
use crate::py_result_ext::PyResultExt;
use crate::types::any::PyAnyMethods;
use crate::types::bytes::PyBytesMethods;
use crate::types::PyBytes;
Expand Down Expand Up @@ -159,6 +160,18 @@ impl PyString {
}
}

/// Deprecated form of [`PyString::intern_bound`].
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyString::intern` will be replaced by `PyString::intern_bound` in a future PyO3 version"
)
)]
pub fn intern<'py>(py: Python<'py>, s: &str) -> &'py Self {
Self::intern_bound(py, s).into_gil_ref()
}

/// Intern the given string
///
/// This will return a reference to the same Python string object if called repeatedly with the same string.
Expand All @@ -167,29 +180,46 @@ impl PyString {
/// temporary Python string object and is thereby slower than [`PyString::new`].
///
/// Panics if out of memory.
pub fn intern<'p>(py: Python<'p>, s: &str) -> &'p PyString {
pub fn intern_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 {
let mut ob = ffi::PyUnicode_FromStringAndSize(ptr, len);
if !ob.is_null() {
ffi::PyUnicode_InternInPlace(&mut ob);
}
py.from_owned_ptr(ob)
ob.assume_owned(py).downcast_into_unchecked()
}
}

/// Deprecated form of [`PyString::from_object_bound`].
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyString::from_object` will be replaced by `PyString::from_object_bound` in a future PyO3 version"
)
)]
pub fn from_object<'py>(src: &'py PyAny, encoding: &str, errors: &str) -> PyResult<&'py Self> {
Self::from_object_bound(&src.as_borrowed(), encoding, errors).map(Bound::into_gil_ref)
}

/// Attempts to create a Python string from a Python [bytes-like object].
///
/// [bytes-like object]: (https://docs.python.org/3/glossary.html#term-bytes-like-object).
pub fn from_object<'p>(src: &'p PyAny, encoding: &str, errors: &str) -> PyResult<&'p PyString> {
pub fn from_object_bound<'py>(
src: &Bound<'py, PyAny>,
encoding: &str,
errors: &str,
) -> PyResult<Bound<'py, PyString>> {
unsafe {
src.py()
.from_owned_ptr_or_err(ffi::PyUnicode_FromEncodedObject(
src.as_ptr(),
encoding.as_ptr() as *const c_char,
errors.as_ptr() as *const c_char,
))
ffi::PyUnicode_FromEncodedObject(
src.as_ptr(),
encoding.as_ptr() as *const c_char,
errors.as_ptr() as *const c_char,
)
.assume_owned_or_err(src.py())
.downcast_into_unchecked()
}
}

Expand Down
Loading