move some methods to an 'internal' trait to avoid leaking information

src/impl_/coroutine.rs

@@ -6,7 +6,7 @@ use std::{
 use crate::{
     coroutine::{cancel::ThrowCallback, Coroutine},
     instance::Bound,
-    pycell::impl_::{InternalPyClassObjectLayout, PyClassBorrowChecker},
+    pycell::impl_::{PyClassBorrowChecker, PyClassObjectLayout},
     pyclass::boolean_struct::False,
     types::{PyAnyMethods, PyString},
     IntoPyObject, Py, PyAny, PyClass, PyErr, PyResult, Python,

src/impl_/pycell.rs

@@ -1,5 +1,5 @@
 //! Externally-accessible implementation of pycell
 pub use crate::pycell::impl_::{
-    GetBorrowChecker, PyClassMutability, PyClassObjectBase, PyClassObjectLayout,
+    GetBorrowChecker, PyClassMutability, PyClassObjectBase, PyClassObjectBasics,
     PyStaticClassObject, PyVariableClassObject, PyVariableClassObjectBase,
 };

src/impl_/pyclass.rs

@@ -1,16 +1,16 @@
 #[cfg(Py_3_12)]
-use crate::pycell::impl_::PyClassObjectContents;
+use crate::pycell::impl_::{PyClassObjectContents, PyClassObjectLayoutInternals};
 use crate::{
     conversion::IntoPyObject,
     exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError},
     ffi,
     impl_::{
         freelist::FreeList,
-        pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout},
+        pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectBasics},
         pyclass_init::PyObjectInit,
         pymethods::{PyGetterDef, PyMethodDefType},
     },
-    pycell::{impl_::InternalPyClassObjectLayout, PyBorrowError},
+    pycell::{impl_::PyClassObjectLayout, PyBorrowError},
     types::{any::PyAnyMethods, PyBool},
     Borrowed, BoundObject, Py, PyAny, PyClass, PyErr, PyRef, PyResult, PyTypeInfo, Python,
 };
@@ -170,7 +170,7 @@ pub trait PyClassImpl: Sized + 'static {
     const IS_SEQUENCE: bool = false;
 
     /// Description of how this class is laid out in memory
-    type Layout: InternalPyClassObjectLayout<Self>;
+    type Layout: PyClassObjectLayout<Self>;
 
     /// Base class
     type BaseType: PyTypeInfo + PyClassBaseType;
@@ -1137,7 +1137,7 @@ impl<T> PyClassThreadChecker<T> for ThreadCheckerImpl {
     )
 )]
 pub trait PyClassBaseType: Sized {
-    type LayoutAsBase: PyClassObjectLayout<Self>;
+    type LayoutAsBase: PyClassObjectBasics<Self>;
     type BaseNativeType;
     type Initializer: PyObjectInit<Self>;
     type PyClassMutability: PyClassMutability;

src/impl_/pyclass_init.rs

@@ -2,7 +2,7 @@
 use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::impl_::pyclass::PyClassImpl;
 use crate::internal::get_slot::TP_ALLOC;
-use crate::pycell::impl_::{InternalPyClassObjectLayout, PyClassObjectContents};
+use crate::pycell::impl_::{PyClassObjectContents, PyClassObjectLayoutInternals};
 use crate::types::PyType;
 use crate::{ffi, Borrowed, PyErr, PyResult, Python};
 use crate::{ffi::PyTypeObject, sealed::Sealed, type_object::PyTypeInfo};

src/impl_/pymethods.rs

@@ -2,9 +2,11 @@ use crate::exceptions::PyStopAsyncIteration;
 use crate::gil::LockGIL;
 use crate::impl_::callback::IntoPyCallbackOutput;
 use crate::impl_::panic::PanicTrap;
-use crate::impl_::pycell::PyClassObjectLayout;
+use crate::impl_::pycell::PyClassObjectBasics;
 use crate::internal::get_slot::{get_slot, TP_BASE, TP_CLEAR, TP_TRAVERSE};
-use crate::pycell::impl_::{InternalPyClassObjectLayout, PyClassBorrowChecker as _};
+use crate::pycell::impl_::{
+    PyClassBorrowChecker as _, PyClassObjectLayout, PyClassObjectLayoutInternals,
+};
 use crate::pycell::{PyBorrowError, PyBorrowMutError};
 use crate::pyclass::boolean_struct::False;
 use crate::types::any::PyAnyMethods;
@@ -310,8 +312,8 @@ where
     if class_object.check_threadsafe().is_ok()
     // ... and we cannot traverse a type which might be being mutated by a Rust thread
     && class_object.borrow_checker().try_borrow().is_ok() {
-        struct TraverseGuard<'a, U: PyClassImpl, V: InternalPyClassObjectLayout<U>>(&'a V, PhantomData<U>);
-        impl<U: PyClassImpl, V: InternalPyClassObjectLayout<U>> Drop for TraverseGuard<'_, U, V> {
+        struct TraverseGuard<'a, U: PyClassImpl, V: PyClassObjectLayout<U>>(&'a V, PhantomData<U>);
+        impl<U: PyClassImpl, V: PyClassObjectLayout<U>> Drop for TraverseGuard<'_, U, V> {
             fn drop(&mut self) {
                 self.0.borrow_checker().release_borrow()
             }

