Ctypes __mul__ by youknowone · Pull Request #6305 · RustPython/RustPython
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.