From 255d9bacce64c4a0712e75c1c4635d0a034d7e51 Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Thu, 19 Jan 2023 21:05:04 +0000
Subject: [PATCH] tidy up implementation of pyclass `tp_dealloc`

---
 src/impl_/pyclass.rs | 9 +--------
 src/pycell.rs        | 8 ++++----
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs
index 6d1a9373..a2671748 100644
--- a/src/impl_/pyclass.rs
+++ b/src/impl_/pyclass.rs
@@ -928,14 +928,7 @@ impl<T: PyClass> PyClassBaseType for T {
 
 /// Implementation of tp_dealloc for all pyclasses
 pub(crate) unsafe extern "C" fn tp_dealloc<T: PyClass>(obj: *mut ffi::PyObject) {
-    /// A wrapper because PyCellLayout::tp_dealloc currently takes the py argument last
-    /// (which is different to the rest of the trampolines which take py first)
-    #[inline]
-    unsafe fn trampoline_dealloc_wrapper<T: PyClass>(py: Python<'_>, slf: *mut ffi::PyObject) {
-        T::Layout::tp_dealloc(slf, py);
-    }
-    // TODO change argument order in PyCellLayout::tp_dealloc so this wrapper isn't needed.
-    crate::impl_::trampoline::dealloc(obj, trampoline_dealloc_wrapper::<T>)
+    crate::impl_::trampoline::dealloc(obj, T::Layout::tp_dealloc)
 }
 
 pub(crate) unsafe extern "C" fn get_sequence_item_from_mapping(
diff --git a/src/pycell.rs b/src/pycell.rs
index 18942f44..0be39ac6 100644
--- a/src/pycell.rs
+++ b/src/pycell.rs
@@ -854,7 +854,7 @@ pub trait PyCellLayout<T>: PyLayout<T> {
     /// # Safety
     /// - slf must be a valid pointer to an instance of a T or a subclass.
     /// - slf must not be used after this call (as it will be freed).
-    unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python<'_>);
+    unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject);
 }
 
 impl<T, U> PyCellLayout<T> for PyCellBase<U>
@@ -863,7 +863,7 @@ where
     T: PyTypeInfo,
 {
     fn ensure_threadsafe(&self) {}
-    unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python<'_>) {
+    unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) {
         // For `#[pyclass]` types which inherit from PyAny, we can just call tp_free
         if T::type_object_raw(py) == &mut PyBaseObject_Type {
             return get_tp_free(ffi::Py_TYPE(slf))(slf as _);
@@ -892,13 +892,13 @@ where
         self.contents.thread_checker.ensure();
         self.ob_base.ensure_threadsafe();
     }
-    unsafe fn tp_dealloc(slf: *mut ffi::PyObject, py: Python<'_>) {
+    unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) {
         // Safety: Python only calls tp_dealloc when no references to the object remain.
         let cell = &mut *(slf as *mut PyCell<T>);
         ManuallyDrop::drop(&mut cell.contents.value);
         cell.contents.dict.clear_dict(py);
         cell.contents.weakref.clear_weakrefs(slf, py);
-        <T::BaseType as PyClassBaseType>::LayoutAsBase::tp_dealloc(slf, py)
+        <T::BaseType as PyClassBaseType>::LayoutAsBase::tp_dealloc(py, slf)
     }
 }