src/instance.rs

@@ -2,7 +2,7 @@ use crate::conversion::IntoPyObject;
 use crate::err::{self, PyErr, PyResult};
 use crate::impl_::pyclass::PyClassImpl;
 use crate::internal_tricks::ptr_from_ref;
-use crate::pycell::impl_::InternalPyClassObjectLayout;
+use crate::pycell::impl_::PyClassObjectLayout;
 use crate::pycell::{PyBorrowError, PyBorrowMutError};
 use crate::pyclass::boolean_struct::{False, True};
 use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods};

src/pycell.rs

@@ -208,7 +208,7 @@ use std::mem::ManuallyDrop;
 use std::ops::{Deref, DerefMut};
 
 pub(crate) mod impl_;
-use impl_::{InternalPyClassObjectLayout, PyClassBorrowChecker, PyClassObjectLayout};
+use impl_::{PyClassBorrowChecker, PyClassObjectBasics, PyClassObjectLayout};
 
 /// A wrapper type for an immutably borrowed value from a [`Bound<'py, T>`].
 ///

src/pycell/impl_.rs

@@ -226,7 +226,7 @@ pub struct PyVariableClassObjectBase {
 
 unsafe impl<T> PyLayout<T> for PyVariableClassObjectBase {}
 
-impl<T: PyTypeInfo> PyClassObjectLayout<T> for PyVariableClassObjectBase {
+impl<T: PyTypeInfo> PyClassObjectBasics<T> for PyVariableClassObjectBase {
     fn ensure_threadsafe(&self) {}
     fn check_threadsafe(&self) -> Result<(), PyBorrowError> {
         Ok(())
@@ -237,7 +237,7 @@ impl<T: PyTypeInfo> PyClassObjectLayout<T> for PyVariableClassObjectBase {
 }
 
 #[doc(hidden)]
-pub trait PyClassObjectLayout<T>: PyLayout<T> {
+pub trait PyClassObjectBasics<T>: PyLayout<T> {
     fn ensure_threadsafe(&self);
     fn check_threadsafe(&self) -> Result<(), PyBorrowError>;
     /// Implementation of tp_dealloc.
@@ -248,26 +248,33 @@ pub trait PyClassObjectLayout<T>: PyLayout<T> {
 }
 
 #[doc(hidden)]
-#[cfg_attr(
-    all(diagnostic_namespace),
-    diagnostic::on_unimplemented(
-        message = "the class layout is not valid",
-        label = "required for `#[pyclass(extends=...)]`",
-        note = "the python version being built against influences which layouts are valid",
-    )
-)]
-pub trait InternalPyClassObjectLayout<T: PyClassImpl>: PyClassObjectLayout<T> {
+pub(crate) trait PyClassObjectLayoutInternals<T: PyClassImpl>: PyLayout<T> {
     /// Obtain a pointer to the contents of an uninitialized PyObject of this type
     /// Safety: the provided object must have the layout that the implementation is expecting
     unsafe fn contents_uninitialised(
         obj: *mut ffi::PyObject,
     ) -> *mut MaybeUninit<PyClassObjectContents<T>>;
 
-    fn get_ptr(&self) -> *mut T;
-
     fn contents(&self) -> &PyClassObjectContents<T>;
 
     fn contents_mut(&mut self) -> &mut PyClassObjectContents<T>;
+}
+
+/// Functionality required for creating and managing the memory associated with a pyclass annotated struct.
+#[doc(hidden)]
+#[cfg_attr(
+    all(diagnostic_namespace),
+    diagnostic::on_unimplemented(
+        message = "the class layout is not valid",
+        label = "required for `#[pyclass(extends=...)]`",
+        note = "the python version being built against influences which layouts are valid",
+    )
+)]
+pub trait PyClassObjectLayout<T: PyClassImpl>:
+    PyClassObjectBasics<T> + PyClassObjectLayoutInternals<T>
+{
+    /// obtain a pointer to the pyclass data
+    fn get_ptr(&self) -> *mut T;
 
     fn ob_base(&self) -> &<T::BaseType as PyClassBaseType>::LayoutAsBase;
 
@@ -287,7 +294,7 @@ pub trait InternalPyClassObjectLayout<T: PyClassImpl>: PyClassObjectLayout<T> {
     fn borrow_checker(&self) -> &<T::PyClassMutability as PyClassMutability>::Checker;
 }
 
-impl<T, U> PyClassObjectLayout<T> for PyClassObjectBase<U>
+impl<T, U> PyClassObjectBasics<T> for PyClassObjectBase<U>
 where
     U: PySizedLayout<T>,
     T: PyTypeInfo,
@@ -301,6 +308,10 @@ where
     }
 }
 
+/// Implementation of tp_dealloc.
+/// # Safety
+/// - obj must be a valid pointer to an instance of the type at `type_ptr` or a subclass.
+/// - obj must not be used after this call (as it will be freed).
 unsafe fn tp_dealloc(py: Python<'_>, obj: *mut ffi::PyObject, type_ptr: *mut ffi::PyTypeObject) {
     // FIXME: there is potentially subtle issues here if the base is overwritten
     // at runtime? To be investigated.
@@ -376,7 +387,7 @@ pub struct PyStaticClassObject<T: PyClassImpl> {
     contents: PyClassObjectContents<T>,
 }
 
-impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyStaticClassObject<T> {
+impl<T: PyClassImpl> PyClassObjectLayoutInternals<T> for PyStaticClassObject<T> {
     unsafe fn contents_uninitialised(
         obj: *mut ffi::PyObject,
     ) -> *mut MaybeUninit<PyClassObjectContents<T>> {
@@ -389,21 +400,23 @@ impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyStaticClassObject<T> {
         addr_of_mut!((*obj).contents)
     }
 
-    fn get_ptr(&self) -> *mut T {
-        self.contents.value.get()
-    }
-
-    fn ob_base(&self) -> &<T::BaseType as PyClassBaseType>::LayoutAsBase {
-        &self.ob_base
-    }
-
     fn contents(&self) -> &PyClassObjectContents<T> {
         &self.contents
     }
 
     fn contents_mut(&mut self) -> &mut PyClassObjectContents<T> {
         &mut self.contents
     }
+}
+
+impl<T: PyClassImpl> PyClassObjectLayout<T> for PyStaticClassObject<T> {
+    fn get_ptr(&self) -> *mut T {
+        self.contents.value.get()
+    }
+
+    fn ob_base(&self) -> &<T::BaseType as PyClassBaseType>::LayoutAsBase {
+        &self.ob_base
+    }
 
     /// used to set PyType_Spec::basicsize
     /// https://docs.python.org/3/c-api/type.html#c.PyType_Spec.basicsize
@@ -453,9 +466,9 @@ impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyStaticClassObject<T> {
 unsafe impl<T: PyClassImpl> PyLayout<T> for PyStaticClassObject<T> {}
 impl<T: PyClass> PySizedLayout<T> for PyStaticClassObject<T> {}
 
-impl<T: PyClassImpl> PyClassObjectLayout<T> for PyStaticClassObject<T>
+impl<T: PyClassImpl> PyClassObjectBasics<T> for PyStaticClassObject<T>
 where
-    <T::BaseType as PyClassBaseType>::LayoutAsBase: PyClassObjectLayout<T::BaseType>,
+    <T::BaseType as PyClassBaseType>::LayoutAsBase: PyClassObjectBasics<T::BaseType>,
 {
     fn ensure_threadsafe(&self) {
         self.contents.thread_checker.ensure();
@@ -496,21 +509,13 @@ impl<T: PyClassImpl> PyVariableClassObject<T> {
 }
 
 #[cfg(Py_3_12)]
-impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyVariableClassObject<T> {
+impl<T: PyClassImpl> PyClassObjectLayoutInternals<T> for PyVariableClassObject<T> {
     unsafe fn contents_uninitialised(
         obj: *mut ffi::PyObject,
     ) -> *mut MaybeUninit<PyClassObjectContents<T>> {
         Self::get_contents_of_obj(obj) as *mut MaybeUninit<PyClassObjectContents<T>>
     }
 
-    fn get_ptr(&self) -> *mut T {
-        self.contents().value.get()
-    }
-
-    fn ob_base(&self) -> &<T::BaseType as PyClassBaseType>::LayoutAsBase {
-        &self.ob_base
-    }
-
     fn contents(&self) -> &PyClassObjectContents<T> {
         unsafe { (self.get_contents_ptr() as *const PyClassObjectContents<T>).as_ref() }
             .expect("should be able to cast PyClassObjectContents pointer")
@@ -520,6 +525,17 @@ impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyVariableClassObject<T>
         unsafe { self.get_contents_ptr().as_mut() }
             .expect("should be able to cast PyClassObjectContents pointer")
     }
+}
+
+#[cfg(Py_3_12)]
+impl<T: PyClassImpl> PyClassObjectLayout<T> for PyVariableClassObject<T> {
+    fn get_ptr(&self) -> *mut T {
+        self.contents().value.get()
+    }
+
+    fn ob_base(&self) -> &<T::BaseType as PyClassBaseType>::LayoutAsBase {
+        &self.ob_base
+    }
 
     /// used to set PyType_Spec::basicsize
     /// https://docs.python.org/3/c-api/type.html#c.PyType_Spec.basicsize
@@ -560,9 +576,9 @@ impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyVariableClassObject<T>
 unsafe impl<T: PyClassImpl> PyLayout<T> for PyVariableClassObject<T> {}
 
 #[cfg(Py_3_12)]
-impl<T: PyClassImpl> PyClassObjectLayout<T> for PyVariableClassObject<T>
+impl<T: PyClassImpl> PyClassObjectBasics<T> for PyVariableClassObject<T>
 where
-    <T::BaseType as PyClassBaseType>::LayoutAsBase: PyClassObjectLayout<T::BaseType>,
+    <T::BaseType as PyClassBaseType>::LayoutAsBase: PyClassObjectBasics<T::BaseType>,
 {
     fn ensure_threadsafe(&self) {
         self.contents().thread_checker.ensure();

src/pyclass/create_type_object.rs

@@ -11,7 +11,7 @@ use crate::{
         trampoline::trampoline,
     },
     internal_tricks::ptr_from_ref,
-    pycell::impl_::InternalPyClassObjectLayout,
+    pycell::impl_::PyClassObjectLayout,
     types::{typeobject::PyTypeMethods, PyType},
     Py, PyClass, PyResult, PyTypeInfo, Python,
 };

src/pyclass_init.rs

@@ -3,7 +3,7 @@ use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::impl_::callback::IntoPyCallbackOutput;
 use crate::impl_::pyclass::{PyClassBaseType, PyClassImpl};
 use crate::impl_::pyclass_init::{PyNativeTypeInitializer, PyObjectInit};
-use crate::pycell::impl_::InternalPyClassObjectLayout;
+use crate::pycell::impl_::PyClassObjectLayoutInternals;
 use crate::types::PyAnyMethods;
 use crate::{ffi, Bound, Py, PyClass, PyResult, Python};
 use crate::{ffi::PyTypeObject, pycell::impl_::PyClassObjectContents};