Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 1, 2026

Summary by CodeRabbit

  • Refactor
    • Internal refactoring of type conversion patterns to use explicit conversion methods instead of dereferencing, improving code clarity and maintainability throughout the standard library and core components.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

📝 Walkthrough

Walkthrough

This PR refactors internal type conversions by introducing explicit conversion methods (into_float(), into_complex(), into_bool(), into_int_ref(), obj()) on specialized wrapper types and removing their Deref implementations. The changes propagate these new method calls throughout stdlib and VM modules, replacing direct dereferencing patterns while preserving control flow and public API signatures.

Changes

Cohort / File(s) Summary
Core conversion API changes
crates/vm/src/function/number.rs, crates/vm/src/function/protocol.rs
Added inherent methods: ArgIntoComplex::into_complex(), ArgIntoFloat::into_float(), ArgIntoBool::into_bool(), ArgIndex::into_int_ref(), ArgMapping::obj(). Removed Deref impls for these types. Added corresponding From trait impls where applicable.
Standard library numeric conversions
crates/stdlib/src/array.rs, crates/stdlib/src/cmath.rs, crates/stdlib/src/math.rs, crates/stdlib/src/statistics.rs
Updated float/complex type conversions to use explicit into_float() and into_complex() method calls instead of dereferencing. Log base handling in cmath now accesses .re on converted complex values.
Index and argument handling
crates/stdlib/src/bisect.rs, crates/vm/src/builtins/slice.rs, crates/vm/src/stdlib/builtins.rs
Replaced direct integer conversions with into_int_ref() before accessing bigint or calling try_to_primitive(). Updated hex/oct functions to use new conversion pattern.
Builtin type conversions
crates/vm/src/buffer.rs, crates/vm/src/builtins/bytes.rs, crates/vm/src/stdlib/itertools.rs
Updated pack operations and truthiness checks to use into_float(), into_bool(), and into_int_ref() methods. Updated rmul and setstate implementations with new conversion paths.
Mapping protocol adjustments
crates/vm/src/builtins/mappingproxy.rs
Updated contains and copy logic for Mapping variant to use new obj() accessor instead of direct field access.
Boolean and control flow
crates/vm/src/stdlib/builtins.rs
Updated all/any truthiness evaluation and print flush check to use explicit into_bool() conversions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hops with joy through conversion's dance,
No more dereference, just methods vast,
Explicit paths where coercion once ran,
Refactored cleanly across the codebase span!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Remove wrong Deref' is vague and generic, using non-descriptive terms that don't clearly convey what specific Deref implementations are being removed or why they are problematic. Consider a more specific title such as 'Replace Deref impls with explicit accessor methods in number types' to better describe the main change across multiple type conversions.
✅ 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 81.63% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 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 b446dac and 331660d.

📒 Files selected for processing (13)
  • crates/stdlib/src/array.rs
  • crates/stdlib/src/bisect.rs
  • crates/stdlib/src/cmath.rs
  • crates/stdlib/src/math.rs
  • crates/stdlib/src/statistics.rs
  • crates/vm/src/buffer.rs
  • crates/vm/src/builtins/bytes.rs
  • crates/vm/src/builtins/mappingproxy.rs
  • crates/vm/src/builtins/slice.rs
  • crates/vm/src/function/number.rs
  • crates/vm/src/function/protocol.rs
  • crates/vm/src/stdlib/builtins.rs
  • crates/vm/src/stdlib/itertools.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/itertools.rs
  • crates/stdlib/src/bisect.rs
  • crates/vm/src/buffer.rs
  • crates/vm/src/builtins/mappingproxy.rs
  • crates/vm/src/builtins/bytes.rs
  • crates/stdlib/src/statistics.rs
  • crates/vm/src/function/protocol.rs
  • crates/stdlib/src/math.rs
  • crates/vm/src/builtins/slice.rs
  • crates/stdlib/src/cmath.rs
  • crates/stdlib/src/array.rs
  • crates/vm/src/stdlib/builtins.rs
  • crates/vm/src/function/number.rs
🧠 Learnings (1)
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

  • crates/stdlib/src/math.rs
🧬 Code graph analysis (7)
crates/stdlib/src/bisect.rs (1)
crates/stdlib/src/opcode.rs (1)
  • v (133-135)
crates/vm/src/buffer.rs (1)
crates/vm/src/function/number.rs (1)
  • into_float (63-65)
crates/vm/src/builtins/mappingproxy.rs (1)
crates/vm/src/builtins/dict.rs (4)
  • mapping (1150-1152)
  • mapping (1214-1216)
  • mapping (1277-1279)
  • d (199-199)
crates/stdlib/src/math.rs (1)
crates/vm/src/function/number.rs (1)
  • into_float (63-65)
crates/vm/src/builtins/slice.rs (1)
crates/vm/src/protocol/object.rs (1)
  • length (677-684)
