From df5aa778009fe8dbcfdf051b4ba2c01b5026a3a7 Mon Sep 17 00:00:00 2001
From: Alex Gaynor <alex.gaynor@gmail.com>
Date: Sun, 30 Jul 2023 14:40:52 -0400
Subject: [PATCH] fixes #3325 -- mark `AsPyPointer` as `unsafe trait`

---
 guide/src/migration.md        | 4 ++++
 newsfragments/3358.changed.md | 1 +
 src/conversion.rs             | 8 +++++---
 src/instance.rs               | 2 +-
 src/pycell.rs                 | 6 +++---
 src/types/any.rs              | 2 +-
 src/types/mod.rs              | 2 +-
 7 files changed, 16 insertions(+), 9 deletions(-)
 create mode 100644 newsfragments/3358.changed.md

diff --git a/guide/src/migration.md b/guide/src/migration.md
index 7e5a930a836..f22b14813ea 100644
--- a/guide/src/migration.md
+++ b/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
diff --git a/newsfragments/3358.changed.md b/newsfragments/3358.changed.md
new file mode 100644
index 00000000000..9cc8176b674
--- /dev/null
+++ b/newsfragments/3358.changed.md
@@ -0,0 +1 @@
+`AsPyPointer` is now `unsafe trait`.
diff --git a/src/conversion.rs b/src/conversion.rs
index 6fc7a6bf328..51a8dd28f08 100644
--- a/src/conversion.rs
+++ b/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,
 {
diff --git a/src/instance.rs b/src/instance.rs
index f7529f2420a..8947c5c87e8 100644
--- a/src/instance.rs
+++ b/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 {
diff --git a/src/pycell.rs b/src/pycell.rs
index 6fb5b1d80ce..c9ba445633e 100644
--- a/src/pycell.rs
+++ b/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()
     }
diff --git a/src/types/any.rs b/src/types/any.rs
index 2ff6527543b..6e7d48360ec 100644
--- a/src/types/any.rs
+++ b/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()
diff --git a/src/types/mod.rs b/src/types/mod.rs
index 4d8b0513adf..172c1d59310 100644
--- a/src/types/mod.rs
+++ b/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 {