Ctypes mul #6305

youknowone · 2025-11-28T16:35:21Z

Summary by CodeRabbit

New Features
- Enabled multiplication of ctypes simple and pointer types by integers to create repeated arrays (e.g., c_int * 5 creates an array of 5 integers).

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

coderabbitai · 2025-11-28T16:35:28Z

Walkthrough

Numeric protocol support (AsNumber) was added to PyCSimpleType and PyCPointerType in the ctypes module, enabling multiplication operations on these types to create repeated arrays. The classmethod-based __mul__ on PyCSimple was replaced with a numeric protocol approach on PyCSimpleType, and corresponding imports were added to support the new trait implementations.

Changes

Cohort / File(s)	Summary
ctypes base type numeric protocol `crates/vm/src/stdlib/ctypes/base.rs`	Added `#[derive(Debug, PyPayload)]` and `#[pyclass(flags(BASETYPE), with(AsNumber))]` to `PyCSimpleType`; implemented `AsNumber` trait with custom multiply handler; added public `__mul__` method delegating to `PyCSimple::repeat`; removed classmethod-based `__mul__` from `PyCSimple`; imported `PyNumberMethods` and `AsNumber`; reworked internal `_type_` attribute lookup for safer handling.
ctypes pointer type numeric protocol `crates/vm/src/stdlib/ctypes/pointer.rs`	Added `flags(IMMUTABLETYPE)` and `with(AsNumber)` to `PyCPointerType`; implemented `AsNumber` trait with multiply operation returning `PyCArrayType`; added `__mul__` method with overflow checks and validation for non-negative integers; imported `PyTypeRef`, `ToPyObject`, `PyNumberMethods`, `AsNumber`, and `VirtualMachine`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

Numeric protocol correctness: Verify AsNumber implementations properly handle type extraction, operand parsing, and overflow/error conditions
Error handling: Review ValueError and type-conversion logic in both __mul__ methods
Type safety: Confirm array/repeat type construction delegates correctly and maintains invariants
Import consistency: Ensure new imports are properly utilized and no redundancies exist

Suggested reviewers

arihant2math

Poem

🐇 Hop, hop! The types now multiply with grace,
No classmethod tricks in this numeric space,
Arrays repeat where numbers dance,
Protocol powers the ctypes prance! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% 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 mul' accurately reflects the main change: adding numeric protocol support via the mul method to ctypes types (PyCSimpleType and PyCPointerType).

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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 (1)

crates/vm/src/stdlib/ctypes/base.rs (1)
247-258: Good error handling improvement, but consider simplifying the lookup logic.

The change from unwrap() to ok_or_else() with a descriptive error message is a good improvement. However, the predicate logic for finding the _type_ attribute seems unnecessarily complex:
k.to_object()
    .str(vm)
    .map(|s| s.to_string() == "_type_")
    .unwrap_or(false)
Depending on the type of k, this might be simplifiable with a direct comparison. Consider investigating whether k can be compared directly without the intermediate conversions.

📜 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 23ec5a5 and 71410b6.

📒 Files selected for processing (2)

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

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

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

**/*.rs: Follow the default rustfmt code style in Rust code (cargo fmt to format)
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

crates/vm/src/stdlib/ctypes/pointer.rs
crates/vm/src/stdlib/ctypes/base.rs

**/src/**/*.rs

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

Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/stdlib/ctypes/pointer.rs
crates/vm/src/stdlib/ctypes/base.rs

🧠 Learnings (1)

📚 Learning: 2025-11-25T11:05:02.899Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:05:02.899Z
Learning: Applies to **/src/**/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

crates/vm/src/stdlib/ctypes/pointer.rs

🧬 Code graph analysis (2)

crates/vm/src/stdlib/ctypes/pointer.rs (1)

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

__mul__ (193-195)

new (169-175)

as_number (199-217)

a (204-205)

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

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

new_simple_type (143-174)

crates/vm/src/stdlib/ctypes/pointer.rs (3)

__mul__ (22-35)

as_number (39-55)

a (42-43)

⏰ 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). (7)

GitHub Check: Run tests under miri
GitHub Check: Check the WASM package and demo
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Check Rust code with clippy
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)

🔇 Additional comments (6)

crates/vm/src/stdlib/ctypes/pointer.rs (3)

1-10: Imports are correctly structured and all are used.

The import organization follows the existing patterns in the codebase. ToPrimitive is properly in scope for the .to_isize() call on line 48.

19-36: Implementation correctly follows the established pattern from PyCSimpleType.

The __mul__ method properly validates the array length and delegates to PyCArrayType construction. The n as usize cast on line 30 is safe due to the negative check on lines 24-26.

38-56: AsNumber implementation is consistent with PyCSimpleType in base.rs.

The multiply handler correctly extracts the type and index, handles overflow, and delegates to the __mul__ method. The static AS_NUMBER reference follows the established pattern.

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

6-6: LGTM! Necessary imports for AsNumber implementation.

The added imports support the numeric protocol integration.

Also applies to: 8-8

162-162: LGTM! Correct setup for numeric protocol.

The Debug derive and with(AsNumber) annotation properly enable numeric operations on this type.

Also applies to: 165-165

192-218: Based on my analysis of the provided code snippet and first-principles understanding of Python/Rust dispatch mechanisms, I can now generate a rewritten review comment.

The key insight is that in Python C extensions (including RustPython), it's common to have both a __mul__ method and an AsNumber implementation. However, the specific concern about duplication in this case is worth examining further. Since I cannot access the full repository or the pointer.rs reference implementation directly, I'll rewrite the comment based on what can be verified from the code snippet itself:

Both __mul__ method and AsNumber::multiply implement identical logic—clarify if this duplication is intentional.

The __mul__ method (lines 192–196) and AsNumber::multiply handler (lines 201–213) perform the same operation: both downcast/extract the arguments and delegate to PyCSimple::repeat. While having both is common in Python C extensions to support different call paths (method access vs. operator dispatch), verify whether:

Both paths are actually needed for the intended API surface of PyCSimpleType.

How this compares to the pattern used in PyCPointerType (pointer.rs).

If only the operator protocol (AsNumber) is required, remove the __mul__ method to reduce maintenance burden. If both are intentional, add a comment explaining why.

youknowone added 2 commits November 29, 2025 01:33

Fix mul

5bf671d

fix mul

71410b6

youknowone marked this pull request as ready for review November 28, 2025 16:48

coderabbitai bot reviewed Nov 28, 2025

View reviewed changes

youknowone merged commit a819128 into RustPython:main Nov 28, 2025
13 checks passed

youknowone deleted the ctypes-mul branch November 28, 2025 17:15

coderabbitai bot mentioned this pull request Nov 29, 2025

ctypes struct/union/array #6309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ctypes mul #6305

Ctypes mul #6305

Uh oh!

youknowone commented Nov 28, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 28, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Ctypes __mul__ #6305

Ctypes __mul__ #6305

Uh oh!

Conversation

youknowone commented Nov 28, 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 Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

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

Ctypes mul #6305

Ctypes mul #6305

youknowone commented Nov 28, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 28, 2025 •

edited

Loading