-
Notifications
You must be signed in to change notification settings - Fork 950
Description
Somewhat inspired by the work going on in #5689.
The current capsule constructors PyCapsule::new and PyCapsule::new_with_destructor have some downsides:
- They unconditionally allocate the
CapsuleContentstype. - They require an allocated
CStringfor the name, if set (the full name type isOption<CString>)
After we started reworking the capsule methods in 0.27 to require checking a name, I think it would make sense to have a name be required as &'static CStr, to further encourage name checking. Also, if we no longer need to store a CString within the CapsuleContents, for the case of a trivial destructor we don't need to have CapusleContents at all (the only thing to store is T).
This would allow us to store the value as Box<T>, or maybe even store the value without boxing as long as T is no more than pointer sized.
While some experimentation to be had, I'd propose we introduce new constructors for now as
impl PyCapsule {
pub fn new_with_name<T: 'static + Send + AssertNotZeroSized>(
py: Python<'_>,
value: T,
name: &'static CStr,
) -> PyResult<Bound<'_, Self>>;
pub fn new_with_name_and_destructor<
T: 'static + Send + AssertNotZeroSized,
F: FnOnce(T, *mut c_void) + Send,
>(
py: Python<'py>,
value: T,
name: &'static CStr,
destructor: F,
) -> PyResult<Bound<'_, Self>>;... and maybe deprecate the existing constructors.
cc @lmmx