Merge pull request #3358 from alex/ptr-unsafe-trait

fixes #3325 -- mark `AsPyPointer` as `unsafe trait`

guide/src/migration.md

@@ -66,6 +66,10 @@ fn add(a: u64, b: u64) -> u64 {
 
 The trait `IntoPyPointer`, which provided the `into_ptr` method on many types, has been removed. `into_ptr` is now available as an inherent method on all types that previously implemented this trait.
 
+### `AsPyPointer` now `unsafe` trait
+
+The trait `AsPyPointer` is now `unsafe trait`, meaning any external implementation of it must be marked as `unsafe impl`, and ensure that they uphold the invariant of returning valid pointers.
+
 ## from 0.18.* to 0.19
 
 ### Access to `Python` inside `__traverse__` implementations are now forbidden

newsfragments/3358.changed.md

@@ -0,0 +1 @@
+`AsPyPointer` is now `unsafe trait`.

src/conversion.rs

@@ -34,7 +34,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
@@ -56,13 +56,15 @@ 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, which borrows a reference count from `&self`.
+pub unsafe trait AsPyPointer {
     /// Returns the underlying FFI pointer as a borrowed pointer.
     fn as_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,
 {

src/instance.rs

@@ -932,7 +932,7 @@ impl<T> IntoPy<PyObject> for &'_ Py<T> {
     }
 }
 
-impl<T> crate::AsPyPointer for Py<T> {
+unsafe impl<T> crate::AsPyPointer for Py<T> {
     /// Gets the underlying FFI pointer, returns a borrowed pointer.
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {

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 _
     }
@@ -756,7 +756,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()
     }
@@ -879,7 +879,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 AsPyPointer for PyAny {
+unsafe impl AsPyPointer for PyAny {
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
         self.0.get()

src/types/mod.rs

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