Skip to content

Commit

Permalink
feature gate deprecated APIs for Python (#4173)
Browse files Browse the repository at this point in the history
  • Loading branch information
Icxolu committed May 10, 2024
1 parent 1e8e09d commit 444be3b
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 134 deletions.
3 changes: 1 addition & 2 deletions guide/src/memory.md
Expand Up @@ -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.
Expand Down
89 changes: 38 additions & 51 deletions 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.
///
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<T>: Sized {
/// Cast from PyObject to a concrete Python object type.
Expand Down Expand Up @@ -506,6 +515,7 @@ impl IntoPy<Py<PyTuple>> 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`.
Expand All @@ -515,25 +525,19 @@ 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.
///
/// # 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)]
Expand All @@ -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)]
Expand All @@ -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)]
Expand All @@ -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>;
Expand All @@ -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)]
Expand All @@ -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)]
Expand All @@ -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>,
Expand All @@ -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
Expand Down
15 changes: 11 additions & 4 deletions src/gil.rs
Expand Up @@ -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();
Expand Down Expand Up @@ -462,6 +462,7 @@ pub unsafe fn register_decref(obj: NonNull<ffi::PyObject>) {
///
/// # Safety
/// The object must be an owned Python reference.
#[cfg(feature = "gil-refs")]
pub unsafe fn register_owned(_py: Python<'_>, obj: NonNull<ffi::PyObject>) {
debug_assert!(gil_is_acquired());
// Ignores the error in case this function called from `atexit`.
Expand Down Expand Up @@ -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;
Expand All @@ -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());
Expand Down Expand Up @@ -554,6 +559,7 @@ mod tests {
}

#[test]
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
fn test_owned() {
Python::with_gil(|py| {
Expand All @@ -580,6 +586,7 @@ mod tests {
}

#[test]
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
fn test_owned_nested() {
Python::with_gil(|py| {
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Expand Up @@ -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::{
Expand Down

0 comments on commit 444be3b

Please sign in to comment.