Initiailzer for PyCStructure #6586

youknowone · 2025-12-30T04:05:55Z

Summary by CodeRabbit

Bug Fixes
- Improved initialization process for ctypes structures
- Fixed field count tracking in ctypes unions
Enhancements
- Better support for positional and keyword arguments when creating structure instances
- Enhanced handling of inherited fields during structure initialization

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-30T04:06:00Z

📝 Walkthrough

Walkthrough

Refactored ctypes structure and union initialization to use Initializer trait implementations instead of slot_new-based initialization. PyCStructType and PyCStructure now implement Initializer to process fields and handle positional/keyword arguments. Added length field tracking to StgInfo for both structures and unions.

Changes

Cohort / File(s)	Summary
Structure initialization refactor `crates/vm/src/stdlib/ctypes/structure.rs`	Introduced Initializer trait implementation for PyCStructType and PyCStructure; added init_pos_args helper for recursive base-class field initialization; moved initialization logic from slot_new to Initializer pathway; updated PyCStructType to record field count in stg_info.length during process_fields
Union length metadata `crates/vm/src/stdlib/ctypes/union.rs`	Added length field to StgInfo for union field counting; set stg_info.length during process_fields; extended stg_info tuple in slot_new to propagate length metadata

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

Init copyslot #6515: Adds Initializer trait implementations and updates pyclass slot wiring across VM types, establishing related trait machinery patterns used in this PR
ctypes buffer #6311: Continues the ctypes structure/union initialization refactoring with Initializer-based flow and stg_info.length tracking, directly building on similar patterns
ctypes struct/union/array #6309: Modifies ctypes structure/union implementation including process_fields, slot initialization, and StgInfo handling in the same code areas

Poem

🐰 A structure takes form, field by field,
Inheritance chains now properly sealed!
With Initializer's grace, recursive and true,
Unions count their members—metadata through and through! 🎪

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The pull request title contains a typo ('Initiailzer' instead of 'Initializer') and is only partially related to the changes, as the PR also implements Initializer for PyCStructType and modifies PyCUnion with length field changes.	Correct the spelling to 'Initializer for PyCStructure' and consider whether the title should reference all significant changes (PyCStructType and PyCUnion modifications) or if the current scope is intentional.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

crates/vm/src/stdlib/ctypes/structure.rs (2)
573-596: Consider logging or erroring on malformed field tuples.

When iterating fields, if a field doesn't downcast to a tuple or lacks a PyStr name (lines 577-580), it's silently skipped without consuming a positional argument. While this may be intentional for flexibility, it could mask issues where _fields_ contains unexpected entries.

If the expectation is that all fields are well-formed (since process_fields validates them earlier), this should be safe. However, a debug assertion or logging might help catch inconsistencies during development.

540-628: Consider extracting shared initialization logic to reduce duplication.

The init_pos_args method and Initializer::init implementation are nearly identical to their counterparts in union.rs (lines 458-547). This ~90 lines of duplicated logic could be refactored into a shared helper in base.rs or via a trait with default implementations.

For example, a generic helper could be added to PyCData or a new trait:
// In base.rs
pub fn init_cdata_from_args<T: AsRef<PyCData>>(
    zelf: &Py<T>,
    cls: &Py<PyType>,
    args: &FuncArgs,
    vm: &VirtualMachine,
) -> PyResult<()> { /* shared logic */ }
This is optional and can be deferred, as the current implementation is correct.

📜 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 489289f and e301c92.

📒 Files selected for processing (2)

crates/vm/src/stdlib/ctypes/structure.rs
crates/vm/src/stdlib/ctypes/union.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/stdlib/ctypes/union.rs
crates/vm/src/stdlib/ctypes/structure.rs

🧬 Code graph analysis (2)

crates/vm/src/stdlib/ctypes/union.rs (2)

crates/vm/src/stdlib/ctypes.rs (2)

stg_info (39-42)

cls (697-697)

crates/vm/src/stdlib/ctypes/base.rs (4)

fields (2284-2284)

fields (2286-2286)

new (149-165)

new (1249-1257)

crates/vm/src/stdlib/ctypes/structure.rs (2)

crates/vm/src/stdlib/ctypes.rs (4)

stg_info (39-42)

base (61-61)

fields_attr (885-885)

field (889-889)

crates/vm/src/stdlib/ctypes/base.rs (20)

fields (2284-2284)

fields (2286-2286)

new (149-165)

new (1249-1257)

from_stg_info (441-450)

value (862-863)

value (871-871)

value (871-871)

value (893-893)

value (907-907)

value (922-922)

value (932-932)

value (1364-1364)

value (1372-1372)

value (1380-1380)

value (1429-1429)

value (1444-1444)

value (1458-1458)

value (1472-1472)

value (1497-1497)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (6)

crates/vm/src/stdlib/ctypes/union.rs (2)

274-291: LGTM: Length tracking added to union StgInfo.

The stg_info.length = fields.len() correctly records the number of fields in the union type metadata. This aligns with the StgInfo::new constructor that initializes length: 0 by default, and ensures the field count is properly propagated to PyCData::from_stg_info which stores it in AtomicCell<usize>.

433-451: LGTM: Length propagation in union slot_new.

The slot_new correctly extracts length alongside total_size and total_align from the type's StgInfo, then propagates it to the new instance via new_stg_info. This ensures union instances have consistent field count metadata matching their type definition.

crates/vm/src/stdlib/ctypes/structure.rs (4)

369-388: LGTM: Length tracking added to structure StgInfo.

The stg_info.length = fields.len() correctly records the field count, consistent with the union implementation. This enables proper length propagation through the structure type system.

515-538: LGTM: Structure slot_new correctly propagates length.

The implementation mirrors the union's slot_new, ensuring consistent length metadata propagation from type to instance. The StgInfo is properly constructed with size, alignment, and length before being passed to PyCData::from_stg_info.

603-628: LGTM: Initializer implementation follows CPython ctypes semantics.

The implementation correctly:

Processes positional arguments recursively through the inheritance chain

Detects duplicate field specifications (positional + keyword for same field)

Reports "too many initializers" when extra positional args are provided

Applies keyword arguments via set_attr, letting field descriptors handle validation

633-636: LGTM: pyclass macro updated to include Initializer.

The with(Constructor, Initializer, AsBuffer) correctly registers the new Initializer trait implementation for PyCStructure.

Initiailzer for PyCStructure

e301c92

youknowone marked this pull request as ready for review December 30, 2025 07:57

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

youknowone merged commit 6bf1ab6 into RustPython:main Dec 30, 2025
23 of 24 checks passed

youknowone deleted the ctypes branch December 30, 2025 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Initiailzer for PyCStructure #6586

Initiailzer for PyCStructure #6586

Uh oh!

youknowone commented Dec 30, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Initiailzer for PyCStructure #6586

Initiailzer for PyCStructure #6586

Uh oh!

Conversation

youknowone commented Dec 30, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 30, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 30, 2025 •

edited

Loading