C API type slots - #8435
C API type slots#8435youknowone wants to merge 9 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds initial support for C extension–provided type slots (starting with tp_new) by storing C ABI function pointers in a per-heap-type table and routing RustPython’s slot dispatch through a shared trampoline. It also exposes a minimal C-API surface to install and query these slots, plus tests validating inheritance and safety behavior.
Changes:
- Introduces a
CSlotstable (owned by heap types) and ac_new_trampolineto dispatchtp_newsupplied from C. - Threads
c_slotsinheritance/identity throughPyTypeSlotsand uses it in the “is not safe”__new__check. - Adds
PyType_GetSlot/Py_tp_newsupport incrates/capiwith tests, and reuses shared C-call utilities inmethodobject.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/vm/src/types/slot.rs | Adds c_slots tracking on PyTypeSlots, plus new_identity() and inheritance/override logic integration. |
| crates/vm/src/types/mod.rs | Exposes the new c_slots module and re-exports selected C-slot types/trampolines. |
| crates/vm/src/types/c_slots.rs | New module implementing C slot tables, trampolines, and shared argument/return conversion utilities. |
| crates/vm/src/builtins/type.rs | Stores owned C-slot tables on heap types and adjusts tp_new identity comparisons for safety checks. |
| crates/capi/src/typeobject.rs | New C-API glue for installing/querying tp_new via CSlots, with unit tests. |
| crates/capi/src/methodobject.rs | Reuses the shared (args, kwds) split + return conversion helpers for C function calls. |
| crates/capi/src/lib.rs | Registers the new typeobject module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let owns_table = self.slots.c_slots.load().is_some() | ||
| && self.slots.c_slots.load() | ||
| != self.base.deref().and_then(|base| base.slots.c_slots.load()); | ||
| if owns_table { |
| /// The C slot table this type owns, once an extension has filled one in. | ||
| /// Types that inherit from it point at this same table. | ||
| pub c_slots: PyRwLock<Option<OwnedCSlots>>, | ||
| pub specialization_cache: TypeSpecializationCache, |
| /// Install a C `newfunc` as the type's tp_new. | ||
| /// | ||
| /// `ty` must be a heap type that does not already define `__new__` itself. | ||
| pub fn set_tp_new(vm: &VirtualMachine, ty: &Py<PyType>, tp_new: newfunc) -> PyResult<()> { |
| pub unsafe extern "C" fn PyType_GetSlot(ty: *const PyTypeObject, slot: c_int) -> *mut c_void { | ||
| let ty = unsafe { &*ty }; | ||
| let Some(c_slots) = ty.slots.c_slots() else { | ||
| return ptr::null_mut(); | ||
| }; | ||
| match CSlotId::from_raw(slot) { | ||
| Some(CSlotId::TpNew) => c_slots | ||
| .new | ||
| .load() | ||
| .map_or(ptr::null_mut(), |f| f as *mut c_void), | ||
| None => ptr::null_mut(), | ||
| } | ||
| } |
There was a problem hiding this comment.
This will not work. As PyType_GetSlot may be called on all types, so this will fail for builtin types. This is also required for extending classes because it requires fetching tp_new of the base type, which is almost always builtins.object.
|
I like the approach but this unfortunately does not work. When constructing heaptypes using the c-api, a type requires the tp_new of the base type. This is most of the time just I tested your approach in this commit bc481ed. Which fails this assert in pyo3: https://github.com/PyO3/pyo3/blob/4df5ac6296fa9c9a375d874082a8ad90e43f9b2d/src/internal/pyclass_init.rs#L50 |
|
Thank you for the review! |
|
CPython semantics are “call base tp_new with subtype as argument”, not “call subtype’s current tp_new”. So we need a |
aaa9f3c to
2c3d5a8
Compare
|
After some experimenting with this, it makes me want fn get_c_tp_new<F>(
_: F,
) -> extern "C" fn(*mut PyTypeObject, *mut PyObject, *mut PyObject) -> *mut PyObject
where
F: FnStatic(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
extern "C" fn trampoline<F>(
subtype: *mut PyTypeObject,
args: *mut PyObject,
kwargs: *mut PyObject,
) -> *mut PyObject
where
F: FnStatic(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
unsafe { F::call_static(&*subtype.to_owned(), tuple_to_args(args), dict_to_kwargs(kwargs)) }
}
trampoline::<F>
}
let c_tp_new = ty.slots.new.load().map(|rust_tp_new| get_c_tp_new(rust_tp_new)I'm not sure how we can do this in todays ✨ rust. |
|
In todays rust we may have to use a static dispatch table. What do you think of this solution? fn get_c_tp_new<F: Any + Send + Sync>(
rust_tp_new: F,
) -> extern "C" fn(*mut PyTypeObject, *mut PyObject, *mut PyObject) -> *mut PyObject
where
F: Fn(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
extern "C" fn trampoline<F: Any + Send + Sync>(
subtype: *mut PyTypeObject,
args: *mut PyObject,
kwargs: *mut PyObject,
) -> *mut PyObject
where
F: Fn(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
let entry = SLOT_CACHE
.get(&TypeId::of::<F>())
.expect("could not find tp_new trampoline");
let func = entry.downcast_ref::<F>().expect("trampoline type mismatch");
with_vm(|vm| unsafe {
let args = FuncArgs::new(
tuple_to_args((&*args).try_downcast_ref(vm)?),
kwargs
.as_ref()
.map(|kwargs| dict_to_kwargs(vm, kwargs.try_downcast_ref::<PyDict>(vm)?))
.transpose()?
.unwrap_or_default(),
);
func((&*subtype).to_owned(), args, vm)
})
}
static SLOT_CACHE: LazyLock<DashMap<TypeId, Box<dyn Any + Send + Sync>>> =
LazyLock::new(DashMap::new);
SLOT_CACHE
.entry(TypeId::of::<F>())
.or_insert_with(|| Box::new(rust_tp_new));
trampoline::<F>
} |
2c3d5a8 to
965e4e0
Compare
|
@bschoenmaeckers Thank you so much! And I am sorry for late response. I am going to try your suggestion without table |
Merging this PR will not alter performance
Comparing Footnotes
|
d9f1e3a to
6ab3f65
Compare
There was a problem hiding this comment.
Could you move this to the object/pytype.rs file?
There was a problem hiding this comment.
Moved crates/capi/src/typeobject.rs into crates/capi/src/object/pytype.rs and dropped the extra typeobject module. Slot APIs are re-exported through object as before.
— commented by Grok 4.6
|
Please limit this to PyType_GetSlot only. I already have code for PySlot & PyType_Slot handling. |
| const ALIGN: usize = 16; | ||
|
|
||
| /// Instance payload whose body is a C-owned byte buffer of `basicsize` bytes. | ||
| pub struct PyCBody { |
There was a problem hiding this comment.
I don't think this will work. The last time I checked it is only possible for a type to have 1 Payload type with data in it. So for a C class extending a PyList, it needs both PyCBody & PyList holding data.
A METH_VARARGS | METH_KEYWORDS function received an empty dict when the call had no keywords, where the calling convention passes NULL. A callee that rejects keywords by testing the pointer saw a non-NULL kwargs. The METH_FASTCALL | METH_KEYWORDS path already passed a NULL kwnames. Name the four function pointer types the PyMethodPointer union holds instead of spelling out each signature inline. Assisted-by: Claude
The tp_new slot holds a Rust fn pointer, which a C `newfunc` cannot be. Put the C function in a `CSlots` table owned by the heap type it belongs to, and store `c_new_trampoline` in `new`; the trampoline reads the table off the type it is called with, so a subclass that inherited both reaches the same function and passes itself as `subtype`. `PyTypeSlots` holds one 8-byte pointer to the table rather than a field per C-provided slot, so further slots are a field in `CSlots` and a trampoline beside this one. The pointer is inherited with `new` by `set_new` and by `update_one_slot`, and dropped when a Python-level `__new__` replaces the slot. Compare what tp_new dispatches to, not the slot, in the "is not safe" check: every C type shares one trampoline, so comparing `new` alone let `CBase.__new__(CSub)` run CSub's tp_new. Move the C call marshalling from capi to `types::c_slots` so the trampoline and the METH_* call paths share it. Assisted-by: Claude
PyType_GetSlot reported Py_tp_new only for a function installed from C. A Rust slot such as int's or object's read as NULL, so an extension that reads its base type's tp_new to call it with its own subtype had nothing to call. Pair each Rust tp_new with a statically instantiated extern "C" function. `c_new_for::<S>` marshals `(subtype, args, kwds)`, calls `S::call`, and returns the object or NULL with the exception pending. `S` is `ViaConstructor<T>` for a `#[pyclass(with(Constructor))]` payload and `PythonNew` for `new_wrapper`; the target is fixed by the type parameter, so a subtype passed in runs the base's body. The derive macro emits a `HasStaticCSlots::TABLE` const per Constructor class holding the (Rust fn, C fn) pair, and stores a reference to it in the new `PyTypeSlots::static_c_slots` field on the defining type only. `PyType::c_tp_new` resolves a C-installed slot from `CSlots`, and a Rust slot by matching the slot's address against the tables along the MRO, then against the `new_wrapper` pair. An empty heap subclass of object now reports object's tp_new rather than NULL; the round-trip test is updated for that. Assisted-by: Claude Assisted-by: Grok
Reproduce what pyo3's PyNativeTypeInitializer does: a heap type whose C tp_new fetches the base type's tp_new through PyType_GetSlot and calls it with the subtype it was given. Cover object and int as bases, a Python-level subclass of the extension type, the slot each type reports, and a ValueError raised by the base crossing both trampolines. Replace a map/unwrap_or_else pair in a test helper that clippy flags. Assisted-by: Claude Assisted-by: Grok
… body An extension built with the 3.15 API creates its types through PyType_FromSlots and writes its per-instance data at the address PyObject_GetTypeData returns. Neither existed, and an instance of a heap type derived from object was always the zero-sized PyBaseObject, so there was nowhere for that data to go. Add PyCBody, an instance payload holding a zero-filled buffer of the type's basicsize bytes, aligned to 16. generic_alloc builds one when the class has a positive basicsize and PyBaseObject otherwise. PyType_FromSlots reads the outer PySlot list (name, flags, inner slots, extra_basicsize, basicsize) and the inner PyType_Slot list (base, new, methods, dealloc, doc), creates the heap type with basicsize = align_up(base.basicsize, 16) + extra_basicsize, honors Py_TPFLAGS_BASETYPE from the spec, publishes the method defs, and stores tp_new and tp_dealloc in the type's CSlots. tp_dealloc is stored and returned by PyType_GetSlot but not called yet. PyObject_GetTypeData returns body + align_up(base.basicsize, 16), and PyType_GetTypeDataSize the matching size. PyType_Freeze is a stub that reports success. A pyo3 #[pyclass] with #[new] and a method now allocates through Py::new, instantiates through a Python-level call, and answers a method call; pyclass_tests.rs covers that end to end. Assisted-by: Claude Assisted-by: Grok
…dObjArgs An extension's tp_dealloc was stored on the type but never called, so the Rust value pyo3 keeps in the instance body was never dropped. drop_slow now reads the class's CSlots: when a C tp_dealloc is set, it runs __del__ and weakref clearing, untracks the object, and hands the pointer to tp_dealloc, which frees the allocation through tp_free. PyType_GetSlot answers Py_tp_free with rustpython_free unless the extension stored its own; it releases the PyInner and the C body without running __del__ again and without decref'ing the class, which tp_dealloc does after tp_free returns. PyType_FromSlots also accepts Py_tp_free and Py_tp_alloc (alloc is stored, not used). PyObject_CallMethodObjArgs is added as a naked entry per ABI, since a variadic definition is not available on stable Rust: Apple ARM64 reads the tail from the stack, AAPCS64 from x2..x7 then the stack, x86_64 SysV from rdx..r9 then the stack, and Win64 from r8/r9 then the stack. Other targets accept only the no-argument form. pyclass_tests.rs now covers call_method0, a Python subclass of a Drop that runs when the last reference goes from Rust or from Python. A variadic test calls PyObject_CallMethodObjArgs with 0, 1 and 7 arguments through its C ABI. Assisted-by: Claude Assisted-by: Grok
Assisted-by: Grok:4.6
The end-to-end pyo3 test sat as a top-level test module in src/. Every other test in this crate lives beside the code it exercises, and an integration test under tests/ cannot link: the exported C symbols in the rlib are not pulled into a separate test binary. Declare it as a test submodule of object/pytype, which owns type creation. Assisted-by: Claude
367e17e to
2fc0aca
Compare
@bschoenmaeckers could you check if this is a working design? tried not to increase runtime cost for non-capi path