Conversation
📝 WalkthroughWalkthroughThis pull request introduces platform-specific math library implementations: UNIX and Windows targets use FFI bindings to the native libm, while other platforms (WASM, etc.) use a pure Rust implementation that delegates to the libm crate dependency. The change reorganizes module structure through conditional compilation while cleaning up unused FFI declarations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/m_rust.rs:
- Around line 159-176: The wrappers frexp and modf currently take safe
references (&mut i32, &mut f64) but the FFI declarations in m_sys.rs expect raw
pointers (*mut i32, *mut f64); change frexp to pub fn frexp(n: f64, exp: *mut
i32) -> f64 and modf to pub fn modf(n: f64, iptr: *mut f64) -> f64, dereference
the pointers inside an unsafe block to assign the returned exponent/int (e.g.,
let (mantissa, exponent) = libm::frexp(n); unsafe { if !exp.is_null() { *exp =
exponent } } ), and similarly for modf, preserving return values; ensure ldexp
remains unchanged, and run the suggested rg search to verify/update all call
sites that pass raw pointers.
- Around line 207-211: m_sys.rs is missing the FFI binding for the sincos
function defined under the complex feature in m_rust.rs, causing unresolved
calls to m::sincos from the cmath module; add the FFI declaration matching the
Rust fallback: declare pub fn sincos(x: f64) -> (f64, f64); inside m_sys.rs
(under the same cfg(feature = "complex")/platform-appropriate block) so calls
from trigonometric.rs, misc.rs and exponential.rs link correctly on unix/windows
when the complex feature is enabled.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Cargo.tomlsrc/err.rssrc/lib.rssrc/m_rust.rssrc/m_sys.rs
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Files:
src/err.rssrc/lib.rssrc/m_sys.rssrc/m_rust.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*.rs : Error handling: Return Result directly with Err(crate::Error::EDOM) or Err(crate::Error::ERANGE) instead of using set_errno. Only valid set_errno call is set_errno(0) to clear errno before libm calls.
Applied to files:
src/err.rssrc/lib.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math.rs : Create missing helpers if CPython has a helper we don't have yet, such as math_1_fn for Rust function pointers or special case handlers for specific functions.
Applied to files:
src/err.rsCargo.tomlsrc/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use crate::mul_add(a, b, c) instead of a * b + c for expressions of the form a * b + c or c + a * b to match CPython's FMA (fused multiply-add) behavior on platforms like macOS where it's enabled.
Applied to files:
Cargo.tomlsrc/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : Use mul_add in cmath functions for expressions like 1.0 + x * x, cross-product calculations (mul_add(s1.re, s2.im, -(s2.re * s1.im))), and squared terms to achieve bit-exact matching with CPython.
Applied to files:
Cargo.tomlsrc/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/**/*.rs : Use existing CPython helper equivalents: FUNC1 → math_1(x, func, can_overflow), FUNC2 → math_2(x, y, func), m_log/m_log10/m_log2 in exponential.rs. If CPython uses a helper, use the corresponding Rust helper.
Applied to files:
Cargo.tomlsrc/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/**/*.rs : For complex math (cmath module), copy special_type() enum and function, 7x7 special value tables (e.g., tanh_special_values), SPECIAL_VALUE macro usage, and complex-specific error handling from Modules/cmathmodule.c.
Applied to files:
src/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/cmath/trigonometric.rs : In cmath functions that use both sin and cos of the same angle (cosh, sinh, exp, rect), use m::sincos(x) which returns (sin, cos) tuple instead of calling sin and cos separately to match CPython's macOS __sincos_stret behavior.
Applied to files:
src/lib.rssrc/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/math/exponential.rs : Copy CPython's explicit special case handling for IEEE specials (NaN, Inf, etc.) exactly, including logic for cases like NaN**0 = 1.
Applied to files:
src/m_sys.rssrc/m_rust.rs
📚 Learning: 2026-01-10T13:40:01.981Z
Learnt from: CR
Repo: RustPython/pymath PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T13:40:01.981Z
Learning: Applies to src/**/*test*.rs : All float functions must be tested with crate::test::EDGE_VALUES which includes zeros, infinities, NaNs with different payloads, subnormals, boundary values (MIN_POSITIVE, MAX, MIN), large values near infinity, and trigonometric special values (PI, PI/2, PI/4, TAU).
Applied to files:
src/m_sys.rssrc/m_rust.rs
🧬 Code graph analysis (1)
src/m_rust.rs (1)
src/m_sys.rs (37)
acos(9-9)asin(10-10)atan(11-11)atan2(12-12)cos(13-13)sin(14-14)tan(15-15)acosh(18-18)asinh(19-19)atanh(20-20)cosh(21-21)sinh(22-22)tanh(23-23)exp(26-26)exp2(27-27)expm1(28-28)log(30-30)log10(31-31)log1p(32-32)log2(34-34)cbrt(37-37)hypot(40-40)pow(43-43)sqrt(44-44)ceil(47-47)copysign(48-48)fabs(49-49)floor(51-51)fmod(52-52)frexp(53-53)ldexp(54-54)modf(55-55)nextafter(56-56)remainder(57-57)trunc(58-58)erf(61-61)erfc(62-62)
🔇 Additional comments (5)
src/err.rs (1)
47-50: LGTM!The multi-line formatting of the
cfgattributes improves readability without changing behavior. The logic correctly handles the catch-all case for other Unix variants using__error().Also applies to: 79-82
Cargo.toml (1)
25-28: LGTM!The target-specific dependency correctly enables the
libmcrate only on platforms without native libm (WASM, etc.), matching the conditional module selection insrc/lib.rs.src/m_rust.rs (1)
1-203: LGTM overall - clean wrapper implementation.The pure Rust wrappers correctly delegate to the
libmcrate with#[inline(always)]for zero-overhead abstraction. The function signatures match the standard libm API for the f64 variants.src/lib.rs (1)
8-16: Platform-specific module routing is correct.The conditional compilation correctly routes:
- Unix/Windows → native FFI (via
mod minsrc/m.rs)- Other targets (WASM) → pure Rust (
m_rust.rs)The
src/m.rsfile exists and provides safe wrapper functions around allm_sysFFI calls, as expected.src/m_sys.rs (1)
1-68: No action required. The f32 functions declared inm_sys.rs(expm1f, log1pf, cbrtf, hypotf, fdimf, erff, erfcf, lgammaf_r, tgammaf) are raw FFI declarations that are not wrapped inm.rsand therefore are not part of the publiccrate::mAPI. They cannot be called through the module's public interface, and WASM builds are unaffected sincem_sys.rsis only compiled on unix/windows platforms.Likely an incorrect or invalid review comment.
| #[inline(always)] | ||
| pub fn frexp(n: f64, exp: &mut i32) -> f64 { | ||
| let (mantissa, exponent) = libm::frexp(n); | ||
| *exp = exponent; | ||
| mantissa | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| pub fn ldexp(x: f64, n: i32) -> f64 { | ||
| libm::ldexp(x, n) | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| pub fn modf(n: f64, iptr: &mut f64) -> f64 { | ||
| let (frac, int) = libm::modf(n); | ||
| *iptr = int; | ||
| frac | ||
| } |
There was a problem hiding this comment.
Signature mismatch: frexp and modf use references vs raw pointers.
The m_sys.rs FFI declarations use raw pointers (*mut i32, *mut f64), while these pure Rust wrappers use safe references (&mut i32, &mut f64). If callers use raw pointers (common in FFI-adjacent code), this will cause compilation errors on WASM.
Verify if existing call sites use references or raw pointers:
#!/bin/bash
# Check how frexp and modf are called in the codebase
rg -n '\bm::(frexp|modf)\s*\(' src/ -A2🤖 Prompt for AI Agents
In @src/m_rust.rs around lines 159 - 176, The wrappers frexp and modf currently
take safe references (&mut i32, &mut f64) but the FFI declarations in m_sys.rs
expect raw pointers (*mut i32, *mut f64); change frexp to pub fn frexp(n: f64,
exp: *mut i32) -> f64 and modf to pub fn modf(n: f64, iptr: *mut f64) -> f64,
dereference the pointers inside an unsafe block to assign the returned
exponent/int (e.g., let (mantissa, exponent) = libm::frexp(n); unsafe { if
!exp.is_null() { *exp = exponent } } ), and similarly for modf, preserving
return values; ensure ldexp remains unchanged, and run the suggested rg search
to verify/update all call sites that pass raw pointers.
| #[cfg(feature = "complex")] | ||
| #[inline(always)] | ||
| pub fn sincos(x: f64) -> (f64, f64) { | ||
| (libm::sin(x), libm::cos(x)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for sincos declaration in m_sys.rs
grep -n "sincos" src/m_sys.rsRepository: RustPython/pymath
Length of output: 178
🏁 Script executed:
# Check for m::sincos usage in cmath module
rg -n 'm::sincos' src/cmath/Repository: RustPython/pymath
Length of output: 349
🏁 Script executed:
# Check the module structure to understand how m:: is resolved
cat -n src/lib.rs | head -80Repository: RustPython/pymath
Length of output: 3360
🏁 Script executed:
# Look for any sincos declarations across the codebase
rg -n "fn sincos" src/ --type rsRepository: RustPython/pymath
Length of output: 87
🏁 Script executed:
# Check if there's feature gating around sincos in m_rust.rs
rg -B 5 -A 5 "sincos" src/m_rust.rsRepository: RustPython/pymath
Length of output: 316
🏁 Script executed:
# Get line count of m_sys.rs
wc -l src/m_sys.rsRepository: RustPython/pymath
Length of output: 76
🏁 Script executed:
# Read m_sys.rs to see if sincos is missing or if there's feature gating
cat -n src/m_sys.rsRepository: RustPython/pymath
Length of output: 2731
🏁 Script executed:
# Verify compilation by checking Cargo.toml features
cat src/../Cargo.toml | grep -A 20 "\[features\]"Repository: RustPython/pymath
Length of output: 992
Add sincos FFI declaration to m_sys.rs.
The sincos function is defined in m_rust.rs (lines 207-211) with the complex feature gate, but m_sys.rs lacks the corresponding FFI binding. On unix/windows targets with the complex feature enabled (default), the cmath module calls m::sincos at 4 locations (trigonometric.rs:121, 167; misc.rs:72; exponential.rs:115), which would fail to compile because sincos is not declared in the system libm FFI. Add the FFI declaration: pub fn sincos(x: f64) -> (f64, f64); to match the fallback implementation in m_rust.rs and enable CPython-compatible behavior on unix/windows.
🤖 Prompt for AI Agents
In @src/m_rust.rs around lines 207 - 211, m_sys.rs is missing the FFI binding
for the sincos function defined under the complex feature in m_rust.rs, causing
unresolved calls to m::sincos from the cmath module; add the FFI declaration
matching the Rust fallback: declare pub fn sincos(x: f64) -> (f64, f64); inside
m_sys.rs (under the same cfg(feature = "complex")/platform-appropriate block) so
calls from trigonometric.rs, misc.rs and exponential.rs link correctly on
unix/windows when the complex feature is enabled.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.