Fix import ctypes by youknowone · Pull Request #6304 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

Walkthrough

Unix library loader now accepts None to obtain the current process handle; ctypes gains cached pointer-type creation and pointer-instantiation APIs exposed as POINTER/pointer; PyCSimpleType.from_param added; PyCFuncPtr constructor refactored to support empty, address, and (name,dll) forms.

Changes

Cohort / File(s)	Summary
Unix library loading `crates/vm/src/stdlib/ctypes.rs`	`load_library_unix` signature changed to accept `Option<String>`; `None` calls `libc::dlopen(null, RTLD_NOW)` to return current process handle; previous cache insertion preserved for Some(name).
Pointer type & instance APIs `crates/vm/src/stdlib/ctypes.rs`	Added `pub fn create_pointer_type(cls: PyObjectRef, vm: &VirtualMachine) -> PyResult` and `pub fn create_pointer_inst(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult`; introduced `_pointer_type_cache` usage and dynamic pointer-type construction; updated `POINTER` and `pointer` bindings to these implementations.
PyCSimpleType.from_param `crates/vm/src/stdlib/ctypes/base.rs`	Added `#[pyclassmethod] fn from_param(cls: PyTypeRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult` that returns value if already instance of `cls`, otherwise reads `_as_parameter_` and delegates recursively or raises TypeError.
PyCFuncPtr constructor refactor `crates/vm/src/stdlib/ctypes/function.rs`	Changed constructor `Args` from `(PyTupleRef, FuncArgs)` to `FuncArgs`; `py_new` now inspects `args` and branches: empty → uninitialized pointer, integer → treat as function address, tuple → treat as `(name, dll)` with library loading and symbol lookup; added error handling for invalid forms.
PyCArray construction typing `crates/vm/src/stdlib/ctypes/array.rs`	`py_new` now accepts `cls: PyTypeRef` and constructs return via `.into_ref_with_type(vm, cls).map(Into::into)` instead of `.into_pyobject(vm)` to preserve type.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant PyCFuncPtr as PyCFuncPtr::py_new
    participant FormInspector as Form Branching
    participant AddrHandler as Address Path
    participant TupleHandler as Tuple (name,dll) Path
    participant LibLoader as Library Loader
    participant Result as PyCFuncPtr Construction

    Caller->>PyCFuncPtr: call py_new(args)
    PyCFuncPtr->>FormInspector: inspect args

    alt args empty
        FormInspector->>Result: create uninitialized PyCFuncPtr
    else first arg is integer
        FormInspector->>AddrHandler: extract address
        AddrHandler->>Result: build PyCFuncPtr with pointer, name, handler
    else first arg is tuple (name,dll)
        FormInspector->>TupleHandler: extract name and dll
        TupleHandler->>LibLoader: resolve library handle (load_library_unix / LoadLibrary)
        LibLoader->>LibLoader: lookup symbol by name
        LibLoader->>Result: return code pointer or None
        Result->>Result: construct PyCFuncPtr with resolved data
    else invalid form
        FormInspector->>Result: return TypeError
    end

    Result-->>Caller: PyResult<PyCFuncPtr>

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Attention areas:
- crates/vm/src/stdlib/ctypes/function.rs: branching correctness, safety around raw addresses, symbol lookup and error propagation.
- crates/vm/src/stdlib/ctypes.rs: _pointer_type_cache concurrency/validation, type derivation correctness, public API bindings.
- crates/vm/src/stdlib/ctypes/base.rs: recursion and attribute lookup semantics in from_param.
- crates/vm/src/stdlib/ctypes/array.rs: ensuring correct type-ref construction and ownership.

Possibly related PRs

Fix import ctypes #6304 — mirrors the same ctypes changes (load_library_unix signature, create_pointer_type/create_pointer_inst, POINTER/pointer bindings).
_ctypes pt. 5 #5653 — overlapping ctypes subsystem modifications including Unix library-loading behavior and pointer/funcptr implementations.

Suggested reviewers

arihant2math

Poem

🐰
I nibble bytes where pointers grow,
Cache a type and off I go.
Address, tuple, or empty start,
Functions bound and types take part.
Hooray — ctypes hops with rabbit heart!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix import ctypes' is vague and does not clearly convey the primary changes. The PR introduces significant new functionality (pointer type generation, caching, Unix/Windows library loading improvements) that goes well beyond a simple 'import' fix.	Consider using a more descriptive title that captures the main purpose, such as 'Implement ctypes pointer type generation and library loading' or 'Add POINTER and pointer functions to ctypes module'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 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 469c93a and 7c66122.

⛔ Files ignored due to path filters (1)

Lib/ctypes/__init__.py is excluded by !Lib/**

📒 Files selected for processing (4)

crates/vm/src/stdlib/ctypes.rs (3 hunks)
crates/vm/src/stdlib/ctypes/array.rs (1 hunks)
crates/vm/src/stdlib/ctypes/base.rs (1 hunks)
crates/vm/src/stdlib/ctypes/function.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.}