fixes #3325 -- mark AsPyPointer and IntoPyPointer as unsafe trait

guide/src/migration.md

@@ -62,6 +62,10 @@ fn add(a: u64, b: u64) -> u64 {
 }
 ```
 
+### `AsPyPointer` and `IntoPyPointer` now `unsafe` traits
+
+The traits `AsPyPointer` and `IntoPyPointer` are now `unsafe trait`, meaning any external implementation of these traits must be marked as `unsafe impl`, and ensure they uphold the invariants of returning valid pointers.
+
 ## from 0.18.* to 0.19
 
 ### Access to `Python` inside `__traverse__` implementations are now forbidden

newsfragments/3325.changed.md

@@ -0,0 +1 @@
+`AsPyPointer` and `IntoPyPointer` are now `unsafe trait`.

src/conversion.rs

@@ -35,7 +35,7 @@ use std::ptr::NonNull;
 ///
 /// # Safety
 ///
-/// It is your responsibility to make sure that the underlying Python object is not dropped too
+/// For callers, it is your responsibility to make sure that the underlying Python object is not dropped too
 /// early. For example, the following code will cause undefined behavior:
 ///
 /// ```rust,no_run
@@ -58,7 +58,9 @@ use std::ptr::NonNull;
 /// and the Python object is dropped immediately after the `0xabad1dea_u32.into_py(py).as_ptr()`
 /// expression is evaluated. To fix the problem, bind Python object to a local variable like earlier
 /// to keep the Python object alive until the end of its scope.
-pub trait AsPyPointer {
+///
+/// Implementors must ensure this returns a valid pointer to a Python object.
+pub unsafe trait AsPyPointer {
     /// Returns the underlying FFI pointer as a borrowed pointer.
     fn as_ptr(&self) -> *mut ffi::PyObject;
 }
@@ -88,15 +90,18 @@ pub trait AsPyPointer {
 ///     unsafe { ffi::Py_DECREF(ptr) };
 /// });
 /// ```
-pub trait IntoPyPointer {
+/// # Safety
+///
+/// Implementors must ensure this returns a valid pointer to a Python object.
+pub unsafe trait IntoPyPointer {
     /// Returns the underlying FFI pointer as an owned pointer.
     ///
     /// If `self` has ownership of the underlying pointer, it will "steal" ownership of it.
     fn into_ptr(self) -> *mut ffi::PyObject;
 }
 
 /// Convert `None` into a null pointer.
-impl<T> AsPyPointer for Option<T>
+unsafe impl<T> AsPyPointer for Option<T>
 where
     T: AsPyPointer,
 {
@@ -108,7 +113,7 @@ where
 }
 
 /// Convert `None` into a null pointer.
-impl<T> IntoPyPointer for Option<T>
+unsafe impl<T> IntoPyPointer for Option<T>
 where
     T: IntoPyPointer,
 {
@@ -118,7 +123,7 @@ where
     }
 }
 
-impl<'a, T> IntoPyPointer for &'a T
+unsafe impl<'a, T> IntoPyPointer for &'a T
 where
     T: AsPyPointer,
 {

src/instance.rs

@@ -897,15 +897,15 @@ impl<T> IntoPy<PyObject> for Py<T> {
     }
 }
 
-impl<T> AsPyPointer for Py<T> {
+unsafe impl<T> AsPyPointer for Py<T> {
     /// Gets the underlying FFI pointer, returns a borrowed pointer.
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
         self.0.as_ptr()
     }
 }
 
-impl<T> IntoPyPointer for Py<T> {
+unsafe impl<T> IntoPyPointer for Py<T> {
     /// Gets the underlying FFI pointer, returns a owned pointer.
     #[inline]
     #[must_use]

src/pycell.rs

@@ -528,7 +528,7 @@ impl<T: PyClassImpl> PyCell<T> {
 unsafe impl<T: PyClassImpl> PyLayout<T> for PyCell<T> {}
 impl<T: PyClass> PySizedLayout<T> for PyCell<T> {}
 
-impl<T: PyClass> AsPyPointer for PyCell<T> {
+unsafe impl<T: PyClass> AsPyPointer for PyCell<T> {
     fn as_ptr(&self) -> *mut ffi::PyObject {
         (self as *const _) as *mut _
     }
@@ -725,7 +725,7 @@ impl<'a, T: PyClass> std::convert::TryFrom<&'a PyCell<T>> for crate::PyRef<'a, T
     }
 }
 
-impl<'a, T: PyClass> AsPyPointer for PyRef<'a, T> {
+unsafe impl<'a, T: PyClass> AsPyPointer for PyRef<'a, T> {
     fn as_ptr(&self) -> *mut ffi::PyObject {
         self.inner.as_ptr()
     }
@@ -816,7 +816,7 @@ impl<T: PyClass<Frozen = False>> IntoPy<PyObject> for PyRefMut<'_, T> {
     }
 }
 
-impl<'a, T: PyClass<Frozen = False>> AsPyPointer for PyRefMut<'a, T> {
+unsafe impl<'a, T: PyClass<Frozen = False>> AsPyPointer for PyRefMut<'a, T> {
     fn as_ptr(&self) -> *mut ffi::PyObject {
         self.inner.as_ptr()
     }

src/types/any.rs

@@ -36,7 +36,7 @@ use std::os::raw::c_int;
 #[repr(transparent)]
 pub struct PyAny(UnsafeCell<ffi::PyObject>);
 
-impl crate::AsPyPointer for PyAny {
+unsafe impl crate::AsPyPointer for PyAny {
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
         self.0.get()

src/types/mod.rs

@@ -144,7 +144,7 @@ macro_rules! pyobject_native_type_named (
             }
         }
 
-        impl<$($generics,)*> $crate::AsPyPointer for $name {
+        unsafe impl<$($generics,)*> $crate::AsPyPointer for $name {
             /// Gets the underlying FFI pointer, returns a borrowed pointer.
             #[inline]
             fn as_ptr(&self) -> *mut $crate::ffi::PyObject {