C API type slots - #8435
Conversation
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
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks 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<()> { |
@bschoenmaeckers could you check if this is a working design? tried not to increase runtime cost for non-capi path