Merge pull request #3781 from davidhewitt/intern-bound

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

guide/src/migration.md

@@ -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:
@@ -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`.
@@ -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

src/impl_/coroutine.rs

@@ -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,
@@ -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 {

src/instance.rs

@@ -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)))
     }
@@ -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,

src/sync.rs

@@ -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.
@@ -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)
     }
 }
 

src/types/mod.rs

@@ -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;

src/types/module.rs

@@ -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__")
 }
 

src/types/string.rs

@@ -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;
@@ -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.
@@ -167,7 +180,7 @@ 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 {
@@ -175,21 +188,38 @@ impl PyString {
             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()
         }
     }