diff --git a/crates/openjd-expr/src/functions/arithmetic.rs b/crates/openjd-expr/src/functions/arithmetic.rs index e71f9f2f..4a9aa887 100644 --- a/crates/openjd-expr/src/functions/arithmetic.rs +++ b/crates/openjd-expr/src/functions/arithmetic.rs @@ -107,8 +107,14 @@ pub fn pow_int(_: Ctx, a: &[ExprValue]) -> R { "Cannot raise zero to a negative power", )); } - let exp32 = i32::try_from(*exp).unwrap_or(i32::MIN); - return Ok(ExprValue::Float(Float64::new((*base as f64).powi(exp32))?)); + // Use `powf`, not `powi`: `powi` evaluates via repeated squaring + // and accumulates a last-ulp error (e.g. `956 ** -74` came out + // as ...9224e-221 instead of ...924e-221), whereas Python + // computes int-base/negative-exponent power in float and matches + // `powf` exactly. + return Ok(ExprValue::Float(Float64::new( + (*base as f64).powf(*exp as f64), + )?)); } // Guard: exponent > 63 with |base| > 1 always overflows i64 if *exp > 63 && !matches!(*base, -1..=1) { @@ -185,8 +191,41 @@ pub fn floordiv_float(_: Ctx, a: &[ExprValue]) -> R { if r == 0.0 { return Err(ExpressionError::division_by_zero("Division")); } - let v = (l / r).floor(); - if v.abs() > i64::MAX as f64 { + // Float floor-division is NOT `(l / r).floor()`: because the true quotient + // is only approximated in f64, plain flooring gives the wrong integer at + // ties (e.g. `205 // 0.1` → 2050 instead of Python's 2049, since 0.1 is + // slightly more than 1/10). Reproduce CPython's `float_divmod` exactly — + // derive the quotient from the `fmod` remainder, apply the floored-division + // sign correction, then round with CPython's `> 0.5` nudge. + let v = { + let modv = l % r; // fmod: remainder with the dividend's sign + let mut div = (l - modv) / r; + // Floored-division correction: when the remainder's sign disagrees with + // the divisor's, the true quotient is one less than the truncated one. + // (CPython also adjusts `mod` here, but floordiv only needs `div`.) + if modv != 0.0 && (r < 0.0) != (modv < 0.0) { + div -= 1.0; + } + if div != 0.0 { + let floordiv = div.floor(); + if div - floordiv > 0.5 { + floordiv + 1.0 + } else { + floordiv + } + } else { + // Sign of a zero quotient follows l/r; as an integer it is just 0. + 0.0 + } + }; + // Compare against the exact 2^63 bound with `>=`: `i64::MAX as f64` rounds + // up to 2^63, so `> i64::MAX as f64` lets a value of exactly 2^63 through, + // which then saturates to i64::MAX on the cast — a silent wrong result + // (e.g. `9.223372036854776e18 // 1` returned i64::MAX instead of erroring, + // where Python raises overflow). i64::MIN (-2^63) is representable, so the + // low side stays inclusive. + const TWO_POW_63: f64 = 9_223_372_036_854_775_808.0; + if !v.is_finite() || !(-TWO_POW_63..TWO_POW_63).contains(&v) { return Err(ExpressionError::integer_overflow()); } Ok(ExprValue::Int(v as i64)) @@ -197,8 +236,18 @@ pub fn mod_float(_: Ctx, a: &[ExprValue]) -> R { if r == 0.0 { return Err(ExpressionError::division_by_zero("Modulo")); } - // Python uses floored modulo: l - r * floor(l / r) - Ok(ExprValue::Float(Float64::new(l - r * (l / r).floor())?)) + // Python's float `%` is floored (the result takes the *divisor's* sign), + // but computing it as `l - r * floor(l / r)` loses all precision when + // `|l/r|` is huge: `9.2e18 % 0.1` rounds `l/r` to ~9.2e19 and cancels to + // `0.0` instead of `2.84e-14`. Match CPython exactly: start from C `fmod` + // (`f64::rem`, which is truncated toward zero and numerically exact), then + // adjust by one divisor when the remainder's sign disagrees with the + // divisor's. See CPython `float___mod__`. + let mut m = l % r; + if m != 0.0 && (m < 0.0) != (r < 0.0) { + m += r; + } + Ok(ExprValue::Float(Float64::new(m)?)) } pub fn pow_float(_: Ctx, a: &[ExprValue]) -> R { @@ -270,7 +319,21 @@ pub fn mul_string(ctx: Ctx, a: &[ExprValue]) -> R { if *n < 0 { return Ok(ExprValue::String(String::new())); } - let result_len = s.len() * (*n as usize); + // `s.len() * n` overflows `usize` for large `n` (e.g. i64::MAX), + // which panics under overflow-checks and silently wraps in release + // — the latter would slip a bogus-small length past the memory + // guard and then attempt a catastrophic `repeat`. On overflow the + // true length is astronomically over any limit, so route the + // maximum through the normal memory check to produce the same + // memory-limit error (matching Python's "String repetition would + // exceed memory limit"). + let result_len = match s.len().checked_mul(*n as usize) { + Some(len) => len, + None => { + ctx.check_memory(usize::MAX)?; + unreachable!("check_memory(usize::MAX) always exceeds the limit") + } + }; ctx.count_string_ops(result_len)?; ctx.check_memory(result_len)?; Ok(ExprValue::String(s.repeat(*n as usize))) diff --git a/crates/openjd-expr/src/functions/conversion.rs b/crates/openjd-expr/src/functions/conversion.rs index b131fe43..c894e99f 100644 --- a/crates/openjd-expr/src/functions/conversion.rs +++ b/crates/openjd-expr/src/functions/conversion.rs @@ -61,7 +61,12 @@ pub fn int_from_string(_: Ctx, a: &[ExprValue]) -> R { pub fn float_from_float(_: Ctx, a: &[ExprValue]) -> R { match &a[0] { - ExprValue::Float(f) => Ok(ExprValue::Float(Float64::new(f.value())?)), + // Identity — return the value *unchanged*, preserving any original + // literal string. Rebuilding via `Float64::new` would drop the + // preserved source text, reformatting e.g. `float(1e308)` from the + // literal `1e308` to the computed form `1e+308` and diverging from + // Python's `_float_identity` (which returns the value as-is). + ExprValue::Float(f) => Ok(ExprValue::Float(f.clone())), _ => Err(ExpressionError::type_error("type error")), } } diff --git a/crates/openjd-expr/src/functions/math.rs b/crates/openjd-expr/src/functions/math.rs index c28c2508..bdd2f825 100644 --- a/crates/openjd-expr/src/functions/math.rs +++ b/crates/openjd-expr/src/functions/math.rs @@ -11,6 +11,23 @@ use crate::value::{ExprValue, Float64}; type R = Result; type Ctx<'a> = &'a mut dyn EvalContext; +/// Convert a whole-valued `f64` to `i64`, erroring on out-of-range input. +/// +/// The naive guard `v.abs() > i64::MAX as f64` is subtly wrong: `i64::MAX as +/// f64` rounds *up* to `2^63` (i64::MAX is not representable in f64), so a value +/// of exactly `2^63` slips past `>` and then saturates to `i64::MAX` on the +/// `as i64` cast — a silent wrong result (e.g. `floor(-float(-i64::MAX))` +/// returned `i64::MAX` instead of erroring). Compare against the exact `2^63` +/// bound with `>=` on the positive side; `i64::MIN` (`-2^63`) *is* representable, +/// so the negative side stays inclusive. +fn f64_to_i64_checked(v: f64) -> R { + const TWO_POW_63: f64 = 9_223_372_036_854_775_808.0; // 2^63 == -(i64::MIN) + if !v.is_finite() || !(-TWO_POW_63..TWO_POW_63).contains(&v) { + return Err(ExpressionError::integer_overflow()); + } + Ok(ExprValue::Int(v as i64)) +} + fn min_max_items(a: &[ExprValue], name: &str) -> Result, ExpressionError> { if a.is_empty() { return Err(ExpressionError::new(format!( @@ -97,13 +114,7 @@ fn round_half_even(x: f64) -> f64 { pub fn floor_float(_: Ctx, a: &[ExprValue]) -> R { match &a[0] { - ExprValue::Float(f) => { - let v = f.floor(); - if v.abs() > i64::MAX as f64 { - return Err(ExpressionError::integer_overflow()); - } - Ok(ExprValue::Int(v as i64)) - } + ExprValue::Float(f) => f64_to_i64_checked(f.floor()), _ => Err(ExpressionError::type_error("type error")), } } @@ -117,13 +128,7 @@ pub fn floor_int(_: Ctx, a: &[ExprValue]) -> R { pub fn ceil_float(_: Ctx, a: &[ExprValue]) -> R { match &a[0] { - ExprValue::Float(f) => { - let v = f.ceil(); - if v.abs() > i64::MAX as f64 { - return Err(ExpressionError::integer_overflow()); - } - Ok(ExprValue::Int(v as i64)) - } + ExprValue::Float(f) => f64_to_i64_checked(f.ceil()), _ => Err(ExpressionError::type_error("type error")), } } @@ -135,67 +140,162 @@ pub fn ceil_int(_: Ctx, a: &[ExprValue]) -> R { } } +/// Largest positive `ndigits` accepted by `round(x, ndigits)`. The Python +/// reference raises "precision too big" once `ndigits` exceeds the platform C +/// `int` range (`i32::MAX`); match that boundary exactly. +const MAX_ROUND_NDIGITS: i64 = i32::MAX as i64; + +/// Round `value` to the nearest multiple of `10^magnitude` (i.e. `round(value, +/// -magnitude)`), robust to astronomically large `magnitude`. +/// +/// `magnitude` can be as large as `i64::MIN.unsigned_abs()` when a caller passes +/// `round(x, i64::MIN)`. Computing `10f64.powi(magnitude)` there overflows to +/// infinity, and the naive `round(value / factor) * factor` then yields `NaN`. +/// Python returns `0` in that regime (any finite value is nearer to `0` than to +/// the first nonzero multiple of an enormous power of ten), so we special-case a +/// zero scaled result to `0.0` instead of `0 * inf`. +fn round_to_neg_power(value: f64, magnitude: u64) -> f64 { + let factor = if magnitude > 308 { + f64::INFINITY + } else { + 10f64.powi(magnitude as i32) + }; + let scaled = round_half_even(value / factor); + if scaled == 0.0 { + 0.0 + } else { + scaled * factor + } +} + pub fn round_fn(_: Ctx, a: &[ExprValue]) -> R { + // Extract the optional `ndigits` argument shared by both numeric branches. + // `None` (single-arg `round`) is distinct from `Some(0)`: both yield an int + // result here, but keeping the distinction mirrors the Python overloads. + let ndigits = match a.get(1) { + Some(ExprValue::Int(n)) => Some(*n), + Some(_) => return Err(ExpressionError::new("round() ndigits must be int")), + None => None, + }; match &a[0] { ExprValue::Float(f) => { - let has_ndigits = a.len() > 1; - let ndigits = a - .get(1) - .and_then(|v| match v { - ExprValue::Int(n) => Some(*n), - _ => None, - }) - .unwrap_or(0); - if !has_ndigits { - let v = round_half_even(f.value()); - if v.abs() > i64::MAX as f64 { - return Err(ExpressionError::integer_overflow()); + match ndigits { + // No ndigits, or ndigits <= 0: Python returns an *int*. + None => f64_to_i64_checked(round_half_even(f.value())), + Some(n) if n <= 0 => { + // A float with |v| >= 2^52 is already integral, so rounding + // it to a negative power smaller than its magnitude is a + // no-op. Returning it directly also dodges the overshoot in + // `round_to_neg_power`'s `v/factor*factor` round-trip, which + // for values near 2^63 lands just past the i64 range and + // spuriously overflows. (A rare 1-ulp double-rounding + // disagreement with the Python reference can remain at this + // magnitude, but avoiding the spurious overflow is the + // important part.) When 10^|n| exceeds the magnitude, the + // value is small enough that `round_to_neg_power` handles it + // and correctly yields 0. + const NO_FRACTION_ABOVE: f64 = 4_503_599_627_370_496.0; // 2^52 + let v = f.value(); + if v.abs() >= NO_FRACTION_ABOVE { + f64_to_i64_checked(v) + } else { + f64_to_i64_checked(round_to_neg_power(v, n.unsigned_abs())) + } } - Ok(ExprValue::Int(v as i64)) - } else if ndigits >= 0 { - let factor = 10f64.powi(ndigits as i32); - let rounded = round_half_even(f.value() * factor) / factor; - if ndigits == 0 { + // ndigits > 0: Python returns a *float*, formatted to that many + // decimal places. + Some(n) => { + if n > MAX_ROUND_NDIGITS { + return Err(ExpressionError::new("round() precision too big")); + } + let n = n as i32; + // Any f64 with |v| >= 2^52 already has no fractional bits, + // so rounding it to `n > 0` decimal places is a no-op — and + // the naive `v * 10^n / 10^n` round-trip would only inject + // error (e.g. -9.2e19 came back as ...16384 instead of the + // exact ...00000). Likewise `n >= 17` exceeds f64's decimal + // precision, and `v * 10^n` may overflow to infinity for + // large v. In all these cases return the value unchanged, + // matching Python's arbitrary-precision `round` (a no-op + // here) and avoiding a spurious infinity error. + const NO_FRACTION_ABOVE: f64 = 4_503_599_627_370_496.0; // 2^52 + let v = f.value(); + let rounded = if n >= 17 || v.abs() >= NO_FRACTION_ABOVE { + v + } else { + let scaled = v * 10f64.powi(n); + if scaled.is_finite() { + round_half_even(scaled) / 10f64.powi(n) + } else { + v + } + }; Ok(ExprValue::Float(Float64::with_str( rounded, - format!("{}.0", rounded as i64), - )?)) - } else { - Ok(ExprValue::Float(Float64::with_str( - rounded, - format!("{:.prec$}", rounded, prec = ndigits as usize), + format!("{:.prec$}", rounded, prec = n as usize), )?)) } - } else { - let factor = 10f64.powi((-ndigits) as i32); - Ok(ExprValue::Float(Float64::new( - round_half_even(f.value() / factor) * factor, - )?)) - } - } - ExprValue::Int(i) => { - let ndigits = a - .get(1) - .and_then(|v| match v { - ExprValue::Int(n) => Some(*n), - _ => None, - }) - .unwrap_or(0); - if ndigits >= 0 { - Ok(ExprValue::Int(*i)) - } else { - let factor = 10f64.powi((-ndigits) as i32); - let v = round_half_even(*i as f64 / factor) * factor; - if v.abs() > i64::MAX as f64 { - return Err(ExpressionError::integer_overflow()); - } - Ok(ExprValue::Int(v as i64)) } } + ExprValue::Int(i) => match ndigits { + // Non-negative ndigits on an int is a no-op (Python returns the int + // unchanged); a single-arg `round(int)` likewise returns it as-is. + None => Ok(ExprValue::Int(*i)), + Some(n) if n >= 0 => Ok(ExprValue::Int(*i)), + // Negative ndigits rounds an int to a multiple of 10^k. Done in + // *integer* arithmetic (not via f64) so large magnitudes keep full + // precision — `round(4611686018427387904, -5)` must yield + // `...400000`, not the `...400192` an f64 round-trip produces. + Some(n) => round_int_neg(*i, n.unsigned_abs()), + }, _ => Err(ExpressionError::new("round() requires numeric argument")), } } +/// Round integer `i` to the nearest multiple of `10^k`, ties to even (matching +/// Python's `round(int, -k)`). Exact — no float round-trip. Returns an overflow +/// error if the result leaves the `i64` range. +fn round_int_neg(i: i64, k: u64) -> R { + // 10^k overflows i64 past k == 18; at that scale any i64 rounds to 0 + // (its magnitude is below half of 10^19). + if k >= 19 { + return Ok(ExprValue::Int(0)); + } + let pow = 10i64.pow(k as u32); + let rem = i.rem_euclid(pow); // in [0, pow); floored remainder + // Largest multiple of `pow` that is <= i (works for negatives). `i - rem` + // can underflow for i near i64::MIN, so use checked subtraction; on + // underflow the floor multiple is unrepresentable but the rounded-up value + // may still be, so fall through to the overflow-checked add below. + let down = i.checked_sub(rem); + let half = pow / 2; + // Decide whether to round up to `down + pow`, ties to even. + let round_up = match rem.cmp(&half) { + std::cmp::Ordering::Greater => true, + std::cmp::Ordering::Less => false, + // Exact tie: round to the even multiple. `(i - rem) / pow` is the + // multiple index; round up iff it is odd. Derive it as `i / pow` + // rounded toward negative infinity, avoiding the unrepresentable + // `down` near i64::MIN: floor-div = (i - rem) / pow, and since + // `rem == i.rem_euclid(pow)`, that equals `i.div_euclid(pow)`. + std::cmp::Ordering::Equal => i.div_euclid(pow) % 2 != 0, + }; + let result = if round_up { + // down + pow == i - rem + pow; compute without materializing `down`. + i.checked_sub(rem) + .and_then(|d| d.checked_add(pow)) + .or_else(|| { + // `down` underflowed but `down + pow` may be fine: it equals + // `i + (pow - rem)`, and `pow - rem` is in (0, pow]. + i.checked_add(pow - rem) + }) + .ok_or_else(ExpressionError::integer_overflow)? + } else { + down.ok_or_else(ExpressionError::integer_overflow)? + }; + Ok(ExprValue::Int(result)) +} + pub fn sum_list(ctx: Ctx, a: &[ExprValue]) -> R { if let Some(iter) = a[0].list_iter() { let mut int_sum: i64 = 0; diff --git a/crates/openjd-expr/src/functions/path.rs b/crates/openjd-expr/src/functions/path.rs index 0e7750f0..86b0dda5 100644 --- a/crates/openjd-expr/src/functions/path.rs +++ b/crates/openjd-expr/src/functions/path.rs @@ -429,6 +429,12 @@ pub fn join(left: &str, right: &str, fmt: PathFormat) -> String { if is_absolute(right, fmt) { return right.to_string(); } + // Joining an empty component is a no-op (Python `PurePath / ""` returns the + // path unchanged). Returning early avoids appending a bare trailing + // separator — `path('a') / ''` must be `"a"`, not `"a/"`. + if right.is_empty() { + return left.to_string(); + } // Windows root-relative: /foo or \foo (but not \\server) replaces the path // but keeps the root from left. Matches ntpath.join behavior. // For drive paths (C:\...), the root is "C:". @@ -445,6 +451,12 @@ pub fn join(left: &str, right: &str, fmt: PathFormat) -> String { if let Some(unc_root) = extract_unc_root(left) { return format!("{unc_root}{right}"); } + // Left has no drive and no UNC root: a root-relative right replaces + // the left entirely (matching ntpath.join('a/b', '/x') == '/x'). + // Falling through to the normal join would wrongly keep the left + // (producing e.g. `日\本\tmp\x` instead of `\tmp\x`). Normalize the + // right's separators to the Windows form on the way out. + return crate::value::normalize_path_separators(right, fmt); } } let left_is_uri = crate::uri_path::is_uri(left); diff --git a/crates/openjd-expr/src/functions/string.rs b/crates/openjd-expr/src/functions/string.rs index f852b77d..6a9fc06e 100644 --- a/crates/openjd-expr/src/functions/string.rs +++ b/crates/openjd-expr/src/functions/string.rs @@ -345,7 +345,11 @@ pub fn capitalize_fn(ctx: Ctx, a: &[ExprValue]) -> R { pub fn center_fn(ctx: Ctx, a: &[ExprValue]) -> R { let s = get_str(&a[0])?; let width = match &a[1] { - ExprValue::Int(w) => *w as usize, + // A negative width is not an error in Python — it just means "no + // padding" (the string is already at least that wide). Clamp to 0; + // casting `-1` straight to `usize` would wrap to `usize::MAX` and blow + // the op-count limit (or, without it, attempt a catastrophic alloc). + ExprValue::Int(w) => (*w).max(0) as usize, _ => return Err(ExpressionError::new("center() width must be int")), }; ctx.count_string_ops(width.max(s.len()))?; @@ -353,8 +357,12 @@ pub fn center_fn(ctx: Ctx, a: &[ExprValue]) -> R { if clen >= width { return Ok(ExprValue::String(s.to_string())); } + // Match CPython `str.center`'s bias exactly: when the padding is odd, the + // extra space goes on the *left* iff `marg & width` is odd. The naive + // `left = pad / 2` biases the extra space right instead, which disagrees + // with the Python reference (e.g. `center('ab', 11)` → `' ab '`). let pad = width - clen; - let left = pad / 2; + let left = pad / 2 + (pad & width & 1); let right = pad - left; Ok(ExprValue::String(format!( "{}{}{}", @@ -367,7 +375,8 @@ pub fn center_fn(ctx: Ctx, a: &[ExprValue]) -> R { pub fn ljust_fn(ctx: Ctx, a: &[ExprValue]) -> R { let s = get_str(&a[0])?; let width = match &a[1] { - ExprValue::Int(w) => *w as usize, + // Negative width clamps to 0 (no padding); see `center_fn`. + ExprValue::Int(w) => (*w).max(0) as usize, _ => return Err(ExpressionError::new("ljust() width must be int")), }; ctx.count_string_ops(width.max(s.len()))?; @@ -377,7 +386,8 @@ pub fn ljust_fn(ctx: Ctx, a: &[ExprValue]) -> R { pub fn rjust_fn(ctx: Ctx, a: &[ExprValue]) -> R { let s = get_str(&a[0])?; let width = match &a[1] { - ExprValue::Int(w) => *w as usize, + // Negative width clamps to 0 (no padding); see `center_fn`. + ExprValue::Int(w) => (*w).max(0) as usize, _ => return Err(ExpressionError::new("rjust() width must be int")), }; ctx.count_string_ops(width.max(s.len()))?; diff --git a/crates/openjd-expr/src/value.rs b/crates/openjd-expr/src/value.rs index 32ba9634..df0d22e3 100644 --- a/crates/openjd-expr/src/value.rs +++ b/crates/openjd-expr/src/value.rs @@ -69,7 +69,14 @@ impl Float64 { } Ok(Self { value: v, - original: if v == 0.0 && s != "0.0" { + // For a zero value, drop only a *negative*-zero string (e.g. the + // "-0.00" that `round(-0.001, 2)` formats) so it normalizes to the + // canonical "0.0" — matching Python, which collapses signed zero. + // A non-negative zero string is preserved verbatim, so legitimate + // trailing zeros like `round(0.0, 7)` → "0.0000000" survive rather + // than being flattened to "0.0". Previously any zero whose string + // wasn't exactly "0.0" was discarded, which dropped those too. + original: if v == 0.0 && s.starts_with('-') { None } else { Some(s.into_boxed_str()) @@ -1133,8 +1140,14 @@ impl ExprValue { (Self::Bool(a), Self::Bool(b)) => Ok(a.cmp(b)), (Self::String(a), Self::String(b)) => Ok(a.cmp(b)), (Self::Path { value: a, .. }, Self::Path { value: b, .. }) => Ok(a.cmp(b)), - (Self::String(a), Self::Path { value: b, .. }) - | (Self::Path { value: b, .. }, Self::String(a)) => Ok(a.cmp(b)), + // Mixed string/path compares by string value — but the operand + // order must be preserved. The previous combined arm bound the + // string to `a` and the path to `b` in *both* directions, silently + // swapping the operands when the path was on the left (so + // `path('/tmp/x') < 'ab'` compared `'ab'` against `'/tmp/x'` and + // returned the wrong result). Keep left-vs-right straight. + (Self::String(a), Self::Path { value: b, .. }) => Ok(a.cmp(b)), + (Self::Path { value: a, .. }, Self::String(b)) => Ok(a.cmp(b)), _ if self.is_list() && other.is_list() => { let (a_iter, b_iter) = match (self.list_iter(), other.list_iter()) { (Some(a), Some(b)) => (a, b), @@ -1248,12 +1261,25 @@ pub fn format_float(f: f64) -> String { } let abs = f.abs(); if !(1e-4..1e16).contains(&abs) { - format!("{:e}", f) - .replace("e-0", "e-") - .replace("e0", "e+0") - .replace("e", "e+") - .replace("e+-", "e-") - .replace("e++", "e+") + // Scientific notation, matching Python/C repr: a shortest-round-trip + // mantissa and an exponent written as `e` + sign + at least two digits + // (`1e+308`, `9.151416593531595e-07`). Rust's `{:e}` gives the right + // shortest mantissa but a sign-less, un-padded exponent (`1e308`, + // `...e-7`), so reformat the exponent explicitly. The previous + // chained `.replace(...)` approach mangled single-digit exponents + // (leaving `e-7`/`e18`), which diverged from the Python reference. + let sci = format!("{f:e}"); // e.g. "-9.151416593531595e-7" + match sci.split_once('e') { + Some((mantissa, exp)) => { + let exp: i32 = exp.parse().unwrap_or(0); + format!( + "{mantissa}e{}{:02}", + if exp < 0 { '-' } else { '+' }, + exp.abs() + ) + } + None => sci, + } } else if f.fract() == 0.0 { format!("{}.0", f as i64) } else { diff --git a/crates/openjd-expr/tests/integration/test_arithmetic.rs b/crates/openjd-expr/tests/integration/test_arithmetic.rs index 3c0cc644..47f6e052 100644 --- a/crates/openjd-expr/tests/integration/test_arithmetic.rs +++ b/crates/openjd-expr/tests/integration/test_arithmetic.rs @@ -92,6 +92,16 @@ fn float_precision_display() { assert_eq!(eval("0.1 + 0.2").to_display_string(), "0.30000000000000004"); } +// A computed float outside [1e-4, 1e16) uses scientific notation with a C/Python +// style exponent: sign plus at least two digits (`e+18`, `e-07`), not Rust's +// bare `e18` / `e-7`. +#[test] +fn float_scientific_notation_exponent_format() { + assert_eq!(eval("9.2e18 + 0.0").to_display_string(), "9.2e+18"); + assert_eq!(eval("1e17 * 10.0").to_display_string(), "1e+18"); + assert_eq!(eval("5e-8 + 0.0").to_display_string(), "5e-08"); +} + #[test] fn float_passthrough_preserves_original() { let mut st = SymbolTable::new(); @@ -155,6 +165,32 @@ fn floordiv_float_truncates_negative() { assert_eq!(eval("-7.5 // 2.0").to_display_string(), "-4"); } +// Float floor-division must reproduce CPython's `float_divmod`, not the naive +// `(l/r).floor()`: because the true quotient is only approximated in f64, plain +// flooring rounds the wrong way at ties. `0.1` is slightly greater than 1/10, +// so `205 / 0.1` is just under 2050 and Python floors it to 2049. +#[test] +fn floordiv_float_tie_matches_cpython() { + assert_eq!(eval("205 // 0.1").to_display_string(), "2049"); +} + +// Large-magnitude float floor-division keeps full precision (no `(l/r).floor()` +// round-trip loss): 9.2e18 // -568 == -16197183098591552, exactly. +#[test] +fn floordiv_float_large_magnitude() { + assert_eq!( + eval("abs(-9.2e18) // -568").to_display_string(), + "-16197183098591552" + ); +} + +// A float floor-division whose quotient exceeds i64 is an overflow error, not a +// saturating cast. `9.223372036854776e18` is exactly 2^63 as an f64. +#[test] +fn floordiv_float_overflow_is_error() { + assert_err("9.223372036854776e18 // 1", &["Integer overflow"]); +} + #[test] fn floordiv_by_zero_int() { assert_err( @@ -163,6 +199,24 @@ fn floordiv_by_zero_int() { ); } +// Float modulo must use CPython's fmod-based algorithm, not `l - r*floor(l/r)`, +// which loses all precision when |l/r| is huge: `9.2e18 % 0.1` collapses to 0.0 +// under the naive form but is 2.842170943040401e-14 under fmod. +#[test] +fn mod_float_large_magnitude_precision() { + assert_eq!( + eval("9223372036854775807 % 0.1").to_display_string(), + "2.842170943040401e-14" + ); +} + +// Floored modulo: the result takes the divisor's sign (matching Python). +#[test] +fn mod_float_sign_follows_divisor() { + assert_eq!(eval("-5.5 % 2.0").to_display_string(), "0.5"); + assert_eq!(eval("5.5 % -2.0").to_display_string(), "-0.5"); +} + #[test] fn floordiv_by_zero_float() { assert_err( @@ -192,6 +246,17 @@ fn int_power_zero() { assert_eq!(eval("5 ** 0").to_display_string(), "1"); } +// int ** negative exponent must be computed as float `powf`, not `powi` +// (repeated squaring): `powi` accumulates a last-ulp error. `956 ** -74` is +// 2.793289646093924e-221 under powf, matching CPython's float `pow`. +#[test] +fn int_power_negative_exponent_precision() { + assert_eq!( + eval("956 ** -74").to_display_string(), + "2.793289646093924e-221" + ); +} + #[test] fn float_power() { let r = eval("2.0 ** 3.0"); @@ -361,11 +426,16 @@ fn round_ndigits_0_75() { } #[test] fn round_ndigits_2_5_0() { - assert_eq!(eval("round(2.5, 0)").to_display_string(), "2.0"); + // round(float, 0) returns an INT, matching Python (banker's rounding). + assert_eq!(eval("round(2.5, 0)").to_display_string(), "2"); + assert_eq!( + eval("round(2.5, 0)").expr_type(), + openjd_expr::ExprType::INT + ); } #[test] fn round_ndigits_3_5_0() { - assert_eq!(eval("round(3.5, 0)").to_display_string(), "4.0"); + assert_eq!(eval("round(3.5, 0)").to_display_string(), "4"); } #[test] @@ -401,6 +471,38 @@ fn round_int_ndigits_250_neg2() { assert_eq!(eval("round(250, -2)").to_display_string(), "200"); } +// round(int, -k) is exact integer arithmetic: a large int keeps every digit +// above the rounding position. An f64 round-trip would corrupt the low digits +// (e.g. yield ...400192 instead of ...400000). +#[test] +fn round_int_ndigits_large_exact() { + assert_eq!( + eval("round(4611686018427387904, -5)").to_display_string(), + "4611686018427400000" + ); + assert_eq!( + eval("round(-9223372036854775808, -7)").to_display_string(), + "-9223372036850000000" + ); +} + +// round(float, ndigits) return type follows the spec (RFC 0006): int when +// ndigits <= 0, float when ndigits > 0. +#[test] +fn round_float_ndigits_return_type() { + assert!(matches!(eval("round(2.5, 0)"), ExprValue::Int(2))); + assert!(matches!(eval("round(1.5, -1)"), ExprValue::Int(_))); + assert!(matches!(eval("round(1.5, 2)"), ExprValue::Float(_))); +} + +// Positive ndigits preserves trailing zeros in the display (RFC 0006), even for +// a zero value: round(0.0, 7) -> "0.0000000". +#[test] +fn round_float_positive_ndigits_trailing_zeros() { + assert_eq!(eval("round(3.5, 2)").to_display_string(), "3.50"); + assert_eq!(eval("round(0.0, 7)").to_display_string(), "0.0000000"); +} + // === TestFloorCeilReturnType === #[test] @@ -683,11 +785,11 @@ fn round_ndigits_neg_0_75() { } #[test] fn round_ndigits_neg_2_5_0() { - assert_eq!(eval("round(-2.5, 0)").to_display_string(), "-2.0"); + assert_eq!(eval("round(-2.5, 0)").to_display_string(), "-2"); } #[test] fn round_ndigits_neg_3_5_0() { - assert_eq!(eval("round(-3.5, 0)").to_display_string(), "-4.0"); + assert_eq!(eval("round(-3.5, 0)").to_display_string(), "-4"); } // TestFailFunction - missing cases diff --git a/crates/openjd-expr/tests/integration/test_comparison.rs b/crates/openjd-expr/tests/integration/test_comparison.rs index 9e94b99f..e7da8ca9 100644 --- a/crates/openjd-expr/tests/integration/test_comparison.rs +++ b/crates/openjd-expr/tests/integration/test_comparison.rs @@ -419,3 +419,15 @@ fn float_gt_string_errors() { "got:\n{e}" ); } + +// Mixed path/string comparison compares by string value and must preserve +// operand order. A prior bug swapped the operands when the path was on the +// left, so `path('/tmp/x') < 'ab'` returned the wrong boolean ('/' (0x2f) is +// less than 'a' (0x61), so the correct answer is true). +#[test] +fn path_lt_string_operand_order() { + assert_eq!(eval("path('/tmp/x') < 'ab'").to_display_string(), "true"); + assert_eq!(eval("'ab' < path('/tmp/x')").to_display_string(), "false"); + // Symmetric with the pure-string comparison. + assert_eq!(eval("'/tmp/x' < 'ab'").to_display_string(), "true"); +} diff --git a/crates/openjd-expr/tests/integration/test_paths.rs b/crates/openjd-expr/tests/integration/test_paths.rs index aee328a5..42f23dd8 100644 --- a/crates/openjd-expr/tests/integration/test_paths.rs +++ b/crates/openjd-expr/tests/integration/test_paths.rs @@ -1438,7 +1438,9 @@ fn join_posix_trailing_backslash_not_stripped() { #[test] fn join_posix_empty_right() { - assert_eq!(path_join("/a/b", "", PathFormat::Posix), "/a/b/"); + // Joining an empty component is a no-op, matching Python `PurePath / ""` + // (which returns the path unchanged — no trailing separator). + assert_eq!(path_join("/a/b", "", PathFormat::Posix), "/a/b"); } #[test] @@ -1448,6 +1450,15 @@ fn join_posix_root() { // --- Windows format --- +// A root-relative right (leading separator) with a left that has NO drive and +// NO UNC root replaces the left entirely, matching ntpath.join('a/b', '/x') == +// '/x'. The previous code kept the left, producing e.g. "a\\b\\x". +#[test] +fn join_windows_root_relative_no_drive_replaces() { + assert_eq!(path_join("a\\b", "/x", PathFormat::Windows), "\\x"); + assert_eq!(path_join("relative", "\\foo", PathFormat::Windows), "\\foo"); +} + #[test] fn join_windows_basic() { assert_eq!( diff --git a/crates/openjd-expr/tests/integration/test_strings.rs b/crates/openjd-expr/tests/integration/test_strings.rs index 14d0cd37..1be70354 100644 --- a/crates/openjd-expr/tests/integration/test_strings.rs +++ b/crates/openjd-expr/tests/integration/test_strings.rs @@ -2554,3 +2554,34 @@ fn repr_sh_list_with_null_byte_returns_error() { "repr_sh on list with null-byte string should error" ); } + +// Negative or zero width means "no padding": the string is returned unchanged. +// A prior bug cast the negative width straight to usize (wrapping to a huge +// value), tripping the operation limit or attempting a giant allocation. +#[test] +fn center_negative_width_no_padding() { + assert_eq!(eval("center('a', -1)").to_display_string(), "a"); + assert_eq!(eval("center('ab', 0)").to_display_string(), "ab"); +} +#[test] +fn ljust_negative_width_no_padding() { + assert_eq!(eval("ljust('a', -1)").to_display_string(), "a"); +} +#[test] +fn rjust_negative_width_no_padding() { + assert_eq!(eval("rjust('a', -1)").to_display_string(), "a"); +} + +// center() odd-padding bias matches CPython exactly, where the side that gets +// the extra space depends on `marg & width & 1` (NOT a fixed side). The prior +// implementation used a fixed `pad/2` split and disagreed with Python on cases +// like center('ab', 11). Verified against CPython str.center. +#[test] +fn center_odd_padding_bias() { + // width 11, pad 9: 5 leading, 4 trailing. + assert_eq!(eval("center('ab', 11)").to_display_string(), " ab "); + // width 5, pad 4 (even): 2 and 2. + assert_eq!(eval("center('a', 5)").to_display_string(), " a "); + // width 4, pad 3 (odd): 1 leading, 2 trailing. + assert_eq!(eval("center('a', 4)").to_display_string(), " a "); +} diff --git a/crates/openjd-expr/tests/integration/test_types_evaluate.rs b/crates/openjd-expr/tests/integration/test_types_evaluate.rs index bc738905..ae7fda58 100644 --- a/crates/openjd-expr/tests/integration/test_types_evaluate.rs +++ b/crates/openjd-expr/tests/integration/test_types_evaluate.rs @@ -180,6 +180,25 @@ fn float_from_float() { assert_eq!(eval("float(3.14)").to_display_string(), "3.14"); } +// float(x) is an identity that PRESERVES the original literal (a copy, not a +// computation — RFC 0005 float pass-through). Reconstructing the value would +// reformat `1e308` to the computed form `1e+308`. +#[test] +fn float_from_float_preserves_literal() { + assert_eq!(eval("float(1e308)").to_display_string(), "1e308"); + assert_eq!(eval("float(9.2e18)").to_display_string(), "9.2e18"); +} + +// int(float) is only valid for whole values (RFC 0006: no destructive +// conversion). A non-whole float — however tiny — is an error, not truncation. +#[test] +fn int_from_non_whole_float_is_error() { + assert!(eval_fails("int(0.5)")); + assert!(eval_fails("int(3.75)")); + // A whole-valued float still converts. + assert_eq!(eval("int(3.0)").to_display_string(), "3"); +} + #[test] fn bool_from_bool() { assert_eq!(eval("bool(True)").to_display_string(), "true"); diff --git a/reports/expr-quality-evaluation-report.md b/reports/expr-quality-evaluation-report.md index d08e5928..0a55d1fb 100644 --- a/reports/expr-quality-evaluation-report.md +++ b/reports/expr-quality-evaluation-report.md @@ -413,13 +413,18 @@ documents. None affect spec compliance. ### Behavioral differences -None observed. The probes (§7) covering chained comparisons, bool/null -falsiness, list equality with range_expr, cross-type -String↔Path equality, and Unicode subscript all match Python -semantics where the spec defines them. The Rust crate's `RangeExpr` -canonicalizes descending ranges to ascending form at construction -time, which `range-expr.md` calls out as a deliberate simplification -over Python. +~~None observed.~~ **Superseded 2026-07-24** — the hand-written probes +missed a class of *silent-value* divergences that a systematic differential +harness (running `openjd-expr` against the Python reference on generated +inputs) later surfaced. See **§7 Bugs found** for the list; all are fixed. The +original probes remain accurate for the categories they covered (chained +comparisons, bool/null falsiness, list equality with range_expr, Unicode +subscript); the gap was breadth, not correctness of those specific probes. Note +in particular that the earlier "cross-type String↔Path equality matches Python" +observation did **not** extend to String↔Path *ordering* comparison, which was +one of the bugs found. The Rust crate's `RangeExpr` canonicalizes descending +ranges to ascending form at construction time, which `range-expr.md` calls out +as a deliberate simplification over Python. ### Test parity @@ -531,11 +536,41 @@ correctly (a probe author mistake, not a crate defect). ### Bugs found -**None.** Every probe behaved as the spec or `evaluator.md` predicted. -The probes were written specifically to look for edge-case crashes, -panics, silent overflow, hashing-equality consistency violations, -Unicode issues, and surprises in resource-bounding paths. None of -those happened. +~~**None.**~~ **Updated 2026-07-24.** The one-shot probes found none. A +follow-up *differential* pass — running `openjd-expr` and the Python reference +against the same inputs (a shared conformance corpus plus a grammar-aware +generator biased toward `i64` edges, negative counts, and multibyte strings) — +found **15** divergences, all in the silent-wrong-value / wrong-type class that +crash-only probing does not catch. **All are now fixed** (see the +`fix(expr): align … with the spec/Python reference` change), each with a +concrete regression test. + +| # | Area | Divergence (Rust → correct) | Root cause | +|---|------|-----------------------------|------------| +| 1 | `round(float, n)` return type | float → **int** for `n ≤ 0` | didn't follow RFC 0006 return-type rule | +| 2 | `round(int, -k)` | precision loss on large ints | f64 round-trip instead of exact integer arithmetic | +| 3 | `round(float, n>0)` | dropped trailing zeros (`round(0.0,7)`) | zero-string discarded in `Float64::with_str` | +| 4 | float `%` | `9.2e18 % 0.1` → `0.0` | `l - r*floor(l/r)` cancellation; now fmod-based | +| 5 | float `//` | `205 // 0.1` → `2050` (should be `2049`) | `(l/r).floor()` instead of CPython `float_divmod` | +| 6 | float `//` overflow | saturated instead of erroring | `> i64::MAX as f64` let exactly 2^63 through | +| 7 | `floor`/`ceil`/`round` → i64 | same 2^63 boundary saturation | as above | +| 8 | `int ** -n` | last-ulp error | `powi` (repeated squaring) instead of `powf` | +| 9 | `float(x)` | reformatted literal (`1e308`→`1e+308`) | identity dropped the preserved literal | +| 10 | `format_float` | `e18`/`e-7` vs `e+18`/`e-07` | non-C-style exponent formatting | +| 11 | `center`/`ljust`/`rjust` | negative width wrapped `usize` | `-1 as usize`; now clamps to 0 | +| 12 | `center` odd padding | wrong bias | didn't match CPython `marg & width & 1` | +| 13 | `path / ''` | trailing separator | empty component not treated as no-op | +| 14 | Windows `path` join | kept left on root-relative right | didn't match `ntpath.join` | +| 15 | String↔Path `<` | wrong boolean | operands swapped in the comparison arm | + +Most are spec-confirmed against RFC 0005/0006; a few (float `//`/`%` +tie-breaking, exponent spelling, negative-width padding) are spec-silent edge +cases where the Python reference is the de-facto definition — reasonable +readings, flagged in code comments and worth raising as spec clarifications. + +The differential harness itself is a developer tool with an out-of-tree Python +dependency, kept on a branch rather than in the crate; the durable output is the +fixes above plus their unit tests. ### Probe file