crates/stdlib/src/cmath.rs (1)
crates/stdlib/src/math.rs (6)
  • isinf (61-63)
  • isfinite (56-58)
  • isnan (66-68)
  • exp (123-125)
  • result_or_overflow (39-47)
  • sin (412-418)
crates/vm/src/function/number.rs (1)
crates/vm/src/function/protocol.rs (4)
  • from (52-54)
  • from (163-165)
  • as_ref (45-47)
  • as_ref (156-158)
🔇 Additional comments (34)
crates/vm/src/stdlib/itertools.rs (2)

558-567: LGTM! Explicit boolean conversion applied correctly.

The change from dereferencing to calling into_bool() makes the conversion explicit and aligns with the PR's objective to remove Deref implementations. The boolean value is correctly stored in the AtomicCell<bool>.


644-654: LGTM! Consistent boolean conversion pattern.

The explicit into_bool() conversion is applied consistently with the Takewhile implementation above. This maintains consistency across similar __setstate__ methods.

crates/vm/src/builtins/bytes.rs (1)

515-519: LGTM! Explicit integer conversion correctly applied.

The change adds an explicit conversion step via into_int_ref() to obtain the PyIntRef before calling try_to_primitive(). This aligns with the PR's goal of replacing implicit Deref access with explicit conversion methods.

crates/vm/src/builtins/slice.rs (1)

176-184: LGTM! Explicit conversion pattern applied correctly.

The addition of into_int_ref() makes the conversion from ArgIndex to PyIntRef explicit before accessing the BigInt value. This is consistent with the project-wide refactor to replace Deref-based access with explicit conversion methods.

crates/vm/src/function/number.rs (4)

23-34: LGTM! Well-structured conversion API for ArgIntoComplex.

The new into_complex() method and From<ArgIntoComplex> trait implementation provide both explicit and idiomatic conversion paths for extracting Complex64 values. The inline annotation ensures no performance overhead.


61-80: LGTM! Consistent conversion pattern for ArgIntoFloat.

The into_float() method and associated From implementation follow the same pattern as ArgIntoComplex, providing consistent API design. The vec_into_f64 utility function complements the conversion API well.


103-117: LGTM! ArgIntoBool conversion API complete.

The into_bool() method and From<ArgIntoBool> implementation provide the expected conversion interface. The constants TRUE and FALSE are useful for common cases.


134-151: LGTM! Comprehensive conversion API for ArgIndex.

The implementation provides three access patterns:

  • into_int_ref() for owned conversion
  • AsRef<PyIntRef> for borrowed access
  • From<ArgIndex> for idiomatic conversion

This gives callers flexibility in how they extract the underlying PyIntRef, which is good API design.

crates/vm/src/buffer.rs (3)

598-616: LGTM! Explicit float conversion in pack implementation.

The change to call into_float() explicitly before casting to the target type ($T) makes the conversion path clear. This is consistent with the refactor's goal to replace implicit Deref with explicit conversions.


622-638: LGTM! Consistent float conversion for f16 packing.

The use of into_float() follows the same pattern as the generic float packing implementation above. The conversion to f64 before constructing f16 remains clear and explicit.


650-661: LGTM! Explicit boolean conversion in pack implementation.

The call to into_bool() before casting to u8 makes the conversion explicit and consistent with the float packing implementations. This completes the pattern for all primitive conversions in the buffer packing code.

crates/vm/src/stdlib/builtins.rs (5)

46-53: LGTM: Explicit boolean conversion improves clarity.

The explicit into_bool() call makes the type conversion from ArgIntoBool to bool clear and visible, improving code readability while maintaining the same semantics.


55-63: LGTM: Consistent with the refactoring pattern.

The explicit conversion aligns with the changes in all() and maintains consistency across the codebase.


452-457: LGTM: Explicit int conversion is correct.

The two-step conversion (into_int_ref() followed by as_bigint()) correctly replaces the previous Deref-based access pattern.


689-700: LGTM: Consistent with hex() refactoring.

The conversion pattern matches the one used in hex(), maintaining consistency across similar builtin functions.


763-796: LGTM: Clean flush condition check.

The into() conversion for the flush option is idiomatic and maintains the same behavior while being more explicit.

crates/stdlib/src/statistics.rs (1)

122-131: LGTM: Clear float conversions for mathematical function.

The explicit into_float() conversions make it clear that the wrapper types are being converted to primitive f64 values before passing to the mathematical calculation. This improves code readability without changing behavior.

crates/stdlib/src/bisect.rs (1)

24-29: LGTM: Correct two-step conversion for ArgIndex.

The conversion chain (into_int_ref() followed by try_to_primitive()) correctly handles the refactored ArgIndex type, maintaining the same behavior with explicit conversion steps.

