From 444be3bafaec65c8293e76b6f9086d85ea3e793c Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 10 May 2024 20:28:30 +0200 Subject: [PATCH] feature gate deprecated APIs for `Python` (#4173) --- guide/src/memory.md | 3 +- src/conversion.rs | 89 +++++++++++++++-------------------- src/gil.rs | 15 ++++-- src/lib.rs | 1 + src/marker.rs | 112 ++++++++++++++------------------------------ src/prelude.rs | 1 + src/pycell.rs | 2 + 7 files changed, 89 insertions(+), 134 deletions(-) diff --git a/guide/src/memory.md b/guide/src/memory.md index 67a78d3be68..b37d83563e5 100644 --- a/guide/src/memory.md +++ b/guide/src/memory.md @@ -154,8 +154,7 @@ at the end of each loop iteration, before the `with_gil()` closure ends. When doing this, you must be very careful to ensure that once the `GILPool` is dropped you do not retain access to any owned references created after the -`GILPool` was created. Read the -[documentation for `Python::new_pool()`]({{#PYO3_DOCS_URL}}/pyo3/marker/struct.Python.html#method.new_pool) +`GILPool` was created. Read the documentation for `Python::new_pool()` for more information on safety. This memory management can also be applicable when writing extension modules. diff --git a/src/conversion.rs b/src/conversion.rs index 4df19730b43..95931d30d2e 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -1,14 +1,21 @@ //! Defines conversions between Rust and Python types. -use crate::err::{self, PyDowncastError, PyResult}; +use crate::err::PyResult; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::pyclass::boolean_struct::False; use crate::types::any::PyAnyMethods; use crate::types::PyTuple; use crate::{ - ffi, gil, Borrowed, Bound, Py, PyAny, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python, + ffi, Borrowed, Bound, Py, PyAny, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python, +}; +#[cfg(feature = "gil-refs")] +use { + crate::{ + err::{self, PyDowncastError}, + gil, + }, + std::ptr::NonNull, }; -use std::ptr::NonNull; /// Returns a borrowed pointer to a Python object. /// @@ -385,6 +392,7 @@ where /// If `T` implements `PyTryFrom`, we can convert `&PyAny` to `&T`. /// /// This trait is similar to `std::convert::TryFrom` +#[cfg(feature = "gil-refs")] #[deprecated(since = "0.21.0")] pub trait PyTryFrom<'v>: Sized + PyNativeType { /// Cast from a concrete Python object type to PyObject. @@ -416,6 +424,7 @@ pub trait PyTryFrom<'v>: Sized + PyNativeType { /// Trait implemented by Python object types that allow a checked downcast. /// This trait is similar to `std::convert::TryInto` +#[cfg(feature = "gil-refs")] #[deprecated(since = "0.21.0")] pub trait PyTryInto: Sized { /// Cast from PyObject to a concrete Python object type. @@ -506,6 +515,7 @@ impl IntoPy> for () { /// # Safety /// /// See safety notes on individual functions. +#[cfg(feature = "gil-refs")] #[deprecated(since = "0.21.0")] pub unsafe trait FromPyPointer<'p>: Sized { /// Convert from an arbitrary `PyObject`. @@ -515,12 +525,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// Implementations must ensure the object does not get freed during `'p` /// and ensure that `ptr` is of the correct type. /// Note that it must be safe to decrement the reference count of `ptr`. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr_or_opt(py, ptr)` or `Bound::from_owned_ptr_or_opt(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr_or_opt(py, ptr)` or `Bound::from_owned_ptr_or_opt(py, ptr)` instead" )] unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option<&'p Self>; /// Convert from an arbitrary `PyObject` or panic. @@ -528,12 +535,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on [`from_owned_ptr_or_opt`](#method.from_owned_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" )] unsafe fn from_owned_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { #[allow(deprecated)] @@ -544,12 +548,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on [`from_owned_ptr_or_opt`](#method.from_owned_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" )] unsafe fn from_owned_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { #[allow(deprecated)] @@ -560,12 +561,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on [`from_owned_ptr_or_opt`](#method.from_owned_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr_or_err(py, ptr)` or `Bound::from_owned_ptr_or_err(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr_or_err(py, ptr)` or `Bound::from_owned_ptr_or_err(py, ptr)` instead" )] unsafe fn from_owned_ptr_or_err(py: Python<'p>, ptr: *mut ffi::PyObject) -> PyResult<&'p Self> { #[allow(deprecated)] @@ -576,12 +574,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Implementations must ensure the object does not get freed during `'p` and avoid type confusion. - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead" )] unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option<&'p Self>; @@ -590,12 +585,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" )] unsafe fn from_borrowed_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { #[allow(deprecated)] @@ -606,12 +598,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" )] unsafe fn from_borrowed_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { #[allow(deprecated)] @@ -622,12 +611,9 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # Safety /// /// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt). - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead" - ) + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead" )] unsafe fn from_borrowed_ptr_or_err( py: Python<'p>, @@ -638,6 +624,7 @@ pub unsafe trait FromPyPointer<'p>: Sized { } } +#[cfg(feature = "gil-refs")] #[allow(deprecated)] unsafe impl<'p, T> FromPyPointer<'p> for T where diff --git a/src/gil.rs b/src/gil.rs index 0bcb8c086c0..29c4ffbe389 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -366,12 +366,12 @@ pub struct GILPool { impl GILPool { /// Creates a new [`GILPool`]. This function should only ever be called with the GIL held. /// - /// It is recommended not to use this API directly, but instead to use [`Python::new_pool`], as + /// It is recommended not to use this API directly, but instead to use `Python::new_pool`, as /// that guarantees the GIL is held. /// /// # Safety /// - /// As well as requiring the GIL, see the safety notes on [`Python::new_pool`]. + /// As well as requiring the GIL, see the safety notes on `Python::new_pool`. #[inline] pub unsafe fn new() -> GILPool { increment_gil_count(); @@ -462,6 +462,7 @@ pub unsafe fn register_decref(obj: NonNull) { /// /// # Safety /// The object must be an owned Python reference. +#[cfg(feature = "gil-refs")] pub unsafe fn register_owned(_py: Python<'_>, obj: NonNull) { debug_assert!(gil_is_acquired()); // Ignores the error in case this function called from `atexit`. @@ -507,9 +508,12 @@ fn decrement_gil_count() { mod tests { #[allow(deprecated)] use super::GILPool; - use super::{gil_is_acquired, GIL_COUNT, OWNED_OBJECTS, POOL}; + use super::{gil_is_acquired, GIL_COUNT, POOL}; use crate::types::any::PyAnyMethods; - use crate::{ffi, gil, PyObject, Python}; + use crate::{ffi, PyObject, Python}; + #[cfg(feature = "gil-refs")] + use {super::OWNED_OBJECTS, crate::gil}; + use std::ptr::NonNull; #[cfg(not(target_arch = "wasm32"))] use std::sync; @@ -518,6 +522,7 @@ mod tests { py.eval_bound("object()", None, None).unwrap().unbind() } + #[cfg(feature = "gil-refs")] fn owned_object_count() -> usize { #[cfg(debug_assertions)] let len = OWNED_OBJECTS.with(|owned_objects| owned_objects.borrow().len()); @@ -554,6 +559,7 @@ mod tests { } #[test] + #[cfg(feature = "gil-refs")] #[allow(deprecated)] fn test_owned() { Python::with_gil(|py| { @@ -580,6 +586,7 @@ mod tests { } #[test] + #[cfg(feature = "gil-refs")] #[allow(deprecated)] fn test_owned_nested() { Python::with_gil(|py| { diff --git a/src/lib.rs b/src/lib.rs index 3923257f5f3..2a40445222e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -317,6 +317,7 @@ //! [`Ungil`]: crate::marker::Ungil pub use crate::class::*; pub use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject}; +#[cfg(feature = "gil-refs")] #[allow(deprecated)] pub use crate::conversion::{FromPyPointer, PyTryFrom, PyTryInto}; pub use crate::err::{ diff --git a/src/marker.rs b/src/marker.rs index ea794856d66..b6821deb036 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -130,6 +130,7 @@ use crate::version::PythonVersionInfo; use crate::PyNativeType; use crate::{ffi, Bound, IntoPy, Py, PyObject, PyTypeInfo}; #[allow(deprecated)] +#[cfg(feature = "gil-refs")] use crate::{gil::GILPool, FromPyPointer}; use std::ffi::{CStr, CString}; use std::marker::PhantomData; @@ -354,36 +355,9 @@ pub use nightly::Ungil; /// # Releasing and freeing memory /// /// The [`Python`] type can be used to create references to variables owned by the Python -/// interpreter, using functions such as `Python::eval` and `PyModule::import`. These -/// references are tied to a [`GILPool`] whose references are not cleared until it is dropped. -/// This can cause apparent "memory leaks" if it is kept around for a long time. -/// -/// ```rust -/// use pyo3::prelude::*; -/// use pyo3::types::PyString; -/// -/// # fn main () -> PyResult<()> { -/// Python::with_gil(|py| -> PyResult<()> { -/// for _ in 0..10 { -/// let hello = py.eval_bound("\"Hello World!\"", None, None)?.downcast_into::()?; -/// println!("Python says: {}", hello.to_cow()?); -/// // Normally variables in a loop scope are dropped here, but `hello` is a reference to -/// // something owned by the Python interpreter. Dropping this reference does nothing. -/// } -/// Ok(()) -/// }) -/// // This is where the `hello`'s reference counts start getting decremented. -/// # } -/// ``` -/// -/// The variable `hello` is dropped at the end of each loop iteration, but the lifetime of the -/// pointed-to memory is bound to [`Python::with_gil`]'s [`GILPool`] which will not be dropped until -/// the end of [`Python::with_gil`]'s scope. Only then is each `hello`'s Python reference count -/// decreased. This means that at the last line of the example there are 10 copies of `hello` in -/// Python's memory, not just one at a time as we might expect from Rust's [scoping rules]. +/// interpreter, using functions such as [`Python::eval_bound`] and [`PyModule::import_bound`]. /// -/// See the [Memory Management] chapter of the guide for more information about how PyO3 uses -/// [`GILPool`] to manage memory. +/// See the [Memory Management] chapter of the guide for more information about how PyO3 manages memory. /// /// [scoping rules]: https://doc.rust-lang.org/stable/book/ch04-01-what-is-ownership.html#ownership-rules /// [`Py::clone_ref`]: crate::Py::clone_ref @@ -874,12 +848,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead" )] pub unsafe fn from_owned_ptr(self, ptr: *mut ffi::PyObject) -> &'py T where @@ -897,12 +869,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr_or_err(py, ptr)` or `Bound::from_owned_ptr_or_err(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr_or_err(py, ptr)` or `Bound::from_owned_ptr_or_err(py, ptr)` instead" )] pub unsafe fn from_owned_ptr_or_err(self, ptr: *mut ffi::PyObject) -> PyResult<&'py T> where @@ -920,12 +890,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_owned_ptr_or_opt(py, ptr)` or `Bound::from_owned_ptr_or_opt(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_owned_ptr_or_opt(py, ptr)` or `Bound::from_owned_ptr_or_opt(py, ptr)` instead" )] pub unsafe fn from_owned_ptr_or_opt(self, ptr: *mut ffi::PyObject) -> Option<&'py T> where @@ -942,12 +910,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead" )] pub unsafe fn from_borrowed_ptr(self, ptr: *mut ffi::PyObject) -> &'py T where @@ -964,12 +930,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead" )] pub unsafe fn from_borrowed_ptr_or_err(self, ptr: *mut ffi::PyObject) -> PyResult<&'py T> where @@ -986,12 +950,10 @@ impl<'py> Python<'py> { /// /// Callers must ensure that ensure that the cast is valid. #[allow(clippy::wrong_self_convention, deprecated)] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead" )] pub unsafe fn from_borrowed_ptr_or_opt(self, ptr: *mut ffi::PyObject) -> Option<&'py T> where @@ -1103,12 +1065,10 @@ impl<'py> Python<'py> { /// /// [`.python()`]: crate::GILPool::python #[inline] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "code not using the GIL Refs API can safely remove use of `Python::new_pool`" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "code not using the GIL Refs API can safely remove use of `Python::new_pool`" )] #[allow(deprecated)] pub unsafe fn new_pool(self) -> GILPool { @@ -1172,12 +1132,10 @@ impl Python<'_> { /// }); /// ``` #[inline] - #[cfg_attr( - not(feature = "gil-refs"), - deprecated( - since = "0.21.0", - note = "code not using the GIL Refs API can safely remove use of `Python::with_pool`" - ) + #[cfg(feature = "gil-refs")] + #[deprecated( + since = "0.21.0", + note = "code not using the GIL Refs API can safely remove use of `Python::with_pool`" )] #[allow(deprecated)] pub fn with_pool(&self, f: F) -> R diff --git a/src/prelude.rs b/src/prelude.rs index ef42b2706e9..7aa45f6ccc2 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -9,6 +9,7 @@ //! ``` pub use crate::conversion::{FromPyObject, IntoPy, ToPyObject}; +#[cfg(feature = "gil-refs")] #[allow(deprecated)] pub use crate::conversion::{PyTryFrom, PyTryInto}; pub use crate::err::{PyErr, PyResult}; diff --git a/src/pycell.rs b/src/pycell.rs index 215ed7bba66..dc5ccc45ea1 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -518,6 +518,7 @@ impl ToPyObject for &PyCell { } } +#[cfg(feature = "gil-refs")] #[allow(deprecated)] impl AsRef for PyCell { fn as_ref(&self) -> &PyAny { @@ -528,6 +529,7 @@ impl AsRef for PyCell { } } +#[cfg(feature = "gil-refs")] #[allow(deprecated)] impl Deref for PyCell { type Target = PyAny;