ctypes struct/union/array by youknowone · Pull Request #6309 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

Walkthrough

Reworks _ctypes: adds metaclass-driven Structure/Union types, converts C arrays to buffer-backed, sequence-enabled types, implements descriptor-based PyCField read/write, changes byref to validate and return a new CArgObject, and exposes a platform-aware _wstring_at_addr attr.

Changes

Cohort / File(s)	Summary
Core ctypes module and wiring `crates/vm/src/stdlib/ctypes.rs`	Registers `PyCStructType` and `PyCUnionType`; exposes `CArgObject` pyclass with `_obj` getter; changes `byref` signature to `(obj, offset)` returning a validated `CArgObject`; adds `_wstring_at_addr` attr; imports `ToPyObject`, `PyPayload`, `PyObjectRef`.
Array refactor & behavior `crates/vm/src/stdlib/ctypes/array.rs`, `crates/vm/src/stdlib/ctypes/base.rs`, `crates/vm/src/stdlib/ctypes/pointer.rs`	`PyCArrayType` now wraps an inner `PyCArray`; `PyCArray` drops `value` in favor of `element_size: AtomicCell` and `buffer: PyRwLock<Vec<u8>>`; adds sequence semantics (`AsSequence`), numeric ops (`AsNumber`/`__mul__`), item access (`__getitem__`, `__setitem__`, `__len__`), packing helpers, and `to_arg` from buffer contents.
Field descriptor implementation `crates/vm/src/stdlib/ctypes/field.rs`	`PyCField` becomes a descriptor: adds `PyCField::new`, `descr_get`, `descr_set`, `__set__`, `__delete__`; `proto` changed to `PyObjectRef`; implements bytes↔value helpers and returns `type` as `PyObjectRef`.
Structure metaclass & Structure class `crates/vm/src/stdlib/ctypes/structure.rs`	Adds `PyCStructType` metaclass with `py_new`, `__init_subclass__`, `process_fields`, `get_field_size`, `__mul__`; introduces `FieldInfo`; refactors `PyCStructure` to use `buffer`, `fields` map and `size`, with `py_new` populating fields and buffer.
Union metaclass & Union class `crates/vm/src/stdlib/ctypes/union.rs`	Replaces direct `Union` with `PyCUnionType` metaclass; adds `py_new`, `process_fields`, and `get_field_size`; registers `PyCField` descriptors (offset 0) during class creation.

Sequence Diagram

sequenceDiagram
    participant User as User code
    participant Meta as PyCStructType / PyCUnionType
    participant Cls as Structure/Union Class
    participant Field as PyCField (descriptor)
    participant Inst as Structure/Union Instance
    participant Core as _ctypes module

    User->>Meta: define class with _fields_
    Meta->>Meta: process_fields(_fields_)
    Meta->>Field: create PyCField descriptors (compute size/offset)
    Meta->>Cls: register descriptors and fields metadata
    Meta-->>User: return created class

    User->>Cls: instantiate(args/kwargs)
    Cls->>Inst: py_new -> allocate buffer (Vec<u8>) and fields map
    Inst->>Inst: populate buffer from args/kwargs
    Inst-->>User: instance returned

    User->>Inst: read attribute
    Inst->>Field: descriptor __get__ (reads buffer[offset...])
    Field->>Field: bytes_to_value -> PyObject
    Field-->>User: value

    User->>Inst: write attribute
    Inst->>Field: descriptor __set__ (value_to_bytes -> bytes)
    Field->>Inst: write bytes into buffer[offset...]
    Field-->>User: done

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Pay special attention to:
- PyCArray buffer lifecycle, element_size correctness, and safety/to_arg pointer/lifetime behavior.
- Descriptor bytes↔value conversion (bitfields, alignment, endianness).
- process_fields offset/size calculations and descriptor registration.
- byref → CArgObject validation, VM exposure, and ABI of returned wrapper.

Possibly related PRs

ctypes struct/union/array #6309 — appears to implement the same ctypes changes (CArgObject, metaclass registrations, byref signature/behavior).
Ctypes __mul__ #6305 — overlaps on adding AsNumber/mul behavior used by pointer/array multiplication and numeric semantics.
Downcastable #5986 — touches ctypes array implementation and to_arg/downcasting details overlapping with buffer/to_arg changes.

Suggested reviewers

arihant2math

"🐰
I nibble bytes and line them neat,
Fields and buffers now all meet,
Metaclasses hop, descriptors sing,
Byref brings CArg with a spring,
Hooray — ctypes leaps to greet!"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 67.27% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'ctypes struct/union/array' is a concise summary of the main changes, which focus on implementing metaclass-driven ctypes structures, unions, and arrays with field processing and buffer management.

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10fe041 and 711ea51.

📒 Files selected for processing (3)

crates/vm/src/stdlib/ctypes.rs (5 hunks)
crates/vm/src/stdlib/ctypes/array.rs (5 hunks)
crates/vm/src/stdlib/ctypes/structure.rs (1 hunks)

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}