crates/vm/src/function/protocol.rs (1)

136-139: Excellent addition: Explicit accessor improves API clarity.

Adding the obj() accessor method instead of relying on Deref coercion is good API design. This follows Rust best practices where Deref should typically be reserved for smart pointer types. The explicit accessor makes the intent clear at call sites.

crates/vm/src/builtins/mappingproxy.rs (2)

120-129: LGTM: Proper usage of new obj() accessor.

The explicit mapping.obj() call correctly uses the new accessor method to access the inner PyObject, making the code more explicit and maintainable.


163-173: LGTM: Consistent accessor usage.

Using d.obj() to access the inner object for the method call is consistent with the pattern established in _contains() and maintains clarity about the object being operated on.

crates/stdlib/src/array.rs (1)

567-573: LGTM! Explicit float conversion improves clarity.

The changes replace implicit dereferencing with explicit into_float() conversions for both f32 and f64 helper functions. The f32 downcast on line 568 correctly narrows the f64 result to f32, maintaining the original behavior while making the conversion path explicit.

crates/stdlib/src/cmath.rs (1)

25-143: LGTM! Consistent explicit conversions throughout the module.

All complex math functions now consistently use into_complex() and into_float() to convert arguments before performing operations. The log function correctly accesses the real component via base.into_complex().re for the logarithm base, and the isclose function properly uses map_or with the conversion methods. The changes maintain identical behavior while improving code clarity.

crates/stdlib/src/math.rs (11)

30-46: LGTM! Macro correctly uses explicit conversion.

The call_math_func! macro now explicitly converts the argument via into_float() before calling the math function, making the conversion path clear while preserving the original behavior and error handling.


56-114: LGTM! Consistent explicit conversions in predicate and comparison functions.

The isfinite, isinf, isnan, and isclose functions now use explicit into_float() conversions. The isclose function correctly uses map_or to handle optional tolerance parameters with inline conversion, maintaining the original logic while improving clarity.


117-243: LGTM! Power, logarithmic, and root functions correctly updated.

All functions (copysign, log, log1p, pow, sqrt, isqrt) now use explicit conversions:

  • into_float() for floating-point arguments
  • Into::into where type inference is sufficient (line 139)
  • into_int_ref() for integer arguments (line 235)

The mix of into_float() and Into::into is intentional and appropriate for type inference contexts.


245-479: LGTM! Trigonometric and hyperbolic functions correctly refactored.

All trigonometric (acos, asin, atan, atan2, cos, sin, tan, degrees, radians) and hyperbolic (acosh, asinh, atanh, cosh, sinh, tanh) functions consistently use into_float() for explicit conversion. The mixed use of into_float() and into() (e.g., line 272: y.into_float().atan2(x.into())) is appropriate for type inference.


482-500: LGTM! Special functions use trait-based conversion.

The special functions (erf, erfc, gamma, lgamma) use Into::into for conversion to the pymath library types, which is appropriate for generic trait-based conversions.


547-555: LGTM! frexp uses type inference for conversion.

The function correctly uses Into trait conversion with explicit type annotation (let value: f64 = x.into();), which is clear and allows type inference.


639-667: LGTM! Integer operations correctly use reference access.

The helper function math_perf_arb_len_int_op and its callers (gcd, lcm) correctly use as_ref() to access already-converted ArgIndex values. This is distinct from the initial conversion pattern using into_int_ref() seen elsewhere, and is appropriate for this context where values are stored in a vector and accessed multiple times.


671-770: LGTM! Aggregate and sequence functions correctly updated.

Functions cbrt, fsum correctly use explicit conversions. The fsum function on line 682 converts each iterated object via obj?.into_float(), maintaining the accumulation logic while making conversions explicit.


792-862: LGTM! Combinatorial functions correctly use integer conversions.

The perm and comb functions correctly use into_int_ref() for their integer arguments (lines 797, 802, 826, 828), ensuring proper conversion to BigInt references for combinatorial calculations.


865-906: LGTM! modf and nextafter correctly refactored.

The modf function uses into_float() for conversion, and nextafter correctly uses:

  • into_int_ref() for the optional steps parameter (line 892)
  • Into::into for the x and y float parameters (lines 895-896)

This mixed approach is appropriate and maintains the original behavior.


909-1051: LGTM! Remaining functions correctly use explicit conversions.

The ulp, fmod, remainder, and fma functions all correctly use explicit into_float() conversions. The fma function properly converts all three arguments (lines 1033-1036) before calling mul_add, maintaining the fused multiply-add semantics with explicit conversion paths.


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.

@youknowone youknowone marked this pull request as ready for review January 1, 2026 09:35
@youknowone youknowone merged commit d3e2fa4 into RustPython:main Jan 1, 2026
13 checks passed
@youknowone youknowone deleted the deref branch January 1, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant