Deprecate py.from_owned_ptr methods (#3875)

* Deprecate `py.from_owned_ptr` methods * Refactor PyString.to_str Co-authored-by: David Hewitt <[email protected]> --------- Co-authored-by: David Hewitt <[email protected]>
PyO3 · Feb 22, 2024 · 5ddcd46 · 5ddcd46
1 parent 885883b
commit 5ddcd46
Show file tree

Hide file tree

Showing 12 changed files with 84 additions and 30 deletions.
diff --git a/src/conversion.rs b/src/conversion.rs
@@ -434,29 +434,60 @@ 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 = "part of the deprecated GIL Ref API; to migrate 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 = "part of the deprecated GIL Ref API; to migrate 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)]
         Self::from_owned_ptr_or_opt(py, ptr).unwrap_or_else(|| err::panic_after_error(py))
     }
     /// 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 = "part of the deprecated GIL Ref API; to migrate 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)]
         Self::from_owned_ptr_or_panic(py, ptr)
     }
     /// Convert from an arbitrary `PyObject`.
     ///
     /// # 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 = "part of the deprecated GIL Ref API; to migrate 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)]
         Self::from_owned_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::fetch(py))
     }
     /// Convert from an arbitrary borrowed `PyObject`.

diff --git a/src/exceptions.rs b/src/exceptions.rs
@@ -43,6 +43,7 @@ macro_rules! impl_exception_boilerplate {
         impl ::std::error::Error for $name {
             fn source(&self) -> ::std::option::Option<&(dyn ::std::error::Error + 'static)> {
                 unsafe {
+                    #[allow(deprecated)]
                     let cause: &$crate::exceptions::PyBaseException = self
                         .py()
                         .from_owned_ptr_or_opt($crate::ffi::PyException_GetCause(self.as_ptr()))?;

diff --git a/src/ffi/tests.rs b/src/ffi/tests.rs
@@ -5,7 +5,7 @@ use crate::Python;
 #[cfg(not(Py_LIMITED_API))]
 use crate::{
     types::{PyDict, PyString},
-    IntoPy, Py, PyAny,
+    Bound, IntoPy, Py, PyAny,
 };
 #[cfg(not(any(Py_3_12, Py_LIMITED_API)))]
 use libc::wchar_t;
@@ -16,9 +16,9 @@ use libc::wchar_t;
 fn test_datetime_fromtimestamp() {
     Python::with_gil(|py| {
         let args: Py<PyAny> = (100,).into_py(py);
-        let dt: &PyAny = unsafe {
+        let dt = unsafe {
             PyDateTime_IMPORT();
-            py.from_owned_ptr(PyDateTime_FromTimestamp(args.as_ptr()))
+            Bound::from_owned_ptr(py, PyDateTime_FromTimestamp(args.as_ptr()))
         };
         let locals = PyDict::new_bound(py);
         locals.set_item("dt", dt).unwrap();
@@ -37,9 +37,9 @@ fn test_datetime_fromtimestamp() {
 fn test_date_fromtimestamp() {
     Python::with_gil(|py| {
         let args: Py<PyAny> = (100,).into_py(py);
-        let dt: &PyAny = unsafe {
+        let dt = unsafe {
             PyDateTime_IMPORT();
-            py.from_owned_ptr(PyDate_FromTimestamp(args.as_ptr()))
+            Bound::from_owned_ptr(py, PyDate_FromTimestamp(args.as_ptr()))
         };
         let locals = PyDict::new_bound(py);
         locals.set_item("dt", dt).unwrap();

diff --git a/src/instance.rs b/src/instance.rs
@@ -488,7 +488,10 @@ impl<'py, T> Bound<'py, T> {
     where
         T: HasPyGilRef,
     {
-        unsafe { self.py().from_owned_ptr(self.into_ptr()) }
+        #[allow(deprecated)]
+        unsafe {
+            self.py().from_owned_ptr(self.into_ptr())
+        }
     }
 }
 
@@ -974,7 +977,10 @@ where
         )
     )]
     pub fn into_ref(self, py: Python<'_>) -> &T::AsRefTarget {
-        unsafe { py.from_owned_ptr(self.into_ptr()) }
+        #[allow(deprecated)]
+        unsafe {
+            py.from_owned_ptr(self.into_ptr())
+        }
     }
 }
 

diff --git a/src/marker.rs b/src/marker.rs
@@ -885,10 +885,18 @@ impl<'py> Python<'py> {
     ///
     /// Callers must ensure that ensure that the cast is valid.
     #[allow(clippy::wrong_self_convention)]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "part of the deprecated GIL Ref API; to migrate use `Py::from_owned_ptr(py, ptr)` or `Bound::from_owned_ptr(py, ptr)` instead"
+        )
+    )]
     pub unsafe fn from_owned_ptr<T>(self, ptr: *mut ffi::PyObject) -> &'py T
     where
         T: FromPyPointer<'py>,
     {
+        #[allow(deprecated)]
         FromPyPointer::from_owned_ptr(self, ptr)
     }
 
@@ -901,10 +909,18 @@ impl<'py> Python<'py> {
     ///
     /// Callers must ensure that ensure that the cast is valid.
     #[allow(clippy::wrong_self_convention)]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "part of the deprecated GIL Ref API; to migrate 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<T>(self, ptr: *mut ffi::PyObject) -> PyResult<&'py T>
     where
         T: FromPyPointer<'py>,
     {
+        #[allow(deprecated)]
         FromPyPointer::from_owned_ptr_or_err(self, ptr)
     }
 
@@ -917,10 +933,18 @@ impl<'py> Python<'py> {
     ///
     /// Callers must ensure that ensure that the cast is valid.
     #[allow(clippy::wrong_self_convention)]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "part of the deprecated GIL Ref API; to migrate 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<T>(self, ptr: *mut ffi::PyObject) -> Option<&'py T>
     where
         T: FromPyPointer<'py>,
     {
+        #[allow(deprecated)]
         FromPyPointer::from_owned_ptr_or_opt(self, ptr)
     }
 

diff --git a/src/pycell.rs b/src/pycell.rs
@@ -296,6 +296,7 @@ impl<T: PyClass> PyCell<T> {
         unsafe {
             let initializer = value.into();
             let self_ = initializer.create_cell(py)?;
+            #[allow(deprecated)]
             FromPyPointer::from_owned_ptr_or_err(py, self_ as _)
         }
     }

diff --git a/src/types/bytearray.rs b/src/types/bytearray.rs
@@ -112,10 +112,7 @@ impl PyByteArray {
         )
     )]
     pub fn from(src: &PyAny) -> PyResult<&PyByteArray> {
-        unsafe {
-            src.py()
-                .from_owned_ptr_or_err(ffi::PyByteArray_FromObject(src.as_ptr()))
-        }
+        PyByteArray::from_bound(&src.as_borrowed()).map(Bound::into_gil_ref)
     }
 
     /// Creates a new Python `bytearray` object from another Python object that

diff --git a/src/types/complex.rs b/src/types/complex.rs
@@ -141,11 +141,7 @@ mod not_limited_impls {
     impl<'py> Neg for &'py PyComplex {
         type Output = &'py PyComplex;
         fn neg(self) -> &'py PyComplex {
-            unsafe {
-                let val = (*self.as_ptr().cast::<ffi::PyComplexObject>()).cval;
-                self.py()
-                    .from_owned_ptr(ffi::PyComplex_FromCComplex(ffi::_Py_c_neg(val)))
-            }
+            (-self.as_borrowed()).into_gil_ref()
         }
     }
 

diff --git a/src/types/memoryview.rs b/src/types/memoryview.rs
@@ -19,10 +19,7 @@ impl PyMemoryView {
         )
     )]
     pub fn from(src: &PyAny) -> PyResult<&PyMemoryView> {
-        unsafe {
-            src.py()
-                .from_owned_ptr_or_err(ffi::PyMemoryView_FromObject(src.as_ptr()))
-        }
+        PyMemoryView::from_bound(&src.as_borrowed()).map(Bound::into_gil_ref)
     }
 
     /// Creates a new Python `memoryview` object from another Python object that

diff --git a/src/types/module.rs b/src/types/module.rs
@@ -41,7 +41,10 @@ impl PyModule {
     pub fn new<'p>(py: Python<'p>, name: &str) -> PyResult<&'p PyModule> {
         // Could use PyModule_NewObject, but it doesn't exist on PyPy.
         let name = CString::new(name)?;
-        unsafe { py.from_owned_ptr_or_err(ffi::PyModule_New(name.as_ptr())) }
+        #[allow(deprecated)]
+        unsafe {
+            py.from_owned_ptr_or_err(ffi::PyModule_New(name.as_ptr()))
+        }
     }
 
     /// Imports the Python module with the specified name.
@@ -67,7 +70,10 @@ impl PyModule {
         N: IntoPy<Py<PyString>>,
     {
         let name: Py<PyString> = name.into_py(py);
-        unsafe { py.from_owned_ptr_or_err(ffi::PyImport_Import(name.as_ptr())) }
+        #[allow(deprecated)]
+        unsafe {
+            py.from_owned_ptr_or_err(ffi::PyImport_Import(name.as_ptr()))
+        }
     }
 
     /// Creates and loads a module named `module_name`,
@@ -146,6 +152,7 @@ impl PyModule {
                 return Err(PyErr::fetch(py));
             }
 
+            #[allow(deprecated)]
             <&PyModule as FromPyObject>::extract(py.from_owned_ptr_or_err(mptr)?)
         }
     }

diff --git a/src/types/slice.rs b/src/types/slice.rs
@@ -78,10 +78,7 @@ impl PySlice {
         )
     )]
     pub fn full(py: Python<'_>) -> &PySlice {
-        unsafe {
-            let ptr = ffi::PySlice_New(ffi::Py_None(), ffi::Py_None(), ffi::Py_None());
-            py.from_owned_ptr(ptr)
-        }
+        PySlice::full_bound(py).into_gil_ref()
     }
 
     /// Constructs a new full slice that is equivalent to `::`.

diff --git a/src/types/string.rs b/src/types/string.rs
@@ -241,10 +241,7 @@ impl PyString {
 
         #[cfg(not(any(Py_3_10, not(Py_LIMITED_API))))]
         {
-            let bytes = unsafe {
-                self.py()
-                    .from_owned_ptr_or_err::<PyBytes>(ffi::PyUnicode_AsUTF8String(self.as_ptr()))
-            }?;
+            let bytes = self.as_borrowed().encode_utf8()?.into_gil_ref();
             Ok(unsafe { std::str::from_utf8_unchecked(bytes.as_bytes()) })
         }
     }