From 56cbd04bb09cc986933f9d6671d077219b75b28f Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 14 Sep 2026 10:05:08 +1000 Subject: [PATCH 1/5] fix(jq): round large int literals to 17 decimal digits before f64 arithmetic (#2906) `succinctly jq -n '869389897822472004 + 944331'` printed 869389897823416400 where jq 1.7.1 prints 869389897823416300. #2631 routed past-2^53 results through the f64 model but widened each operand with a bare `i64 as f64`; jq instead converts a *literal* through `jvp_literal_number_to_double` (src/jv.c): `decNumberReduce` under a DECIMAL64 context with `digits = 17` (half-even), then a correctly-rounded strtod -- a double rounding that lands on a different double for some 18/19-digit values. Modelled that way, jq's answer was reproduced on 5600/5600 random differential cases; the earlier "~4% unexplained" figure came from modelling the operands as exact integers. Add `jq_literal_int_to_f64` (identity below 10^17, integer-only rounding above) and an `EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS` gate, and use it wherever jq mode widens an `Int`: `jq_checked_int_arith`'s fallback, the mixed arms of `+ - * /`, `%` (now jq's `binop_mod` truncate-the-double model, so `869389897822472004 % 1000` is 936 and `9007199254740993 % 2` is 0), `numeric_repr_eq`, and ordering via `compare_values`, which becomes generic over `S` so `sort`/`unique`/`min`/`max`/`bsearch`/`delpaths` inherit the gate. yq mode keeps its exact int64 arithmetic and plain cast. Out of scope, recorded in docs/compliance/jq/limitations.md: >17-digit float and over-i64 literals, the math builtins' own widening, floor-family display past 2^53, and the reindex bridge re-parsing a computed double as an integer literal before `sort`/`unique`/`min`/`max`. --- docs/compliance/jq/limitations.md | 90 ++++++---- src/jq/eval.rs | 231 +++++++++++++++--------- src/jq/eval_generic.rs | 23 ++- src/jq/value.rs | 285 ++++++++++++++++++++++++++++-- tests/jq_cli_tests.rs | 166 +++++++++++++++++ tests/yq_cli_tests.rs | 24 +++ 6 files changed, 683 insertions(+), 136 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 15b01b707..d66064fa6 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -5775,45 +5775,73 @@ itself a rule-4 condition — the condition being invoked here is 4(c), inherite `range` entry above, not re-derived from consistency alone.) Pinned by `test_unary_minus_destroys_literal_preservation_2357` (`tests/jq_cli_tests.rs`). -### Large-integer `+`/`-` past `2^53` can still round to a different `f64` than real jq even after #2631's fix — no carve-out; recorded as a still-open gap (#2906) +### Large-integer arithmetic past `2^53` — closed for `i64` literals: jq rounds a literal to 17 decimal digits *before* the double conversion (#2906) #2631 fixed a fast-path bug where an exact, non-overflowing `i64` `+`/`-`/`*` result past `2^53` was kept as `OwnedValue::Int` and printed via its own exact digits, bypassing -`jq_bare_float_display`'s shortest-round-trip formatting entirely. The fix (`eval.rs`'s -`jq_checked_int_arith`) now falls back to jq's own `f64` model — `a as f64 op b as f64` — -whenever either operand or the exact result exceeds `2^53`, which is a strict improvement -(that fallback matches jq in the large majority of cases where it wasn't reachable at all -before). It is not a complete fix, though: differential fuzzing against `/usr/bin/jq` 1.7.1 -found real jq's own `+`/`-` sometimes rounds to a *different* double than plain -`a as f64 op b as f64`, even with **both operands non-negative** (so unrelated to the -separate unary-minus divergence recorded above): +`jq_bare_float_display`'s shortest-round-trip formatting entirely. Its fallback — `a as f64 +op b as f64` — still disagreed with real jq on a residual few percent of random 18/19-digit +operands, even with both operands non-negative: ```console $ jq -n '869389897822472004 + 944331' # 869389897823416300 -$ sjq -n '869389897822472004 + 944331' # 869389897823416400 +$ sjq -n '869389897822472004 + 944331' # 869389897823416400 (before #2906) ``` -Both tools agree the *exact* integer sum is `869389897823416335`; the divergence is in -which double each implementation's addition produces. A 400-case-per-operator random -sample (both operands drawn from a wide magnitude range, `f64(a) op f64(b)` vs. the exact -integer sum cast to `f64` once, each compared against jq's live answer) found neither -model predicts jq in ~4% of cases for `+`/`-` and ~4% for `*`, while the naive -per-operand-cast model (what `jq_checked_int_arith` falls back to) is right in the -remaining ~96% — see #2906 for the full breakdown. This is almost certainly jq's own -decNumber-backed literal preservation interacting with plain `f64` arithmetic in some -mixed, not-fully-naive way that hasn't been traced to jq's C source, rather than a -tie-break or formatting question `jq_bare_float_display` could resolve on its own — #2542's -tie-break mechanism only ever acts on an already-odd trailing digit, and every case found -here already has an even last digit on both sides. - -Recorded here rather than fixed because closing it fully would mean replicating jq's actual -arbitrary-precision decimal arithmetic model for `+`/`-` (and, per the ~4% mul figure above, -possibly `*` too) — a real decNumber-equivalent dependency, not a formatting-path change — -and no model tried so far explains 100% of cases even as a starting point. This does not fit -ADR-0018 rule 4's four named conditions (the output is readable, nothing is corrupted, the -process doesn't die), so per rule 4 it is recorded as a still-open gap rather than a settled -divergence, matching this file's own "`foreach`/`reduce`'s INIT-fork re-entry" entry above. -`jq_checked_int_arith`'s own doc comment in `src/jq/eval.rs` links back here. +An earlier revision of this entry recorded that residue as an open gap on the theory that +jq must be doing arbitrary-precision decimal arithmetic. It is not; the earlier analysis +modelled the *operands* as exact integers, and that was the mistake. jq 1.7.1 keeps every +parsed number — program text, document input, `tonumber`, `fromjson`, `--argjson` — as an +exact `decNumber` literal and converts it to a double only when arithmetic first reads it, +in `jvp_literal_number_to_double` (`src/jv.c`): `decNumberReduce` under a +`DEC_INIT_DECIMAL64` context whose `digits` is raised to 17 (`DEC_NUBMER_DOUBLE_PRECISION`, +round-half-even), then `decNumberToString`, then a correctly-rounded `strtod`. That is a +*double* rounding — the literal's decimal value is first rounded to 17 significant decimal +digits, and only that shorter number is rounded to the nearest double — and it lands on a +different double than the exact integer's nearest one whenever the 17-digit intermediate +sits across a rounding boundary. `869389897822472004` (18 digits) becomes +`869389897822472000`, whose nearest double is `869389897822471936` (a tie, resolved to +even); the exact integer's nearest double is `869389897822472064`. Every binary operator +then runs plain `double` arithmetic on those values, `%` truncates each of them to +`intmax_t` (`dtoi`, saturating) before taking the remainder, and a *computed* number is a +plain double that is never re-rounded. Modelled that way, jq's answer was reproduced on +100% of 1200 random `+`/`-`/`*`/`/` cases, 200 input-sourced cases including negatives and +over-`i64` values, and 100 program-literal negatives (the "no model explains it" bucket +was 0). + +For an `i64` the two conversions differ only when the magnitude is at least `10^17` (18 or +19 digits): below that a decimal integer has at most 17 significant digits, so the +intermediate rounding is the identity and `n as f64` was already right — everything in +`[2^53, 10^17)` behaved correctly before this fix. `jq_literal_int_to_f64` +(`src/jq/value.rs`) implements the rounding with integer arithmetic only, and jq mode uses +it wherever an `Int` is widened for `+`/`-`/`*`/`/`/`%`, for `==`, and for ordering +(`<`, `sort`, `unique`, `group_by`, `min`/`max`, `bsearch`), in both evaluators; +`%` additionally follows `binop_mod`'s truncate-the-double model, so +`869389897822472004 % 1000` is `936` (jq) rather than the exact `4`, and +`9007199254740993 % 2` is `0`. yq mode is untouched: real yq's `int64` arithmetic is +exact (`869389897822472004 + 944331` is `869389897823416335` there, as it always was +here), so its widening stays a plain cast (`EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS`). + +**Still open, same mechanism, out of #2906's scope** (tracked as follow-up issues): +a *float* literal with more than 17 significant digits, or an integer literal beyond +`i64`, is still parsed with one correct rounding (`2.7293109604053567083 + 0` is +`2.7293109604053565` in jq, `2.729310960405357` here) — closing it needs the mode plumbed +through the number-materialisation funnels, not just the arithmetic; the math builtins +(`floor`, `sqrt`, `pow`, …) still widen a large `Int` with a bare cast +(`869389897822472004 | sqrt` is `932410798.8555645` in jq, `…647` here); and +`floor`/`ceil`/`round`/`trunc` of such a value print their exact integer digits where jq +prints the double (`869389897822472000`). Two *literals* compared against each other are +still widened here where jq compares them exactly as decimals (`9007199254740993 == +9007199254740992.0`, `numeric_repr_eq`'s own doc comment). And a computed double that +crosses the reindex bridge into a document-input builtin (`sort`, `unique`, `min`, `max`, +`group_by` on an array built in the filter) is re-parsed from its printed digits as an +*integer* literal, so it then compares exactly against a real literal instead of equal: +`[869389897822472004, (869389897822472000+0), 5] | sort` is +`[5,869389897822472000,869389897822472004]` here and +`[5,869389897822472004,869389897822472000]` in jq (stable, the two are equal there) — +identical before and after this fix, since the comparator is right and the bridge is what +changes the operand. The `range` entry above and the unary-minus entry (#2357) are +unaffected — both stay on the exact `i64` path they document. ### `--argjson`/`--jsonargs` still reject a bare trailing decimal point with no exponent (`1.`) — accepted divergence, ADR-0018 rule 4c (#2240) diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 1981b12a9..575a0a2b2 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -71,6 +71,12 @@ pub enum EvalTag { pub trait EvalSemantics: Copy + Default { /// If true, integer overflow wraps (yq). If false, converts to float (jq). const OVERFLOW_WRAPS: bool; + /// If true (jq), an `Int` widened to `f64` for arithmetic or ordering is + /// treated as a decNumber *literal* and rounded to 17 significant + /// decimal digits before the double conversion, matching jq 1.7.1's + /// `jvp_literal_number_to_double` (#2906, `jq_literal_int_to_f64`). If + /// false (yq), the widening is a plain cast. + const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool; /// If true, division by zero returns infinity (yq). If false, returns error (jq). const DIV_BY_ZERO_IS_INFINITY: bool; /// If true (yq), `has()`/`in()`'s array-index arm accepts *any* negative @@ -307,6 +313,7 @@ pub struct JqSemantics; impl EvalSemantics for JqSemantics { const OVERFLOW_WRAPS: bool = false; + const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool = true; const DIV_BY_ZERO_IS_INFINITY: bool = false; const NEGATIVE_INDEX_IN_HAS: bool = false; const MOD_TRUNCATES_FLOATS: bool = true; @@ -340,6 +347,7 @@ pub struct YqSemantics; impl EvalSemantics for YqSemantics { const OVERFLOW_WRAPS: bool = true; + const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool = false; const DIV_BY_ZERO_IS_INFINITY: bool = true; const NEGATIVE_INDEX_IN_HAS: bool = true; const MOD_TRUNCATES_FLOATS: bool = false; @@ -369,8 +377,9 @@ use super::expr::{ Pattern, PatternEntry, SliceBoundKey, StringPart, Tracked, }; use super::value::{ - assert_value_tree_depth, cmp_f64, infinite_float_preview_text, is_infinity_sentinel, - is_nan_sentinel, numeric_repr_cmp, owned_value_eq, NumberRepr, OwnedValue, + assert_value_tree_depth, infinite_float_preview_text, int_to_f64, is_infinity_sentinel, + is_nan_sentinel, jq_literal_int_to_f64, numeric_repr_cmp, owned_value_eq, NumberRepr, + OwnedValue, }; /// Which binary operator an operand that produced *zero outputs* is being @@ -8796,6 +8805,23 @@ fn jq_int_within_exact_f64_range(value: i64) -> bool { value.unsigned_abs() <= (1u64 << 53) } +/// Wrap a jq-mode integer *result* the way real jq holds it -- as a double. +/// Within `f64`'s exact-integer range an `Int` is that same double and +/// prints identically, so it stays exact; past it the value is stored as +/// the `Float` jq would compute (`r as f64`, one correct rounding, matching +/// C's `(double)intmax`), so it prints through +/// [`jq_bare_float_display`]'s shortest-round-trip formatting rather than +/// its own exact digits (#2631, #2906), and never becomes an exact `Int` +/// past `2^53` that [`jq_literal_int_to_f64`] would later mistake for a +/// literal. +fn jq_f64_backed_int(result: i64) -> OwnedValue { + if jq_int_within_exact_f64_range(result) { + OwnedValue::Int(result) + } else { + OwnedValue::Float(result as f64) + } +} + /// Compute a jq-mode `i64`/`i64` `+`/`-`/`*` the way real jq's own, /// always-`f64` arithmetic would (#2631): only keep the exact `i64` result /// when **both operands and the result** are within `f64`'s exact-integer @@ -8817,15 +8843,14 @@ fn jq_int_within_exact_f64_range(value: i64) -> bool { /// range -- no separate proof is needed once every value in play is /// individually exact. /// -/// This deliberately does *not* attempt to correct for jq's separate, -/// pre-existing decNumber-literal-preservation quirks once `f64_op` is -/// reached (documented in -/// [`docs/compliance/jq/limitations.md`](../../docs/compliance/jq/limitations.md) -/// and tracked as #2906) -- matching those exactly would mean replicating -/// jq's full arbitrary-precision decimal arithmetic model, not just -/// deciding when to trust an exact `i64` shortcut over it. Out of scope -/// for #2631, which is specifically about the exact-integer-result -/// bypassing the float formatter entirely, described above. +/// The operands handed to `f64_op` are widened with +/// [`jq_literal_int_to_f64`], not a bare `as f64` (#2906): both are +/// literals in jq's number model, and jq rounds a literal to 17 +/// significant decimal digits *before* converting it to a double, which +/// lands on a different double than the exact integer's nearest one for +/// some 18- and 19-digit values (`869389897822472004 + 944331` is +/// `869389897823416300` in jq, `869389897823416400` with a bare cast). +/// Below `2^53` the two agree, so the exact fast path above is unaffected. /// /// yq mode never calls this: `S::OVERFLOW_WRAPS` gates every call site to /// this function's own jq-only branch, since yq's wrapping arithmetic has @@ -8844,7 +8869,7 @@ fn jq_checked_int_arith( { OwnedValue::Int(result) } - _ => OwnedValue::Float(f64_op(a as f64, b as f64)), + _ => OwnedValue::Float(f64_op(jq_literal_int_to_f64(a), jq_literal_int_to_f64(b))), } } @@ -8929,8 +8954,12 @@ fn arith_add( Ok(jq_checked_int_arith(a, b, i64::checked_add, |x, y| x + y)) } } - (OwnedValue::Int(a), OwnedValue::Float(b)) => Ok(OwnedValue::Float(a as f64 + b)), - (OwnedValue::Float(a), OwnedValue::Int(b)) => Ok(OwnedValue::Float(a + b as f64)), + (OwnedValue::Int(a), OwnedValue::Float(b)) => { + Ok(OwnedValue::Float(int_to_f64::(a) + b)) + } + (OwnedValue::Float(a), OwnedValue::Int(b)) => { + Ok(OwnedValue::Float(a + int_to_f64::(b))) + } (OwnedValue::Float(a), OwnedValue::Float(b)) => Ok(OwnedValue::Float(a + b)), _ => unreachable!("both operands already confirmed numeric by the guard above"), } @@ -9021,8 +9050,12 @@ fn arith_sub( Ok(jq_checked_int_arith(a, b, i64::checked_sub, |x, y| x - y)) } } - (OwnedValue::Int(a), OwnedValue::Float(b)) => Ok(OwnedValue::Float(a as f64 - b)), - (OwnedValue::Float(a), OwnedValue::Int(b)) => Ok(OwnedValue::Float(a - b as f64)), + (OwnedValue::Int(a), OwnedValue::Float(b)) => { + Ok(OwnedValue::Float(int_to_f64::(a) - b)) + } + (OwnedValue::Float(a), OwnedValue::Int(b)) => { + Ok(OwnedValue::Float(a - int_to_f64::(b))) + } (OwnedValue::Float(a), OwnedValue::Float(b)) => Ok(OwnedValue::Float(a - b)), _ => unreachable!("both operands already confirmed numeric by the guard above"), } @@ -9176,10 +9209,10 @@ fn arith_mul( } } (_, _, Some(NumberRepr::Int(a)), Some(NumberRepr::Float(b))) => { - Ok(OwnedValue::Float(a as f64 * b)) + Ok(OwnedValue::Float(int_to_f64::(a) * b)) } (_, _, Some(NumberRepr::Float(a)), Some(NumberRepr::Int(b))) => { - Ok(OwnedValue::Float(a * b as f64)) + Ok(OwnedValue::Float(a * int_to_f64::(b))) } (_, _, Some(NumberRepr::Float(a)), Some(NumberRepr::Float(b))) => { Ok(OwnedValue::Float(a * b)) @@ -9486,27 +9519,27 @@ fn arith_div( if b == 0 { if S::DIV_BY_ZERO_IS_INFINITY { // yq behavior: return infinity - Ok(OwnedValue::Float(a as f64 / b as f64)) + Ok(OwnedValue::Float(int_to_f64::(a) / int_to_f64::(b))) } else { // jq behavior: error Err(EvalError::divisor_is_zero(&left, &right, BinOp::Divide)) } } else { - Ok(OwnedValue::Float(a as f64 / b as f64)) + Ok(OwnedValue::Float(int_to_f64::(a) / int_to_f64::(b))) } } (Some(NumberRepr::Int(a)), Some(NumberRepr::Float(b))) => { if b == 0.0 && !S::DIV_BY_ZERO_IS_INFINITY { Err(EvalError::divisor_is_zero(&left, &right, BinOp::Divide)) } else { - Ok(OwnedValue::Float(a as f64 / b)) + Ok(OwnedValue::Float(int_to_f64::(a) / b)) } } (Some(NumberRepr::Float(a)), Some(NumberRepr::Int(b))) => { if b == 0 && !S::DIV_BY_ZERO_IS_INFINITY { Err(EvalError::divisor_is_zero(&left, &right, BinOp::Divide)) } else { - Ok(OwnedValue::Float(a / b as f64)) + Ok(OwnedValue::Float(a / int_to_f64::(b))) } } (Some(NumberRepr::Float(a)), Some(NumberRepr::Float(b))) => { @@ -9565,18 +9598,39 @@ fn arith_mod( } else { Err(EvalError::divisor_is_zero(&left, &right, BinOp::Modulo)) } - } else { + } else if S::OVERFLOW_WRAPS { + // yq behavior: exact int64 remainder. Ok(OwnedValue::Int(a.wrapping_rem(b))) + } else { + // jq behavior (#2906): `binop_mod` truncates each operand's + // *double* -- for a literal, the 17-digit-rounded one -- to + // `intmax_t` with a saturating cast (`dtoi`), takes the + // remainder, and holds the result as a double again. `as + // i64` is the same saturating truncation. The divisor + // cannot truncate to zero here (a nonzero `i64` never rounds + // to zero), but `% -1` must short-circuit to `0` exactly as + // jq does rather than trip `i64::MIN % -1`. + let (ai, bi) = ( + jq_literal_int_to_f64(a) as i64, + jq_literal_int_to_f64(b) as i64, + ); + if bi == 0 { + Err(EvalError::divisor_is_zero(&left, &right, BinOp::Modulo)) + } else if bi == -1 { + Ok(OwnedValue::Int(0)) + } else { + Ok(jq_f64_backed_int(ai.wrapping_rem(bi))) + } } } (Some(NumberRepr::Float(a)), Some(NumberRepr::Float(b))) => { mod_floats::(a, b, &left, &right) } (Some(NumberRepr::Int(a)), Some(NumberRepr::Float(b))) => { - mod_floats::(a as f64, b, &left, &right) + mod_floats::(int_to_f64::(a), b, &left, &right) } (Some(NumberRepr::Float(a)), Some(NumberRepr::Int(b))) => { - mod_floats::(a, b as f64, &left, &right) + mod_floats::(a, int_to_f64::(b), &left, &right) } _ => Err(EvalError::binary_op(&left, &right, BinOp::Modulo)), } @@ -9602,8 +9656,9 @@ fn mod_floats( if bi == 0 { Err(EvalError::divisor_is_zero(left, right, BinOp::Modulo)) } else { - // wrapping_rem: i64::MIN % -1 must not panic. - Ok(OwnedValue::Int(ai.wrapping_rem(bi))) + // wrapping_rem: i64::MIN % -1 must not panic. jq holds the + // remainder as a double, so past 2^53 it is a `Float` (#2906). + Ok(jq_f64_backed_int(ai.wrapping_rem(bi))) } } else if b == 0.0 && !S::DIV_BY_ZERO_IS_INFINITY { Err(EvalError::divisor_is_zero(left, right, BinOp::Modulo)) @@ -9659,10 +9714,10 @@ pub(crate) fn apply_compare_op( match op { CompareOp::Eq => owned_value_eq::(left, right), CompareOp::Ne => !owned_value_eq::(left, right), - CompareOp::Lt => compare_values(left, right) == core::cmp::Ordering::Less, - CompareOp::Le => compare_values(left, right) != core::cmp::Ordering::Greater, - CompareOp::Gt => compare_values(left, right) == core::cmp::Ordering::Greater, - CompareOp::Ge => compare_values(left, right) != core::cmp::Ordering::Less, + CompareOp::Lt => compare_values::(left, right) == core::cmp::Ordering::Less, + CompareOp::Le => compare_values::(left, right) != core::cmp::Ordering::Greater, + CompareOp::Gt => compare_values::(left, right) == core::cmp::Ordering::Greater, + CompareOp::Ge => compare_values::(left, right) != core::cmp::Ordering::Less, } } @@ -9691,8 +9746,11 @@ pub(crate) fn apply_compare_op( /// left-to-right) and `unique`'s/`unique_by`'s `dedup_by` only compares /// adjacent elements, so neither has any panic surface at all. See /// `test_sort_many_nans_does_not_panic_421`. -pub(crate) fn compare_values(left: &OwnedValue, right: &OwnedValue) -> core::cmp::Ordering { - compare_values_at_depth(left, right, 0) +pub(crate) fn compare_values( + left: &OwnedValue, + right: &OwnedValue, +) -> core::cmp::Ordering { + compare_values_at_depth::(left, right, 0) } /// Panics past [`MAX_VALUE_TREE_DEPTH`](super::value::MAX_VALUE_TREE_DEPTH) @@ -9702,7 +9760,7 @@ pub(crate) fn compare_values(left: &OwnedValue, right: &OwnedValue) -> core::cmp /// `repeat` build up at query-evaluation time bypasses #998's /// document-input guards entirely: no adversarial document is involved, /// only enough loop iterations to grow the accumulator past the limit. -fn compare_values_at_depth( +fn compare_values_at_depth( left: &OwnedValue, right: &OwnedValue, depth: usize, @@ -9721,25 +9779,24 @@ fn compare_values_at_depth( match (left, right) { (OwnedValue::Null, OwnedValue::Null) => Ordering::Equal, (OwnedValue::Bool(a), OwnedValue::Bool(b)) => a.cmp(b), - (OwnedValue::Int(a), OwnedValue::Int(b)) => a.cmp(b), - (OwnedValue::Float(a), OwnedValue::Float(b)) => cmp_f64(*a, *b), - (OwnedValue::Int(a), OwnedValue::Float(b)) => cmp_f64(*a as f64, *b), - (OwnedValue::Float(a), OwnedValue::Int(b)) => cmp_f64(*a, *b as f64), - // A `NumberLiteral` operand compares by its parsed value, exactly - // like `Int`/`Float` -- ordering never looks at the source text. - // `numeric_repr_cmp` dispatches on the same `(Int,Int)`/`(Float,Float)`/ - // mixed pairing `==` uses (`numeric_repr_eq`), so ordering can't - // disagree with equality about the same pair (see its doc comment). - (OwnedValue::NumberLiteral(..), _) | (_, OwnedValue::NumberLiteral(..)) => { - match (left.number_repr(), right.number_repr()) { - (Some(a), Some(b)) => numeric_repr_cmp(a, b), - _ => Ordering::Equal, - } - } + // Every numeric pairing -- `Int`, `Float`, or `NumberLiteral` in any + // combination -- compares by parsed value through one dispatch; + // ordering never looks at a literal's source text. + // `numeric_repr_cmp` uses the same `(Int,Int)`/`(Float,Float)`/mixed + // pairing `==` uses (`numeric_repr_eq`), so ordering can't disagree + // with equality about the same pair (see its doc comment), and its + // mixed arms widen an `Int` under `S`'s number model (#2906). + ( + OwnedValue::Int(_) | OwnedValue::Float(_) | OwnedValue::NumberLiteral(..), + OwnedValue::Int(_) | OwnedValue::Float(_) | OwnedValue::NumberLiteral(..), + ) => match (left.number_repr(), right.number_repr()) { + (Some(a), Some(b)) => numeric_repr_cmp::(a, b), + _ => Ordering::Equal, + }, (OwnedValue::String(a), OwnedValue::String(b)) => a.cmp(b), (OwnedValue::Array(a), OwnedValue::Array(b)) => { for (av, bv) in a.iter().zip(b.iter()) { - match compare_values_at_depth(av, bv, depth + 1) { + match compare_values_at_depth::(av, bv, depth + 1) { Ordering::Equal => continue, other => return other, } @@ -9758,7 +9815,7 @@ fn compare_values_at_depth( other => return other, } for k in a_keys { - match compare_values_at_depth(&a[k], &b[k], depth + 1) { + match compare_values_at_depth::(&a[k], &b[k], depth + 1) { Ordering::Equal => continue, other => return other, } @@ -10235,8 +10292,8 @@ fn eval_builtin<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Builtin::All => builtin_all::(value, optional), Builtin::AllF(cond) => builtin_all_f::(cond, value, optional), Builtin::AllCond(gen, cond) => builtin_all_cond::(gen, cond, value, optional), - Builtin::Min => builtin_min::(value, optional), - Builtin::Max => builtin_max::(value, optional), + Builtin::Min => builtin_min::(value, optional), + Builtin::Max => builtin_max::(value, optional), Builtin::MinBy(f) => builtin_min_by::(f, value, optional), Builtin::MaxBy(f) => builtin_max_by::(f, value, optional), @@ -10293,7 +10350,7 @@ fn eval_builtin<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( Builtin::GroupBy(f) => builtin_group_by::(f, value, optional), Builtin::Unique => builtin_unique::(value, optional), Builtin::UniqueBy(f) => builtin_unique_by::(f, value, optional), - Builtin::Sort => builtin_sort::(value, optional), + Builtin::Sort => builtin_sort::(value, optional), Builtin::SortBy(f) => builtin_sort_by::(f, value, optional), // Phase 5: Object Functions @@ -11832,7 +11889,7 @@ fn builtin_all_cond<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } /// Builtin: min -fn builtin_min>( +fn builtin_min, S: EvalSemantics>( value: StandardJson<'_, W>, optional: bool, ) -> QueryResult<'_, W> { @@ -11849,7 +11906,7 @@ fn builtin_min>( return QueryResult::Owned(OwnedValue::Null); } - let min = items.into_iter().min_by(compare_values).unwrap(); + let min = items.into_iter().min_by(compare_values::).unwrap(); QueryResult::Owned(min) } // #1755: a decode failure on the scalar itself must raise @@ -11862,7 +11919,7 @@ fn builtin_min>( } /// Builtin: max -fn builtin_max>( +fn builtin_max, S: EvalSemantics>( value: StandardJson<'_, W>, optional: bool, ) -> QueryResult<'_, W> { @@ -11877,7 +11934,7 @@ fn builtin_max>( return QueryResult::Owned(OwnedValue::Null); } - let max = items.into_iter().max_by(compare_values).unwrap(); + let max = items.into_iter().max_by(compare_values::).unwrap(); QueryResult::Owned(max) } // #1755: same reasoning as builtin_min's own scalar arm above. @@ -12005,7 +12062,7 @@ fn builtin_min_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // `builtin_del`'s doc comment for the shared reasoning. let (_, v) = keyed .into_iter() - .min_by(|(a, _), (b, _)| compare_values(a, b)) + .min_by(|(a, _), (b, _)| compare_values::(a, b)) .unwrap(); let min = match to_owned(&v) { Ok(v) => v, @@ -12073,7 +12130,7 @@ fn builtin_max_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // `builtin_min_by`'s sibling comment above. let (_, v) = keyed .into_iter() - .max_by(|(a, _), (b, _)| compare_values(a, b)) + .max_by(|(a, _), (b, _)| compare_values::(a, b)) .unwrap(); let max = match to_owned(&v) { Ok(v) => v, @@ -13227,7 +13284,8 @@ fn flatten_owned_at_depth( for item in items { match item { OwnedValue::Array(inner) - if compare_values(&depth, &OwnedValue::Int(0)) != core::cmp::Ordering::Equal => + if compare_values::(&depth, &OwnedValue::Int(0)) + != core::cmp::Ordering::Equal => { let next_depth = arith_sub::(depth.clone(), OwnedValue::Int(1))?; result.extend(flatten_owned_at_depth::( @@ -13280,7 +13338,7 @@ fn builtin_flatten_depth<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // text with no yq oracle behind it. ref other if S::TAG == EvalTag::Jq - && compare_values(other, &OwnedValue::Int(0)) + && compare_values::(other, &OwnedValue::Int(0)) == core::cmp::Ordering::Less => { return QueryResult::Error(EvalError::new( @@ -13384,7 +13442,7 @@ fn builtin_group_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } // Sort by key - keyed.sort_by(|(a, _), (b, _)| compare_values(a, b)); + keyed.sort_by(|(a, _), (b, _)| compare_values::(a, b)); // Group consecutive items with same key let mut groups: Vec = Vec::new(); @@ -13466,7 +13524,7 @@ fn builtin_unique<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( let mut items = to_owned_vec_or_suppress!(elements, optional); // Sort first (jq's unique returns sorted unique values) - items.sort_by(compare_values); + items.sort_by(compare_values::); // Remove consecutive duplicates. `owned_value_eq::`, not // `compare_values(..) == Equal` (#950 review, same reasoning @@ -13558,7 +13616,7 @@ fn builtin_unique_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } // Sort by key - keyed.sort_by(|(a, _), (b, _)| compare_values(a, b)); + keyed.sort_by(|(a, _), (b, _)| compare_values::(a, b)); // Remove consecutive duplicates by key. `owned_value_eq::`, // not `compare_values(..) == Equal` (#950 review, same @@ -13592,7 +13650,7 @@ fn builtin_unique_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } /// Builtin: sort - sort array -fn builtin_sort>( +fn builtin_sort, S: EvalSemantics>( value: StandardJson<'_, W>, optional: bool, ) -> QueryResult<'_, W> { @@ -13603,7 +13661,7 @@ fn builtin_sort>( // #2327: to_owned_vec_or_suppress!, not a bare match -- see // `builtin_del`'s doc comment for the shared reasoning. let mut items = to_owned_vec_or_suppress!(elements, optional); - items.sort_by(compare_values); + items.sort_by(compare_values::); QueryResult::Owned(OwnedValue::Array(items)) } // #1755: a decode failure on the scalar itself must raise @@ -13663,7 +13721,7 @@ fn builtin_sort_by<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( } // Sort by key - keyed.sort_by(|(a, _), (b, _)| compare_values(a, b)); + keyed.sort_by(|(a, _), (b, _)| compare_values::(a, b)); let result: Vec = keyed.into_iter().map(|(_, v)| v).collect(); QueryResult::Owned(OwnedValue::Array(result)) @@ -42239,7 +42297,7 @@ fn builtin_truncate_stream<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( return QueryResult::Error(EvalError::new("Invalid path in streaming format")); }; let path_len = OwnedValue::Int(path.len() as i64); - if compare_values(&path_len, &depth) == core::cmp::Ordering::Greater { + if compare_values::(&path_len, &depth) == core::cmp::Ordering::Greater { // Reachable non-number depths here are only null/bool (jq order: // null < bool < number < ...), never string/array/object, since // a number never sorts above those. Both null and bool slice as @@ -43009,7 +43067,7 @@ fn builtin_setpath<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( /// simultaneous, so each index resolves against the length the array had /// *before* any sibling was removed. `delpaths([[-1],[-2]])` is `[10,20]`, not /// the `[10,30]` that deleting one at a time gives (#398). -fn delete_paths_sorted( +fn delete_paths_sorted( mut value: OwnedValue, paths: &[&[OwnedValue]], start: usize, @@ -43024,7 +43082,8 @@ fn delete_paths_sorted( while i < paths.len() { let key = &paths[i][start]; let mut j = i + 1; - while j < paths.len() && compare_values(&paths[j][start], key) == core::cmp::Ordering::Equal + while j < paths.len() + && compare_values::(&paths[j][start], key) == core::cmp::Ordering::Equal { j += 1; } @@ -43034,7 +43093,7 @@ fn delete_paths_sorted( if paths[i].len() == start + 1 { del_keys.push(key); } else { - value = delete_paths_under(value, key, &paths[i..j], start + 1, yq_mode)?; + value = delete_paths_under::(value, key, &paths[i..j], start + 1, yq_mode)?; } i = j; } @@ -43043,7 +43102,7 @@ fn delete_paths_sorted( /// Recurse into the child of `value` under `key`. A key that names nothing is /// a no-op, as it is in jq — `{"a":1} | delpaths([["b","c"]])` is `{"a":1}`. -fn delete_paths_under( +fn delete_paths_under( value: OwnedValue, key: &OwnedValue, paths: &[&[OwnedValue]], @@ -43058,7 +43117,7 @@ fn delete_paths_under( // jq leaves an existing key where it was, and `IndexMap` // would move it to the end after a `shift_remove`. let old = core::mem::replace(slot, OwnedValue::Null); - *slot = delete_paths_sorted(old, paths, start, yq_mode)?; + *slot = delete_paths_sorted::(old, paths, start, yq_mode)?; } Ok(OwnedValue::Object(entries)) } @@ -43083,7 +43142,8 @@ fn delete_paths_under( OwnedValue::Object(desc) => { let range = SliceBounds::from_descriptor(desc)?.resolve(arr.len()); let sub = OwnedValue::Array(arr[range.clone()].to_vec()); - let OwnedValue::Array(items) = delete_paths_sorted(sub, paths, start, yq_mode)? + let OwnedValue::Array(items) = + delete_paths_sorted::(sub, paths, start, yq_mode)? else { unreachable!("deleting from an array yields an array") }; @@ -43111,11 +43171,12 @@ fn delete_paths_under( match resolve_delete_index(key, arr.len()) { DeleteIndexResolution::InRange(index) => { let old = core::mem::replace(&mut arr[index], OwnedValue::Null); - arr[index] = delete_paths_sorted(old, paths, start, yq_mode)?; + arr[index] = delete_paths_sorted::(old, paths, start, yq_mode)?; } DeleteIndexResolution::PositiveOutOfRange(index) if yq_mode => { pad_with_nulls(&mut arr, index)?; - arr[index] = delete_paths_sorted(OwnedValue::Null, paths, start, yq_mode)?; + arr[index] = + delete_paths_sorted::(OwnedValue::Null, paths, start, yq_mode)?; } DeleteIndexResolution::PositiveOutOfRange(_) | DeleteIndexResolution::Skip => {} } @@ -49173,7 +49234,7 @@ fn delpaths_one<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( // time instead of handing the whole (sorted) batch to // `delete_paths_sorted` in one call. if S::TAG != EvalTag::Yq { - paths.sort_by(compare_values); + paths.sort_by(compare_values::); } // Every survivor of the `retain` is an array, so this only re-borrows. @@ -49216,7 +49277,7 @@ fn delpaths_one<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( root_deleted = true; OwnedValue::Null } else { - delete_paths_sorted(v, core::slice::from_ref(&path), 0, true)? + delete_paths_sorted::(v, core::slice::from_ref(&path), 0, true)? }; } Ok(v) @@ -49230,7 +49291,7 @@ fn delpaths_one<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( None => to_owned(value), Some([]) => Ok(OwnedValue::Null), // STYLE-0012: see the note above `root_deleted`. - Some(_) => to_owned(value).and_then(|v| delete_paths_sorted(v, &paths, 0, false)), + Some(_) => to_owned(value).and_then(|v| delete_paths_sorted::(v, &paths, 0, false)), } }; match result { @@ -50347,7 +50408,7 @@ fn builtin_bsearch<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( optional, // Real yq has no `bsearch` (lexer-rejected); see `builtin_ltrimstr`. ArgFanout::All, - |x| bsearch_one_target::(value.clone(), x, optional), + |x| bsearch_one_target::(value.clone(), x, optional), ) } @@ -50356,7 +50417,7 @@ fn builtin_bsearch<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( /// /// Takes `value` by value because the array arm consumes the element cursor by /// move; the fan-out loop clones the cursor per iteration. -fn bsearch_one_target>( +fn bsearch_one_target, S: EvalSemantics>( value: StandardJson<'_, W>, x: OwnedValue, optional: bool, @@ -50412,7 +50473,7 @@ fn bsearch_one_target>( // copy that used to live here lacked `(Array, Array)` and // `(Object, Object)` arms, so every pair of containers compared Equal // and `bsearch` reported absent values as found (#384). - match compare_values(&elements[mid as usize], &x) { + match compare_values::(&elements[mid as usize], &x) { core::cmp::Ordering::Equal => return QueryResult::Owned(OwnedValue::Int(mid)), core::cmp::Ordering::Less => lo = mid + 1, core::cmp::Ordering::Greater => hi = mid - 1, @@ -87556,14 +87617,14 @@ mod tests { let under_a = linear_array_nest(MAX_VALUE_TREE_DEPTH - 1); let under_b = linear_array_nest(MAX_VALUE_TREE_DEPTH - 1); assert_eq!( - compare_values(&under_a, &under_b), + compare_values::(&under_a, &under_b), core::cmp::Ordering::Equal ); let over_a = linear_array_nest(MAX_VALUE_TREE_DEPTH); let over_b = linear_array_nest(MAX_VALUE_TREE_DEPTH); let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - compare_values(&over_a, &over_b) + compare_values::(&over_a, &over_b) })); assert!( result.is_err(), diff --git a/src/jq/eval_generic.rs b/src/jq/eval_generic.rs index d2877b318..0b97e4e88 100644 --- a/src/jq/eval_generic.rs +++ b/src/jq/eval_generic.rs @@ -1728,7 +1728,10 @@ fn materialize_lazy_keys( ) -> Result { let mut keys = effective_key_values(fields, collapse)?; if sorted { - keys.sort_by(compare_values); + // Object keys are always strings, so the mode-forked numeric arms + // of `compare_values` (#2906) are unreachable here and no `S` is + // threaded through this streaming entry point. + keys.sort_by(compare_values::); } Ok(OwnedValue::Array(keys)) } @@ -6282,7 +6285,7 @@ fn each_lazy_keys_iterate_sink( Ok(keys) => keys, Err(e) => return Flow::Escaped(Control::Error(e)), }; - keys.sort_by(compare_values); + keys.sort_by(compare_values::); drive_pipe_elements_generic::( keys.into_iter().map(|k| Ok(GenericItem::Owned(k))), rest, @@ -14843,8 +14846,8 @@ fn sort_family_array_generic( /// been flattened to equal `OwnedValue`s, whereas the cursors this returns /// still point at distinct document positions that can print differently /// (two mappings with the same collapsed form but different duplicate keys). -fn sort_keyed_elements(keyed: &mut [(OwnedValue, V::Cursor)]) { - keyed.sort_by(|(a, _), (b, _)| compare_values(a, b)); +fn sort_keyed_elements(keyed: &mut [(OwnedValue, V::Cursor)]) { + keyed.sort_by(|(a, _), (b, _)| compare_values::(a, b)); } /// Whether `path(expr)` can be resolved by walking cursors instead of @@ -20631,7 +20634,7 @@ fn eval_builtin( // comment above for why). let cursors = owned_or_suppress!(elements.collect_cursors_checked(), optional); sort_family_array_generic::(cursors, key, optional, |mut keyed| { - sort_keyed_elements::(&mut keyed); + sort_keyed_elements::(&mut keyed); if dedup { // `owned_value_eq::`, not `compare_values(..) == // Equal`: the sort above stays widening, but two @@ -20700,11 +20703,11 @@ fn eval_builtin( let winner = if matches!(builtin, Builtin::Min | Builtin::MinBy(_)) { keyed .into_iter() - .min_by(|(a, _), (b, _)| compare_values(a, b)) + .min_by(|(a, _), (b, _)| compare_values::(a, b)) } else { keyed .into_iter() - .max_by(|(a, _), (b, _)| compare_values(a, b)) + .max_by(|(a, _), (b, _)| compare_values::(a, b)) }; match winner { Some((_, cursor)) => GenericResult::OneCursor(cursor), @@ -23025,13 +23028,15 @@ fn owned_identity_placed_by( OwnedValue::Array(items) => items .iter() .position(|item| { - crate::jq::eval::compare_values(item, output) == core::cmp::Ordering::Equal + crate::jq::eval::compare_values::(item, output) + == core::cmp::Ordering::Equal }) .map(|i| OwnedValue::Int(i as i64)), OwnedValue::Object(map) => map .iter() .find(|(_, item)| { - crate::jq::eval::compare_values(item, output) == core::cmp::Ordering::Equal + crate::jq::eval::compare_values::(item, output) + == core::cmp::Ordering::Equal }) .map(|(k, _)| OwnedValue::String(k.clone())), _ => None, diff --git a/src/jq/value.rs b/src/jq/value.rs index 41076e486..afe231f0f 100644 --- a/src/jq/value.rs +++ b/src/jq/value.rs @@ -153,6 +153,84 @@ pub(crate) fn parse_i64_or_f64(s: &str) -> Option { } } +/// Convert an integer *literal* to `f64` the way real jq 1.7.1 does (#2906). +/// +/// jq keeps every parsed number -- program text, document input, +/// `tonumber`, `fromjson`, `--argjson` -- as an exact `decNumber` literal, +/// and converts it to a double only when arithmetic first reads it +/// (`jvp_literal_number_to_double`, `src/jv.c`): `decNumberReduce` under a +/// `DEC_INIT_DECIMAL64` context whose `digits` is raised to 17 +/// (`DEC_NUBMER_DOUBLE_PRECISION`, round-half-even), `decNumberToString`, +/// then a correctly-rounded `strtod`. That is a *double* rounding: the +/// literal's decimal value is first rounded to 17 significant decimal +/// digits, and only that shorter number is rounded to the nearest double. +/// A plain `n as f64` rounds the exact integer once, and the two disagree +/// whenever the 17-digit intermediate sits on the other side of a +/// double's rounding boundary -- `869389897822472004` (18 digits) becomes +/// `869389897822472000`, whose nearest double is `869389897822471936` +/// (a tie, resolved to even), where the exact integer's nearest double is +/// `869389897822472064`. +/// +/// A decimal integer below `10^17` has at most 17 significant digits, so +/// the intermediate rounding is the identity there and this is exactly +/// `n as f64`; only 18- and 19-digit magnitudes are ever changed. The +/// rounding unit is `10` for 18 digits and `100` for 19 (an `i64`'s +/// magnitude is at most `2^63`, 19 digits), the tie rule is half-even on +/// the kept digit, and `q * unit` cannot overflow a `u64` (at most +/// `2^63 + 100`). The final `u64 as f64` is Rust's correctly-rounded +/// integer-to-float conversion, which is what `strtod` produces for a +/// decimal string of at most 17 digits, so no float arithmetic happens +/// before the last step. +/// +/// **This treats every jq-mode `Int` as a literal.** That is sound for the +/// `Int`s that are *not* literals: `+`/`-`/`*` already return a `Float` for +/// anything past `2^53` (`jq_checked_int_arith`, #2631) and `%` does the +/// same (`jq_f64_backed_int`); an `Int` that came *from* a double +/// (`floor`/`ceil`/`round`/`trunc`, `%`'s truncating operands) is exactly +/// representable, and for such values this function is provably the +/// identity -- doubles are at least 16 apart on `[10^17, 10^18)` and at +/// least 128 apart above `10^18`, while the 17-digit rounding moves a +/// value by at most 5 or 50, so it rounds straight back to the double it +/// started as; unary minus and `length` produce the exact negation or +/// magnitude of a literal, and this function is sign-symmetric; and the +/// reindex bridge re-bakes an `Int` as a `NumberLiteral` of the same +/// value. The one exact-`i64` producer past `2^53` that is *not* a literal +/// is `range`, which stays on its documented exact walk +/// (`docs/compliance/jq/limitations.md`). +/// +/// yq mode never calls this: real yq's arithmetic is exact `int64`, so its +/// `Int`-to-`f64` widening stays a plain cast (`int_to_f64`). +pub(crate) fn jq_literal_int_to_f64(n: i64) -> f64 { + const TEN_POW_17: u64 = 100_000_000_000_000_000; + const TEN_POW_18: u64 = 1_000_000_000_000_000_000; + let magnitude = n.unsigned_abs(); + if magnitude < TEN_POW_17 { + return n as f64; + } + let unit = if magnitude < TEN_POW_18 { 10 } else { 100 }; + let (quotient, remainder) = (magnitude / unit, magnitude % unit); + let round_up = remainder > unit / 2 || (remainder == unit / 2 && quotient & 1 == 1); + let rounded = ((quotient + u64::from(round_up)) * unit) as f64; + if n < 0 { + -rounded + } else { + rounded + } +} + +/// Widen an `Int` operand to `f64` under `S`'s number model: jq's +/// literal rounding ([`jq_literal_int_to_f64`]) when +/// `EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS`, otherwise the plain +/// cast real yq's `int64`-to-`float64` conversion amounts to. +#[inline] +pub(crate) fn int_to_f64(n: i64) -> f64 { + if S::INT_LITERAL_ROUNDS_TO_17_DIGITS { + jq_literal_int_to_f64(n) + } else { + n as f64 + } +} + /// Canonicalize a non-exponent literal's insignificant leading zero/`+` /// (#1224, mirroring #1180's identical fix for the exponent-notation /// mantissa path -- `split_mantissa`/`normalize_extreme_literal_mantissa` @@ -2640,9 +2718,18 @@ pub(crate) fn is_nan_sentinel(bytes: &[u8]) -> bool { /// - Objects compare order-insensitively (`IndexMap`'s own `PartialEq`), as in jq. /// /// Known divergence: above 2^53 a mixed `Int`/`Float` comparison widens the -/// integer to `f64`, whereas jq 1.7 retains the decimal literal. So -/// `9007199254740993 == 9007199254740992.0` is `true` here and `false` in jq. -/// Every value representable exactly as an `f64` agrees. +/// integer to `f64`, whereas jq 1.7 compares two *literals* exactly as +/// decimals. So `9007199254740993 == 9007199254740992.0` is `true` here and +/// `false` in jq. Every value representable exactly as an `f64` agrees. The +/// widening itself follows jq's own literal-to-double rule +/// ([`jq_literal_int_to_f64`], #2906), so an `Int` literal against a +/// *computed* `Float` -- `869389897822472004 == (869389897822472004 + 0)` +/// -- agrees with jq. +/// +/// This is jq's rule only, and needs no `EvalSemantics`: yq's own equality +/// (`STRICT_NUMERIC_EQUALITY`) never widens a mixed pair at all -- +/// [`owned_value_eq`] routes every yq-mode numeric pair to +/// [`numeric_repr_eq_strict`] before this can run. /// /// `NumberLiteral` compares purely on its parsed [`NumberRepr`], never on the /// source text -- two spellings of the same number (`1.0` and `1e0`) are @@ -2651,8 +2738,8 @@ pub(crate) fn numeric_repr_eq(a: NumberRepr, b: NumberRepr) -> bool { match (a, b) { (NumberRepr::Int(a), NumberRepr::Int(b)) => a == b, (NumberRepr::Float(a), NumberRepr::Float(b)) => a == b, - (NumberRepr::Int(a), NumberRepr::Float(b)) => (a as f64) == b, - (NumberRepr::Float(a), NumberRepr::Int(b)) => a == (b as f64), + (NumberRepr::Int(a), NumberRepr::Float(b)) => jq_literal_int_to_f64(a) == b, + (NumberRepr::Float(a), NumberRepr::Int(b)) => a == jq_literal_int_to_f64(b), } } @@ -2774,12 +2861,20 @@ pub(crate) fn cmp_f64(a: f64, b: f64) -> core::cmp::Ordering { /// `unique`/`group_by` disagreeing with `==` about the same two numbers. /// /// The NaN rule (#421) is centralized in [`cmp_f64`], not repeated here. -pub(crate) fn numeric_repr_cmp(a: NumberRepr, b: NumberRepr) -> core::cmp::Ordering { +/// +/// Unlike [`numeric_repr_eq`], ordering runs in both modes (`sort`, `min`, +/// `max`, `<` all reach it under yq too), so the mixed-pair widening is +/// mode-forked through [`int_to_f64`]: jq's literal rounding +/// ([`jq_literal_int_to_f64`], #2906) in jq mode, the plain cast in yq mode. +pub(crate) fn numeric_repr_cmp( + a: NumberRepr, + b: NumberRepr, +) -> core::cmp::Ordering { match (a, b) { (NumberRepr::Int(a), NumberRepr::Int(b)) => a.cmp(&b), (NumberRepr::Float(a), NumberRepr::Float(b)) => cmp_f64(a, b), - (NumberRepr::Int(a), NumberRepr::Float(b)) => cmp_f64(a as f64, b), - (NumberRepr::Float(a), NumberRepr::Int(b)) => cmp_f64(a, b as f64), + (NumberRepr::Int(a), NumberRepr::Float(b)) => cmp_f64(int_to_f64::(a), b), + (NumberRepr::Float(a), NumberRepr::Int(b)) => cmp_f64(a, int_to_f64::(b)), } } @@ -2891,8 +2986,12 @@ fn owned_value_eq_at_depth(a: &OwnedValue, b: &OwnedValue, depth: usize) -> bool (OwnedValue::Bool(a), OwnedValue::Bool(b)) => a == b, (OwnedValue::Int(a), OwnedValue::Int(b)) => a == b, (OwnedValue::Float(a), OwnedValue::Float(b)) => a == b, - (OwnedValue::Int(a), OwnedValue::Float(b)) => (*a as f64) == *b, - (OwnedValue::Float(a), OwnedValue::Int(b)) => *a == (*b as f64), + (OwnedValue::Int(a), OwnedValue::Float(b)) => { + numeric_repr_eq(NumberRepr::Int(*a), NumberRepr::Float(*b)) + } + (OwnedValue::Float(a), OwnedValue::Int(b)) => { + numeric_repr_eq(NumberRepr::Float(*a), NumberRepr::Int(*b)) + } (OwnedValue::NumberLiteral(a, _), OwnedValue::NumberLiteral(b, _)) => { numeric_repr_eq(*a, *b) } @@ -3500,6 +3599,13 @@ mod tests { OwnedValue::from_number_literal("9007199254740993"), OwnedValue::Float(9007199254740992.0), ), + // #2906: an 18-digit `Int` literal against the double jq's own + // literal rounding produces for it (not the exact integer's + // nearest double, `869389897822472064.0`). + ( + OwnedValue::from_number_literal("869389897822472004"), + OwnedValue::Float(869389897822471936.0), + ), ]; for (a, b) in pairs { let are_eq = a == b; @@ -3511,13 +3617,170 @@ mod tests { "numeric_repr_eq disagrees with OwnedValue::eq for {a:?} vs {b:?}" ); assert_eq!( - numeric_repr_cmp(ra, rb) == core::cmp::Ordering::Equal, + numeric_repr_cmp::(ra, rb) == core::cmp::Ordering::Equal, are_eq, "numeric_repr_cmp disagrees with OwnedValue::eq for {a:?} vs {b:?}" ); } } + /// #2906: jq converts an integer literal to a double by first rounding + /// its decimal digits to 17 significant digits (half-even), then + /// rounding that to the nearest double. Every expectation here is the + /// double real jq 1.7.1 prints for ` + 0`, compared bit-exact. + #[test] + fn test_jq_literal_int_to_f64_rounds_to_17_decimal_digits_2906() { + let cases: [(i64, f64); 16] = [ + // Below 10^17 the 17-digit rounding is the identity. + (0, 0.0), + (9007199254740993, 9007199254740992.0), + (10000000000000005, 10000000000000004.0), + (99999999999999999, 100000000000000000.0), + // 10^17 itself and a tie that rounds to the even kept digit. + (100000000000000000, 100000000000000000.0), + (100000000000000005, 100000000000000000.0), + (100000000000000015, 100000000000000020.0), + // The issue's own operand: 18 digits, rounds to ...000, whose + // nearest double is ...1936 (tie to even), not the exact + // integer's nearest double ...2064. + (869389897822472004, 869389897822471936.0), + // Ties on an odd/even kept digit. + (123456789012345675, 123456789012345680.0), + (123456789012345665, 123456789012345660.0), + (123456789012345685, 123456789012345680.0), + // Carry across every digit: 18 digits become 10^18. + (999999999999999995, 1000000000000000000.0), + // 19 digits round in units of 100. + (1234567890123456750, 1234567890123456800.0), + (1234567890123456650, 1234567890123456600.0), + (i64::MAX, 9223372036854775808.0), + (i64::MIN, -9223372036854775808.0), + ]; + for (n, want) in cases { + let got = jq_literal_int_to_f64(n); + assert_eq!( + got.to_bits(), + want.to_bits(), + "jq_literal_int_to_f64({n}) = {got}, want {want}" + ); + // `0` negates to `-0.0` (a different bit pattern) and `i64::MIN` + // has no negation, so those two only check the positive side. + if let Some(negated) = n.checked_neg().filter(|_| n != 0) { + assert_eq!( + jq_literal_int_to_f64(negated).to_bits(), + (-want).to_bits(), + "jq_literal_int_to_f64 is not sign-symmetric at {n}" + ); + } + } + } + + /// Hand-round the decimal spelling of `n` to 17 significant digits + /// (half-even) and parse the result -- an independent, string-based + /// route to the same double [`jq_literal_int_to_f64`] computes with + /// integer arithmetic, mirroring jq's own `decNumberToString` + + /// `strtod` shape. + fn round_decimal_string_to_17_digits(n: i64) -> f64 { + let digits: Vec = n.unsigned_abs().to_string().into_bytes(); + if digits.len() <= 17 { + return n as f64; + } + let (kept, dropped) = digits.split_at(17); + let mut kept: Vec = kept.to_vec(); + let first_dropped = dropped[0]; + let rest_nonzero = dropped[1..].iter().any(|d| *d != b'0'); + let last_kept_odd = (kept[16] - b'0') % 2 == 1; + let round_up = + first_dropped > b'5' || (first_dropped == b'5' && (rest_nonzero || last_kept_odd)); + let mut exponent = dropped.len(); + if round_up { + let mut i = 16; + loop { + if kept[i] == b'9' { + kept[i] = b'0'; + if i == 0 { + kept.insert(0, b'1'); + kept.pop(); + exponent += 1; + break; + } + i -= 1; + } else { + kept[i] += 1; + break; + } + } + } + let sign = if n < 0 { "-" } else { "" }; + let text = format!("{sign}{}e{exponent}", String::from_utf8(kept).unwrap()); + text.parse::().unwrap() + } + + /// #2906: property check of [`jq_literal_int_to_f64`] over seeded + /// pseudo-random `i64`s spanning every magnitude -- against the + /// independent string-based rounding above, sign symmetry, the + /// "already a double, so a no-op" lemma the helper's soundness rests + /// on, monotonicity, and yq mode's untouched plain cast. + #[test] + fn test_jq_literal_int_to_f64_properties_2906() { + // xorshift64*, seeded; no external crate needed here. + let mut state: u64 = 0x2906_2906_2906_2906; + let mut next = || { + state ^= state >> 12; + state ^= state << 25; + state ^= state >> 27; + state.wrapping_mul(0x2545_F491_4F6C_DD1D) + }; + let mut previous: Option<(i64, f64)> = None; + for i in 0..20_000 { + // Spread the samples across the 18/19-digit band and below it, + // rather than letting a uniform u64 land past 10^18 nearly every + // time. + let raw = next(); + let n = match i % 4 { + 0 => (raw % 1_000_000_000_000_000_000) as i64, + 1 => (raw % 100_000_000_000_000_000) as i64 + 100_000_000_000_000_000, + 2 => raw as i64, + _ => (raw % 1_000_000_000_000_000_000) as i64 + 1_000_000_000_000_000_000, + }; + let got = jq_literal_int_to_f64(n); + assert_eq!( + got.to_bits(), + round_decimal_string_to_17_digits(n).to_bits(), + "integer and string roundings disagree at {n}" + ); + if let Some(negated) = n.checked_neg().filter(|_| n != 0) { + assert_eq!( + jq_literal_int_to_f64(negated).to_bits(), + (-got).to_bits(), + "not sign-symmetric at {n}" + ); + } + let as_double = n as f64; + if as_double as i64 == n { + assert_eq!( + got.to_bits(), + as_double.to_bits(), + "an exactly representable {n} must round to itself" + ); + } + assert_eq!( + int_to_f64::(n).to_bits(), + as_double.to_bits(), + "yq mode must stay a plain cast at {n}" + ); + assert_eq!(int_to_f64::(n).to_bits(), got.to_bits()); + if let Some((prev_n, prev_got)) = previous { + if prev_n <= n { + assert!(prev_got <= got, "not monotone: {prev_n} -> {n}"); + } else { + assert!(prev_got >= got, "not monotone: {prev_n} -> {n}"); + } + } + previous = Some((n, got)); + } + } + #[test] fn test_number_repr_is_none_for_non_numeric_variants() { assert_eq!(OwnedValue::Null.number_repr(), None); diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index 3a834ef80..0964d7b61 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -53676,6 +53676,172 @@ fn test_int_arith_within_exact_f64_range_or_already_overflowing_unaffected_2631( Ok(()) } +/// #2906: jq converts an integer *literal* to a double by first rounding +/// its decimal digits to 17 significant digits (half-even, +/// `jvp_literal_number_to_double` in jq's `src/jv.c`), then rounding that +/// to the nearest double -- so an 18/19-digit literal can land on a +/// different double than `i64 as f64` gives, and every arithmetic result +/// built on it differs. `%` follows jq's `binop_mod` (each operand's double +/// truncated to `intmax_t`). Every expectation was captured live against +/// `/usr/bin/jq` 1.7.1. +#[test] +fn test_large_int_literal_rounds_to_17_digits_before_f64_arith_2906() -> Result<()> { + for (filter, want) in [ + // The issue's own repro: the literal rounds to 869389897822472000, + // whose nearest double is ...1936 (tie to even), not the exact + // integer's nearest double ...2064 -- so the sum is ...6300, not + // ...6400. + ("869389897822472004 + 944331", "869389897823416300"), + ("869389897822472004 + 944331 + 1", "869389897823416300"), + ("869389897822472004 * 2", "1738779795644944000"), + ("869389897822472004 / 3", "289796632607490600"), + ("869389897822472004 % 1000", "936"), + ("869389897822472004 % 7", "1"), + ("1000 % 869389897822472004", "1000"), + // Below 10^17 the literal is already jq's double, but `%` still + // truncates that double, not the exact integer. + ("9007199254740993 % 2", "0"), + // Ties resolve to the even kept digit; carries propagate. + ("123456789012345675 + 0", "123456789012345680"), + ("123456789012345685 + 0", "123456789012345680"), + ("123456789012345665 + 0", "123456789012345660"), + ("100000000000000015 + 0", "100000000000000020"), + ("999999999999999995 + 0", "1e+18"), + // 19 digits round in units of 100; the second's 17-digit + // intermediate ...6600 then rounds to the double ...6512. + ("1234567890123456750 + 0", "1234567890123456800"), + ("1234567890123456650 + 0", "1234567890123456500"), + // `tonumber` and `fromjson` yield literals too. + ( + "\"869389897822472004\" | tonumber + 944331", + "869389897823416300", + ), + ( + "\"869389897822472004\" | fromjson + 944331", + "869389897823416300", + ), + ] { + let (output, code) = run_jq_null(filter, &["-c"])?; + assert_eq!(code, 0, "`{filter}`"); + assert_eq!(output.trim(), want, "`{filter}`"); + } + Ok(()) +} + +/// #2906: the same literal-to-double rule applies when an `Int` is widened +/// for `==`/`!=` and the ordering operators, so a literal compares equal to +/// the double jq's own arithmetic produced from it. Two literals still +/// compare exactly (`869389897822472004 == 869389897822472000` stays +/// `false`, as in jq's `decNumberCompare`). Captured live against +/// `/usr/bin/jq` 1.7.1. +/// +/// Only the binary operators are pinned here. `sort`/`unique`/`min`/`max` +/// on an array built in the filter cross the reindex bridge, which +/// re-parses the computed double `869389897822472000` as an *integer* +/// literal, and from then on it compares exactly against the real literal +/// (`[869389897822472004, (869389897822472000+0), 5] | sort` is +/// `[5,869389897822472000,869389897822472004]` here and +/// `[5,869389897822472004,869389897822472000]` in jq -- identical before +/// and after #2906, a separate bridge gap recorded in +/// `docs/compliance/jq/limitations.md`). +#[test] +fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<()> { + for (filter, want) in [ + ("(869389897822472004+0) == (869389897822472000+0)", "true"), + ("(869389897822472004+0) != (869389897822472000+0)", "false"), + ("869389897822472004 == (869389897822472004+0)", "true"), + ("869389897822472004 == (869389897822472000+0)", "true"), + ("869389897822472004 == 869389897822472000", "false"), + ( + "[869389897822472004 < (869389897822472004+0), 869389897822472004 > (869389897822472004+0)]", + "[false,false]", + ), + ( + "[869389897822472004 <= (869389897822472000+0), 869389897822472004 >= (869389897822472000+0)]", + "[true,true]", + ), + ( + "[(869389897822472000+0) < 869389897822472004, 869389897822472004 < 869389897822472100]", + "[false,true]", + ), + ] { + let (output, code) = run_jq_null(filter, &["-c"])?; + assert_eq!(code, 0, "`{filter}`"); + assert_eq!(output.trim(), want, "`{filter}`"); + } + Ok(()) +} + +/// #2906: document input, `--argjson`, and the generic (stdin) evaluator +/// reach the same arithmetic as `-n` program literals, including a +/// negative literal arriving as data (unaffected by the separate #2357 +/// unary-minus divergence). Captured live against `/usr/bin/jq` 1.7.1. +#[test] +fn test_large_int_literal_rounds_to_17_digits_from_input_sources_2906() -> Result<()> { + let (output, code) = run_jq_stdin(". + 944331", "869389897822472004", &["-c"])?; + assert_eq!(code, 0); + assert_eq!(output.trim(), "869389897823416300"); + + let (output, code) = run_jq_stdin( + "[.[0] + .[1], .[2] + .[1], .[0]]", + "[869389897822472004, 944331, -869389897822472004]", + &["-c"], + )?; + assert_eq!(code, 0); + assert_eq!( + output.trim(), + "[869389897823416300,-869389897821527600,869389897822472004]" + ); + + let (output, code) = run_jq_stdin( + "reduce .[] as $x (0; . + $x)", + "[869389897822472004, 944331]", + &["-c"], + )?; + assert_eq!(code, 0); + assert_eq!(output.trim(), "869389897823416300"); + + let (output, code) = run_jq_null( + "[$a, $a + 944331]", + &["-c", "--argjson", "a", "869389897822472004"], + )?; + assert_eq!(code, 0); + assert_eq!(output.trim(), "[869389897822472004,869389897823416300]"); + Ok(()) +} + +/// #2906 must-not-regress companion: a literal that is only *displayed* +/// keeps its exact spelling, 17-or-fewer-digit literals are unchanged (the +/// rounding is the identity below 10^17), genuine `i64` overflow still goes +/// through the pre-existing float path, and the #2357 unary-minus +/// divergence (succinctly keeps the exact value) is untouched. jq's answers +/// captured live against `/usr/bin/jq` 1.7.1 except where noted. +#[test] +fn test_large_int_literal_rounding_leaves_display_and_small_ints_alone_2906() -> Result<()> { + for (filter, want) in [ + ("869389897822472004", "869389897822472004"), + ("869389897822472004 | tostring", "\"869389897822472004\""), + ("[869389897822472004] | tojson", "\"[869389897822472004]\""), + ("10000000000000005 + 0", "10000000000000004"), + ("99999999999999999 + 0", "1e+17"), + ("100000000000000000 + 0", "1e+17"), + ("9223372036854775807 + 1", "9223372036854776000"), + ( + "9223372036854775807 + 9223372036854775807", + "18446744073709552000", + ), + // #2357: real jq prints -869389897822472000 here; succinctly's + // documented divergence keeps the exact literal, and this fix must + // not change that (see `docs/compliance/jq/limitations.md`). + ("0 | -869389897822472004", "-869389897822472004"), + ] { + let (output, code) = run_jq_null(filter, &["-c"])?; + assert_eq!(code, 0, "`{filter}`"); + assert_eq!(output.trim(), want, "`{filter}`"); + } + Ok(()) +} + /// #2259: `getpath(EXPR)`'s path-context arm (reached whenever a downstream /// `key`/`parent`/`path`/`file_index` forces path-context routing) used to /// drain `EXPR`'s whole path-argument generator up front and walk each path diff --git a/tests/yq_cli_tests.rs b/tests/yq_cli_tests.rs index 7475d50dc..ca4de2cfd 100644 --- a/tests/yq_cli_tests.rs +++ b/tests/yq_cli_tests.rs @@ -45771,6 +45771,30 @@ fn test_yq_int_overflow_wraps_unaffected_by_2631() -> Result<()> { Ok(()) } +/// #2906's fix (jq mode only, see `jq_cli_tests.rs`) rounds an integer +/// literal to 17 significant decimal digits before widening it to `f64`, +/// and routes `%` through jq's truncate-the-double model -- both gated on +/// `S::INT_LITERAL_ROUNDS_TO_17_DIGITS`/`S::OVERFLOW_WRAPS`. Real yq's +/// `int64` arithmetic is exact, and every value here was captured live +/// against yq v4.53.3. +#[test] +fn test_yq_large_int_arith_stays_exact_unaffected_by_2906() -> Result<()> { + let input = "a: 869389897822472004\nb: 944331\n"; + for (filter, want) in [ + (".a + .b", "869389897823416335"), + (".a - .b", "869389897821527673"), + (".a * 2", "1738779795644944008"), + (".a % 1000", "4"), + (".a == .a + 0", "true"), + ("[.a, .b] | sort", "[944331,869389897822472004]"), + ] { + let (out, code) = run_yq_stdin(filter, input, &["-o=json", "-I0"])?; + assert_eq!(code, 0, "`{filter}` out={out:?}"); + assert_eq!(out.trim(), want, "`{filter}`"); + } + Ok(()) +} + /// #2259's jq-mode fix (see `jq_cli_tests.rs`) is shared, mode-generic code /// (`path_context_component_each`/`path_context_step_getpath`, /// `src/jq/eval_generic.rs`), so yq mode gets it too, behind From 386170e7bcbde11604e46f193e86ee8058b81fbe Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 14 Sep 2026 10:22:24 +1000 Subject: [PATCH 2/5] docs(jq): link the #2906 follow-up issues from the limitations entry The out-of-scope surfaces the #2906 entry lists are now filed: >17-digit float and over-i64 literals (#2936), the math builtins' widening and the floor-family display past 2^53 (#2937), and the reindex bridge re-parsing a computed double as an integer literal before sort/unique/min/max (#2938). --- docs/compliance/jq/limitations.md | 10 +++++----- tests/jq_cli_tests.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index d66064fa6..555550897 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -5822,15 +5822,15 @@ it wherever an `Int` is widened for `+`/`-`/`*`/`/`/`%`, for `==`, and for order exact (`869389897822472004 + 944331` is `869389897823416335` there, as it always was here), so its widening stays a plain cast (`EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS`). -**Still open, same mechanism, out of #2906's scope** (tracked as follow-up issues): +**Still open, same mechanism, out of #2906's scope** (tracked as #2936, #2937 and #2938): a *float* literal with more than 17 significant digits, or an integer literal beyond `i64`, is still parsed with one correct rounding (`2.7293109604053567083 + 0` is `2.7293109604053565` in jq, `2.729310960405357` here) — closing it needs the mode plumbed -through the number-materialisation funnels, not just the arithmetic; the math builtins -(`floor`, `sqrt`, `pow`, …) still widen a large `Int` with a bare cast +through the number-materialisation funnels, not just the arithmetic (#2936); the math +builtins (`floor`, `sqrt`, `pow`, …) still widen a large `Int` with a bare cast (`869389897822472004 | sqrt` is `932410798.8555645` in jq, `…647` here); and `floor`/`ceil`/`round`/`trunc` of such a value print their exact integer digits where jq -prints the double (`869389897822472000`). Two *literals* compared against each other are +prints the double (`869389897822472000`) (both #2937). Two *literals* compared against each other are still widened here where jq compares them exactly as decimals (`9007199254740993 == 9007199254740992.0`, `numeric_repr_eq`'s own doc comment). And a computed double that crosses the reindex bridge into a document-input builtin (`sort`, `unique`, `min`, `max`, @@ -5840,7 +5840,7 @@ crosses the reindex bridge into a document-input builtin (`sort`, `unique`, `min `[5,869389897822472000,869389897822472004]` here and `[5,869389897822472004,869389897822472000]` in jq (stable, the two are equal there) — identical before and after this fix, since the comparator is right and the bridge is what -changes the operand. The `range` entry above and the unary-minus entry (#2357) are +changes the operand (#2938). The `range` entry above and the unary-minus entry (#2357) are unaffected — both stay on the exact `i64` path they document. ### `--argjson`/`--jsonargs` still reject a bare trailing decimal point with no exponent (`1.`) — accepted divergence, ADR-0018 rule 4c (#2240) diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index 0964d7b61..59e4af503 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -53742,7 +53742,7 @@ fn test_large_int_literal_rounds_to_17_digits_before_f64_arith_2906() -> Result< /// (`[869389897822472004, (869389897822472000+0), 5] | sort` is /// `[5,869389897822472000,869389897822472004]` here and /// `[5,869389897822472004,869389897822472000]` in jq -- identical before -/// and after #2906, a separate bridge gap recorded in +/// and after #2906, a separate bridge gap tracked as #2938 and recorded in /// `docs/compliance/jq/limitations.md`). #[test] fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<()> { From 3b31c67af3ceb9ca65522120035b10caa927168c Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 14 Sep 2026 10:41:58 +1000 Subject: [PATCH 3/5] fix(jq): compare two number literals exactly and keep yq's plain widening (#2906) Review of the #2906 fix found two regressions in its comparison half. jq's `jvp_number_cmp` has two rules, not one: a literal against a *computed* double widens the literal through the 17-digit rounding, but two *literals* compare exactly as decimals (`decNumberCompare`). Rounding unconditionally made `869389897822472004 == 869389897822471936.0` true (jq: false) and `869389897822472004.0` unequal to its own integer spelling (jq: equal). And `OwnedValue`'s mode-blind `PartialEq` -- which the yq presentation layer uses to align comments across a `|=` -- had inherited the jq rounding, so a yq-mode `Int` no longer matched the plain-cast double yq's own arithmetic produces and comments moved onto the wrong elements. Add `jq_numeric_cmp`: bare `Int` and `NumberLiteral` are literals, bare `Float` is computed; two literals are ordered by their correctly-rounded doubles (monotonic, so a strict order is exact) and only a tie falls through to `cmp_decimal_literals`, an exact digit comparison of the two spellings. jq-mode `==` (`owned_value_eq`) and ordering (`compare_values`) go through it; `PartialEq`/`numeric_repr_eq`/`numeric_repr_cmp` go back to the plain widening, so yq mode and its alignment hash are untouched. As a consequence `9007199254740993 == 9007199254740992.0` is now jq's `false` rather than the recorded divergence. Also from the review: rename the gate to `DECNUMBER_LITERALS`; take `%`'s in-range operands straight to `wrapping_rem` and delegate the rest to `mod_floats` (one copy of `binop_mod`); let `jq_checked_int_arith`'s exact arm use `jq_f64_backed_int`; keep direct `Int`/`Int` and `Float`/`Float` arms in `compare_values`; derive `delete_paths_*`'s yq flag from `S`; correct the `materialize_lazy_keys` comment; pin the three follow-up gaps (#2936, #2937, #2938) with a characterization test; and note jq's `dtoi` UB at 2^63 and the `-x % y` precedence gap it exposes. --- docs/compliance/jq/limitations.md | 47 ++- src/jq/eval.rs | 124 ++++---- src/jq/eval_generic.rs | 9 +- src/jq/value.rs | 440 +++++++++++++++++++++++------ tests/jq_cli_tests.rs | 139 +++++++-- tests/jq_evaluator_parity_tests.rs | 16 +- 6 files changed, 589 insertions(+), 186 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 555550897..9ffc90ddb 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -5814,13 +5814,33 @@ For an `i64` the two conversions differ only when the magnitude is at least `10^ intermediate rounding is the identity and `n as f64` was already right — everything in `[2^53, 10^17)` behaved correctly before this fix. `jq_literal_int_to_f64` (`src/jq/value.rs`) implements the rounding with integer arithmetic only, and jq mode uses -it wherever an `Int` is widened for `+`/`-`/`*`/`/`/`%`, for `==`, and for ordering -(`<`, `sort`, `unique`, `group_by`, `min`/`max`, `bsearch`), in both evaluators; -`%` additionally follows `binop_mod`'s truncate-the-double model, so -`869389897822472004 % 1000` is `936` (jq) rather than the exact `4`, and -`9007199254740993 % 2` is `0`. yq mode is untouched: real yq's `int64` arithmetic is -exact (`869389897822472004 + 944331` is `869389897823416335` there, as it always was -here), so its widening stays a plain cast (`EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS`). +it wherever an `Int` is widened for `+`/`-`/`*`/`/`/`%`, in both evaluators; `%` +additionally follows `binop_mod`'s truncate-the-double model, so `869389897822472004 % +1000` is `936` (jq) rather than the exact `4`, and `9007199254740993 % 2` is `0`. + +Comparison follows `jvp_number_cmp`, which has two rules (`jq_numeric_cmp`, behind +jq-mode `==` and every ordering consumer — `<`, `sort`, `unique`, `group_by`, `min`/`max`, +`bsearch`): a literal against a *computed* double widens the literal through the same +17-digit rounding, so `869389897822472004 == (869389897822472004 + 0)` is `true`; two +*literals* compare exactly as decimals (`decNumberCompare`), with no rounding on either +side, so `869389897822472004 == 869389897822471936.0` is `false` while `869389897822472004 +== 869389897822472004.0` is `true`, `9007199254740993 == 9007199254740992.0` is `false` +(previously a recorded divergence), and `1.00000000000000001 == 1.0` is `false`. The +mode-blind `PartialEq` on `OwnedValue` keeps the old plain-cast widening: the yq +presentation layer relies on it (comment alignment across a `|=` matches a yq-mode `Int` +against the plain-cast double yq's own arithmetic produced), and a review round caught +that routing it through the jq rounding moved yq comments onto the wrong elements. yq mode +is otherwise untouched: real yq's `int64` arithmetic is exact (`869389897822472004 + +944331` is `869389897823416335` there, as it always was here), so its widening stays a +plain cast (`EvalSemantics::DECNUMBER_LITERALS`). + +Two things #2906 makes *visible* without changing: jq's `dtoi` on exactly `2^63` is a C +cast that saturates on the arm64 oracle (`as i64` does the same) but is undefined +behaviour and yields `INT64_MIN` on x86_64 jq, so `9223372036854775807 % 10` is `7` on the +pin and `-8` there; and jq parses `-x % y` as `-(x % y)` (unary minus binds looser than +`%`) where succinctly parses `(-x) % y`, a pre-existing precedence gap that only shows at +this magnitude (`-9223372036854775807 % 10` is `-7` in jq, `-8` here; the parenthesised +and data-sourced spellings agree on `-8`). **Still open, same mechanism, out of #2906's scope** (tracked as #2936, #2937 and #2938): a *float* literal with more than 17 significant digits, or an integer literal beyond @@ -5829,13 +5849,12 @@ a *float* literal with more than 17 significant digits, or an integer literal be through the number-materialisation funnels, not just the arithmetic (#2936); the math builtins (`floor`, `sqrt`, `pow`, …) still widen a large `Int` with a bare cast (`869389897822472004 | sqrt` is `932410798.8555645` in jq, `…647` here); and -`floor`/`ceil`/`round`/`trunc` of such a value print their exact integer digits where jq -prints the double (`869389897822472000`) (both #2937). Two *literals* compared against each other are -still widened here where jq compares them exactly as decimals (`9007199254740993 == -9007199254740992.0`, `numeric_repr_eq`'s own doc comment). And a computed double that -crosses the reindex bridge into a document-input builtin (`sort`, `unique`, `min`, `max`, -`group_by` on an array built in the filter) is re-parsed from its printed digits as an -*integer* literal, so it then compares exactly against a real literal instead of equal: +`floor`/`ceil`/`round`/`trunc` (and `length`, which is `fabs(jv_number_value(x))` in jq) +of such a value print their exact integer digits where jq prints the double +(`869389897822472000`) (all #2937). And a computed double that crosses the reindex bridge +into a document-input builtin (`sort`, `unique`, `min`, `max`, `group_by` on an array +built in the filter) is re-parsed from its printed digits as an *integer* literal, so it +then compares exactly against a real literal instead of equal: `[869389897822472004, (869389897822472000+0), 5] | sort` is `[5,869389897822472000,869389897822472004]` here and `[5,869389897822472004,869389897822472000]` in jq (stable, the two are equal there) — diff --git a/src/jq/eval.rs b/src/jq/eval.rs index 575a0a2b2..cf1902414 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -71,12 +71,15 @@ pub enum EvalTag { pub trait EvalSemantics: Copy + Default { /// If true, integer overflow wraps (yq). If false, converts to float (jq). const OVERFLOW_WRAPS: bool; - /// If true (jq), an `Int` widened to `f64` for arithmetic or ordering is - /// treated as a decNumber *literal* and rounded to 17 significant - /// decimal digits before the double conversion, matching jq 1.7.1's - /// `jvp_literal_number_to_double` (#2906, `jq_literal_int_to_f64`). If - /// false (yq), the widening is a plain cast. - const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool; + /// If true (jq), numbers follow jq 1.7.1's decNumber literal model + /// (#2906): every parsed number is an exact decimal *literal*, two + /// literals compare exactly (`jq_numeric_cmp`), and a literal widened to + /// `f64` for arithmetic or for comparison against a computed double is + /// first rounded to 17 significant decimal digits + /// (`jvp_literal_number_to_double`, `jq_literal_int_to_f64`). If false + /// (yq), integers are plain `int64`s and the widening is a plain cast. + /// jq-only by definition -- not an independently tunable knob. + const DECNUMBER_LITERALS: bool; /// If true, division by zero returns infinity (yq). If false, returns error (jq). const DIV_BY_ZERO_IS_INFINITY: bool; /// If true (yq), `has()`/`in()`'s array-index arm accepts *any* negative @@ -313,7 +316,7 @@ pub struct JqSemantics; impl EvalSemantics for JqSemantics { const OVERFLOW_WRAPS: bool = false; - const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool = true; + const DECNUMBER_LITERALS: bool = true; const DIV_BY_ZERO_IS_INFINITY: bool = false; const NEGATIVE_INDEX_IN_HAS: bool = false; const MOD_TRUNCATES_FLOATS: bool = true; @@ -347,7 +350,7 @@ pub struct YqSemantics; impl EvalSemantics for YqSemantics { const OVERFLOW_WRAPS: bool = true; - const INT_LITERAL_ROUNDS_TO_17_DIGITS: bool = false; + const DECNUMBER_LITERALS: bool = false; const DIV_BY_ZERO_IS_INFINITY: bool = true; const NEGATIVE_INDEX_IN_HAS: bool = true; const MOD_TRUNCATES_FLOATS: bool = false; @@ -377,9 +380,9 @@ use super::expr::{ Pattern, PatternEntry, SliceBoundKey, StringPart, Tracked, }; use super::value::{ - assert_value_tree_depth, infinite_float_preview_text, int_to_f64, is_infinity_sentinel, - is_nan_sentinel, jq_literal_int_to_f64, numeric_repr_cmp, owned_value_eq, NumberRepr, - OwnedValue, + assert_value_tree_depth, cmp_f64, infinite_float_preview_text, int_to_f64, + is_infinity_sentinel, is_nan_sentinel, jq_literal_int_to_f64, jq_numeric_cmp, numeric_repr_cmp, + owned_value_eq, NumberRepr, OwnedValue, }; /// Which binary operator an operand that produced *zero outputs* is being @@ -8862,12 +8865,11 @@ fn jq_checked_int_arith( f64_op: impl FnOnce(f64, f64) -> f64, ) -> OwnedValue { match checked_op(a, b) { - Some(result) - if jq_int_within_exact_f64_range(a) - && jq_int_within_exact_f64_range(b) - && jq_int_within_exact_f64_range(result) => - { - OwnedValue::Int(result) + // Both operands exact: the exact `i64` result is what the double + // arithmetic would compute, and `jq_f64_backed_int` decides whether + // it stays an `Int` or becomes jq's double past 2^53. + Some(result) if jq_int_within_exact_f64_range(a) && jq_int_within_exact_f64_range(b) => { + jq_f64_backed_int(result) } _ => OwnedValue::Float(f64_op(jq_literal_int_to_f64(a), jq_literal_int_to_f64(b))), } @@ -9598,29 +9600,23 @@ fn arith_mod( } else { Err(EvalError::divisor_is_zero(&left, &right, BinOp::Modulo)) } - } else if S::OVERFLOW_WRAPS { - // yq behavior: exact int64 remainder. + } else if !S::MOD_TRUNCATES_FLOATS + || (jq_int_within_exact_f64_range(a) && jq_int_within_exact_f64_range(b)) + { + // yq: exact int64 remainder. jq: the same whenever both + // operands are exact doubles -- the literal rounding is the + // identity there, the `intmax_t` truncation round-trips, and + // `|a % b| < |b| <= 2^53` keeps the result an exact `Int` + // (#2906). `wrapping_rem`: `i64::MIN % -1` must not panic. Ok(OwnedValue::Int(a.wrapping_rem(b))) } else { - // jq behavior (#2906): `binop_mod` truncates each operand's - // *double* -- for a literal, the 17-digit-rounded one -- to - // `intmax_t` with a saturating cast (`dtoi`), takes the - // remainder, and holds the result as a double again. `as - // i64` is the same saturating truncation. The divisor - // cannot truncate to zero here (a nonzero `i64` never rounds - // to zero), but `% -1` must short-circuit to `0` exactly as - // jq does rather than trip `i64::MIN % -1`. - let (ai, bi) = ( - jq_literal_int_to_f64(a) as i64, - jq_literal_int_to_f64(b) as i64, - ); - if bi == 0 { - Err(EvalError::divisor_is_zero(&left, &right, BinOp::Modulo)) - } else if bi == -1 { - Ok(OwnedValue::Int(0)) - } else { - Ok(jq_f64_backed_int(ai.wrapping_rem(bi))) - } + // jq (#2906): `binop_mod` is `dtoi(a) % dtoi(b)` on the + // operands' *doubles* -- a literal's 17-digit-rounded one -- + // with the remainder held as a double again, which is + // exactly `mod_floats`' truncating model. (`dtoi` saturates + // like `as i64` on the arm64 oracle; the C cast of exactly + // 2^63 is undefined behaviour and differs on x86_64 jq.) + mod_floats::(int_to_f64::(a), int_to_f64::(b), &left, &right) } } (Some(NumberRepr::Float(a)), Some(NumberRepr::Float(b))) => { @@ -9779,20 +9775,29 @@ fn compare_values_at_depth( match (left, right) { (OwnedValue::Null, OwnedValue::Null) => Ordering::Equal, (OwnedValue::Bool(a), OwnedValue::Bool(b)) => a.cmp(b), - // Every numeric pairing -- `Int`, `Float`, or `NumberLiteral` in any - // combination -- compares by parsed value through one dispatch; - // ordering never looks at a literal's source text. - // `numeric_repr_cmp` uses the same `(Int,Int)`/`(Float,Float)`/mixed - // pairing `==` uses (`numeric_repr_eq`), so ordering can't disagree - // with equality about the same pair (see its doc comment), and its - // mixed arms widen an `Int` under `S`'s number model (#2906). + // Same-variant pairs order the same way under every number model, + // and are the hot case for a sort of computed values. + (OwnedValue::Int(a), OwnedValue::Int(b)) => a.cmp(b), + (OwnedValue::Float(a), OwnedValue::Float(b)) => cmp_f64(*a, *b), + // Every other numeric pairing follows the mode's own number model + // (#2906): jq's decNumber rule (`jq_numeric_cmp` -- two literals + // exactly, a literal against a computed double through the 17-digit + // rounding; the same function `owned_value_eq` uses, so ordering + // can't disagree with `==` about a pair), or yq's plain widening + // (`numeric_repr_cmp`, the same dispatch as `numeric_repr_eq`). ( OwnedValue::Int(_) | OwnedValue::Float(_) | OwnedValue::NumberLiteral(..), OwnedValue::Int(_) | OwnedValue::Float(_) | OwnedValue::NumberLiteral(..), - ) => match (left.number_repr(), right.number_repr()) { - (Some(a), Some(b)) => numeric_repr_cmp::(a, b), - _ => Ordering::Equal, - }, + ) => { + if S::DECNUMBER_LITERALS { + jq_numeric_cmp(left, right).expect("both operands are numbers") + } else { + let (Some(a), Some(b)) = (left.number_repr(), right.number_repr()) else { + unreachable!("number_repr is Some for every numeric variant") + }; + numeric_repr_cmp(a, b) + } + } (OwnedValue::String(a), OwnedValue::String(b)) => a.cmp(b), (OwnedValue::Array(a), OwnedValue::Array(b)) => { for (av, bv) in a.iter().zip(b.iter()) { @@ -43071,8 +43076,8 @@ fn delete_paths_sorted( mut value: OwnedValue, paths: &[&[OwnedValue]], start: usize, - yq_mode: bool, ) -> Result { + let yq_mode = S::TAG == EvalTag::Yq; debug_assert!( paths.iter().all(|p| p.len() > start), "every path in a run is longer than the depth it is grouped at" @@ -43093,7 +43098,7 @@ fn delete_paths_sorted( if paths[i].len() == start + 1 { del_keys.push(key); } else { - value = delete_paths_under::(value, key, &paths[i..j], start + 1, yq_mode)?; + value = delete_paths_under::(value, key, &paths[i..j], start + 1)?; } i = j; } @@ -43107,8 +43112,8 @@ fn delete_paths_under( key: &OwnedValue, paths: &[&[OwnedValue]], start: usize, - yq_mode: bool, ) -> Result { + let yq_mode = S::TAG == EvalTag::Yq; match value { OwnedValue::Object(mut entries) => match key { OwnedValue::String(name) => { @@ -43117,7 +43122,7 @@ fn delete_paths_under( // jq leaves an existing key where it was, and `IndexMap` // would move it to the end after a `shift_remove`. let old = core::mem::replace(slot, OwnedValue::Null); - *slot = delete_paths_sorted::(old, paths, start, yq_mode)?; + *slot = delete_paths_sorted::(old, paths, start)?; } Ok(OwnedValue::Object(entries)) } @@ -43142,9 +43147,7 @@ fn delete_paths_under( OwnedValue::Object(desc) => { let range = SliceBounds::from_descriptor(desc)?.resolve(arr.len()); let sub = OwnedValue::Array(arr[range.clone()].to_vec()); - let OwnedValue::Array(items) = - delete_paths_sorted::(sub, paths, start, yq_mode)? - else { + let OwnedValue::Array(items) = delete_paths_sorted::(sub, paths, start)? else { unreachable!("deleting from an array yields an array") }; arr.splice(range, items); @@ -43171,12 +43174,11 @@ fn delete_paths_under( match resolve_delete_index(key, arr.len()) { DeleteIndexResolution::InRange(index) => { let old = core::mem::replace(&mut arr[index], OwnedValue::Null); - arr[index] = delete_paths_sorted::(old, paths, start, yq_mode)?; + arr[index] = delete_paths_sorted::(old, paths, start)?; } DeleteIndexResolution::PositiveOutOfRange(index) if yq_mode => { pad_with_nulls(&mut arr, index)?; - arr[index] = - delete_paths_sorted::(OwnedValue::Null, paths, start, yq_mode)?; + arr[index] = delete_paths_sorted::(OwnedValue::Null, paths, start)?; } DeleteIndexResolution::PositiveOutOfRange(_) | DeleteIndexResolution::Skip => {} } @@ -49277,7 +49279,7 @@ fn delpaths_one<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( root_deleted = true; OwnedValue::Null } else { - delete_paths_sorted::(v, core::slice::from_ref(&path), 0, true)? + delete_paths_sorted::(v, core::slice::from_ref(&path), 0)? }; } Ok(v) @@ -49291,7 +49293,7 @@ fn delpaths_one<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>( None => to_owned(value), Some([]) => Ok(OwnedValue::Null), // STYLE-0012: see the note above `root_deleted`. - Some(_) => to_owned(value).and_then(|v| delete_paths_sorted::(v, &paths, 0, false)), + Some(_) => to_owned(value).and_then(|v| delete_paths_sorted::(v, &paths, 0)), } }; match result { diff --git a/src/jq/eval_generic.rs b/src/jq/eval_generic.rs index 0b97e4e88..81aae9e44 100644 --- a/src/jq/eval_generic.rs +++ b/src/jq/eval_generic.rs @@ -1728,9 +1728,12 @@ fn materialize_lazy_keys( ) -> Result { let mut keys = effective_key_values(fields, collapse)?; if sorted { - // Object keys are always strings, so the mode-forked numeric arms - // of `compare_values` (#2906) are unreachable here and no `S` is - // threaded through this streaming entry point. + // `sorted` is only ever `true` in jq mode: the yq parser lowers + // `keys` to `Builtin::KeysUnsorted` (real yq returns keys in + // document order), so this streaming entry point -- which has no + // `S` of its own -- never sorts under yq semantics, and jq's number + // model (#2906) is the only one it can need. (YAML keys are typed, + // so "keys are always strings" would be the wrong reason.) keys.sort_by(compare_values::); } Ok(OwnedValue::Array(keys)) diff --git a/src/jq/value.rs b/src/jq/value.rs index afe231f0f..efcb5a881 100644 --- a/src/jq/value.rs +++ b/src/jq/value.rs @@ -218,13 +218,14 @@ pub(crate) fn jq_literal_int_to_f64(n: i64) -> f64 { } } -/// Widen an `Int` operand to `f64` under `S`'s number model: jq's -/// literal rounding ([`jq_literal_int_to_f64`]) when -/// `EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS`, otherwise the plain -/// cast real yq's `int64`-to-`float64` conversion amounts to. +/// Widen an `Int` operand to `f64` for *arithmetic* under `S`'s number +/// model: jq's literal rounding ([`jq_literal_int_to_f64`]) when +/// `EvalSemantics::DECNUMBER_LITERALS`, otherwise the plain cast real yq's +/// `int64`-to-`float64` conversion amounts to. (jq-mode comparison has its +/// own rule, [`jq_numeric_cmp`]: two literals never widen at all.) #[inline] pub(crate) fn int_to_f64(n: i64) -> f64 { - if S::INT_LITERAL_ROUNDS_TO_17_DIGITS { + if S::DECNUMBER_LITERALS { jq_literal_int_to_f64(n) } else { n as f64 @@ -2718,18 +2719,19 @@ pub(crate) fn is_nan_sentinel(bytes: &[u8]) -> bool { /// - Objects compare order-insensitively (`IndexMap`'s own `PartialEq`), as in jq. /// /// Known divergence: above 2^53 a mixed `Int`/`Float` comparison widens the -/// integer to `f64`, whereas jq 1.7 compares two *literals* exactly as -/// decimals. So `9007199254740993 == 9007199254740992.0` is `true` here and -/// `false` in jq. Every value representable exactly as an `f64` agrees. The -/// widening itself follows jq's own literal-to-double rule -/// ([`jq_literal_int_to_f64`], #2906), so an `Int` literal against a -/// *computed* `Float` -- `869389897822472004 == (869389897822472004 + 0)` -/// -- agrees with jq. +/// integer to `f64` with a plain cast, whereas jq 1.7 compares two +/// *literals* exactly as decimals and a literal against a computed double +/// through its 17-digit rounding. So `9007199254740993 == 9007199254740992.0` +/// is `true` here and `false` in jq. Every value representable exactly as an +/// `f64` agrees. /// -/// This is jq's rule only, and needs no `EvalSemantics`: yq's own equality -/// (`STRICT_NUMERIC_EQUALITY`) never widens a mixed pair at all -- -/// [`owned_value_eq`] routes every yq-mode numeric pair to -/// [`numeric_repr_eq_strict`] before this can run. +/// This mode-blind widening is the rule behind `OwnedValue`'s `PartialEq`, +/// which the yq presentation layer also relies on (`align_by_value` in the +/// CLI matches a yq-mode `Int` against the plain-cast double yq's own +/// arithmetic produced, and `owned_value_align_hash` must agree with it). +/// jq-mode `==` does **not** stop here: [`owned_value_eq`] routes a jq-mode +/// numeric pair through [`jq_numeric_cmp`] (#2906), and a yq-mode one +/// through [`numeric_repr_eq_strict`]. /// /// `NumberLiteral` compares purely on its parsed [`NumberRepr`], never on the /// source text -- two spellings of the same number (`1.0` and `1e0`) are @@ -2738,8 +2740,8 @@ pub(crate) fn numeric_repr_eq(a: NumberRepr, b: NumberRepr) -> bool { match (a, b) { (NumberRepr::Int(a), NumberRepr::Int(b)) => a == b, (NumberRepr::Float(a), NumberRepr::Float(b)) => a == b, - (NumberRepr::Int(a), NumberRepr::Float(b)) => jq_literal_int_to_f64(a) == b, - (NumberRepr::Float(a), NumberRepr::Int(b)) => a == jq_literal_int_to_f64(b), + (NumberRepr::Int(a), NumberRepr::Float(b)) => (a as f64) == b, + (NumberRepr::Float(a), NumberRepr::Int(b)) => a == (b as f64), } } @@ -2797,15 +2799,20 @@ fn owned_value_eq_at_depth_generic( } // Every other pairing (Null/Bool/String, Int/Float/NumberLiteral, // and any type mismatch) has no further nesting to thread through. - // Only checked only when *both* operands are numeric under strict - // mode; a number-vs-non-number comparison (already `false` either - // way) and every jq-mode comparison fall straight through to the - // ordinary widening `PartialEq`. + // A numeric pair takes the mode's own number model -- yq's strict + // never-widen rule, or jq's decNumber rule (#2906: two literals + // compare exactly, a literal against a computed double through the + // 17-digit rounding); a number-vs-non-number comparison (already + // `false` either way) and everything non-numeric fall straight + // through to the ordinary `PartialEq`. _ => { - if S::STRICT_NUMERIC_EQUALITY { - if let (Some(x), Some(y)) = (a.number_repr(), b.number_repr()) { + if let (Some(x), Some(y)) = (a.number_repr(), b.number_repr()) { + if S::STRICT_NUMERIC_EQUALITY { return numeric_repr_eq_strict(x, y); } + if S::DECNUMBER_LITERALS { + return jq_numeric_cmp(a, b) == Some(core::cmp::Ordering::Equal); + } } a == b } @@ -2862,20 +2869,209 @@ pub(crate) fn cmp_f64(a: f64, b: f64) -> core::cmp::Ordering { /// /// The NaN rule (#421) is centralized in [`cmp_f64`], not repeated here. /// -/// Unlike [`numeric_repr_eq`], ordering runs in both modes (`sort`, `min`, -/// `max`, `<` all reach it under yq too), so the mixed-pair widening is -/// mode-forked through [`int_to_f64`]: jq's literal rounding -/// ([`jq_literal_int_to_f64`], #2906) in jq mode, the plain cast in yq mode. -pub(crate) fn numeric_repr_cmp( - a: NumberRepr, - b: NumberRepr, -) -> core::cmp::Ordering { +/// Like [`numeric_repr_eq`], this is the mode-blind plain widening: yq-mode +/// ordering uses it as is, while jq-mode ordering (`compare_values`) uses +/// [`jq_numeric_cmp`] for any pair that is not `Int`/`Int` or +/// `Float`/`Float` (#2906). +pub(crate) fn numeric_repr_cmp(a: NumberRepr, b: NumberRepr) -> core::cmp::Ordering { match (a, b) { (NumberRepr::Int(a), NumberRepr::Int(b)) => a.cmp(&b), (NumberRepr::Float(a), NumberRepr::Float(b)) => cmp_f64(a, b), - (NumberRepr::Int(a), NumberRepr::Float(b)) => cmp_f64(int_to_f64::(a), b), - (NumberRepr::Float(a), NumberRepr::Int(b)) => cmp_f64(a, int_to_f64::(b)), + (NumberRepr::Int(a), NumberRepr::Float(b)) => cmp_f64(a as f64, b), + (NumberRepr::Float(a), NumberRepr::Int(b)) => cmp_f64(a, b as f64), + } +} + +/// Compare two number literals exactly, by their decimal digits, the way +/// jq's `jvp_number_cmp` compares two decNumber literals +/// (`decNumberCompare`, #2906): neither side is converted to `f64`, so +/// `869389897822472004` vs `869389897822472004.0` is `Equal` and +/// `869389897822472004` vs `869389897822471936.5` is `Greater`, whatever +/// doubles those spellings round to. Accepts every spelling the literal +/// paths store (RFC 8259 numbers plus #1171's `.5`/`-.5` leniency); `-0` +/// and `0` are equal. +pub(crate) fn cmp_decimal_literals(a: &str, b: &str) -> core::cmp::Ordering { + use core::cmp::Ordering; + let (a_negative, a_digits, a_exponent) = decompose_decimal_literal(a); + let (b_negative, b_digits, b_exponent) = decompose_decimal_literal(b); + let sign = |negative: bool, digits: &[u8]| -> i8 { + if digits.is_empty() { + 0 + } else if negative { + -1 + } else { + 1 + } + }; + let (a_sign, b_sign) = (sign(a_negative, &a_digits), sign(b_negative, &b_digits)); + if a_sign != b_sign { + return a_sign.cmp(&b_sign); + } + if a_sign == 0 { + return Ordering::Equal; + } + // Same nonzero sign: the magnitude with its leading digit further left + // is larger; at the same position, the digit strings (no leading or + // trailing zeros) order lexicographically, a proper prefix being smaller. + let a_lead = a_digits.len() as i128 + a_exponent; + let b_lead = b_digits.len() as i128 + b_exponent; + let magnitude = a_lead.cmp(&b_lead).then_with(|| a_digits.cmp(&b_digits)); + if a_negative { + magnitude.reverse() + } else { + magnitude + } +} + +/// `(negative, significant digits, exponent)` with value +/// `digits * 10^exponent`, the digits stripped of leading and trailing +/// zeros (empty means zero). Parsing stops at the first byte that is not +/// part of a number, so a stray suffix cannot panic. +fn decompose_decimal_literal(text: &str) -> (bool, Vec, i128) { + let bytes = text.as_bytes(); + let mut i = 0; + let negative = match bytes.first() { + Some(b'-') => { + i = 1; + true + } + Some(b'+') => { + i = 1; + false + } + _ => false, + }; + let mut digits = Vec::new(); + let mut exponent: i128 = 0; + while i < bytes.len() && bytes[i].is_ascii_digit() { + digits.push(bytes[i]); + i += 1; + } + if i < bytes.len() && bytes[i] == b'.' { + i += 1; + while i < bytes.len() && bytes[i].is_ascii_digit() { + digits.push(bytes[i]); + exponent -= 1; + i += 1; + } + } + if i < bytes.len() && (bytes[i] == b'e' || bytes[i] == b'E') { + i += 1; + let exponent_negative = match bytes.get(i) { + Some(b'-') => { + i += 1; + true + } + Some(b'+') => { + i += 1; + false + } + _ => false, + }; + let mut e: i128 = 0; + while i < bytes.len() && bytes[i].is_ascii_digit() { + // Saturate far beyond any exponent a literal can hold, so an + // absurdly long exponent still orders correctly without + // overflowing the arithmetic above. + e = (e * 10 + i128::from(bytes[i] - b'0')).min(1 << 62); + i += 1; + } + exponent += if exponent_negative { -e } else { e }; + } + let leading_zeros = digits.iter().take_while(|d| **d == b'0').count(); + digits.drain(..leading_zeros); + while digits.last() == Some(&b'0') { + digits.pop(); + exponent += 1; + } + (negative, digits, exponent) +} + +/// jq's own number comparison (`jvp_number_cmp`, `src/jv.c`), behind +/// jq-mode `==` and ordering (#2906): two *literals* compare exactly as +/// decimals (`decNumberCompare`), while any pair involving a *computed* +/// double compares as doubles, a literal converted through the same +/// 17-digit rounding arithmetic uses ([`jq_literal_int_to_f64`]). `None` +/// unless both operands are numbers. +/// +/// A `NumberLiteral` (either repr) and a bare `Int` are literals (the +/// invariant [`jq_literal_int_to_f64`] documents); a bare `Float` is +/// computed. Two literals are ordered by their correctly-rounded doubles +/// first -- rounding is monotonic, so a strict double order *is* the exact +/// order -- and only a double tie falls through to the digit comparison, +/// so sorting document floats pays for text only when two values' doubles +/// coincide (byte-equal spellings short-circuit). A `Float` literal with +/// more than 17 significant digits is still widened by its parsed double +/// rather than jq's rounded one (#2936). +pub(crate) fn jq_numeric_cmp(left: &OwnedValue, right: &OwnedValue) -> Option { + use core::cmp::Ordering; + + struct Literal<'a> { + double: f64, + int: Option, + text: Option<&'a str>, + } + enum Side<'a> { + Literal(Literal<'a>), + Computed(f64), + } + fn side(value: &OwnedValue) -> Option> { + Some(match value { + OwnedValue::Int(n) => Side::Literal(Literal { + double: *n as f64, + int: Some(*n), + text: None, + }), + OwnedValue::NumberLiteral(NumberRepr::Int(n), text) => Side::Literal(Literal { + double: *n as f64, + int: Some(*n), + text: Some(text), + }), + OwnedValue::NumberLiteral(NumberRepr::Float(f), text) => Side::Literal(Literal { + double: *f, + int: None, + text: Some(text), + }), + OwnedValue::Float(f) => Side::Computed(*f), + _ => return None, + }) + } + fn to_double(literal: &Literal<'_>) -> f64 { + match literal.int { + Some(n) => jq_literal_int_to_f64(n), + None => literal.double, + } + } + fn text<'a>(literal: &'a Literal<'a>) -> Cow<'a, str> { + match (literal.text, literal.int) { + (Some(text), _) => Cow::Borrowed(text), + (None, Some(n)) => Cow::Owned(n.to_string()), + (None, None) => unreachable!("a literal without text always carries an i64"), + } + } + fn cmp_literals(a: &Literal<'_>, b: &Literal<'_>) -> Ordering { + if let (Some(x), Some(y)) = (a.int, b.int) { + return x.cmp(&y); + } + match cmp_f64(a.double, b.double) { + Ordering::Equal => { + let (a_text, b_text) = (text(a), text(b)); + if a_text == b_text { + Ordering::Equal + } else { + cmp_decimal_literals(&a_text, &b_text) + } + } + strict => strict, + } } + + Some(match (side(left)?, side(right)?) { + (Side::Literal(a), Side::Literal(b)) => cmp_literals(&a, &b), + (Side::Literal(a), Side::Computed(b)) => cmp_f64(to_double(&a), b), + (Side::Computed(a), Side::Literal(b)) => cmp_f64(a, to_double(&b)), + (Side::Computed(a), Side::Computed(b)) => cmp_f64(a, b), + }) } impl OwnedValue { @@ -3599,9 +3795,11 @@ mod tests { OwnedValue::from_number_literal("9007199254740993"), OwnedValue::Float(9007199254740992.0), ), - // #2906: an 18-digit `Int` literal against the double jq's own - // literal rounding produces for it (not the exact integer's - // nearest double, `869389897822472064.0`). + // An 18-digit `Int` literal against the double jq's own literal + // rounding produces for it: *unequal* under the mode-blind plain + // widening (`PartialEq`, `numeric_repr_cmp`), which is the pair + // both must agree on; jq-mode `==` answers separately through + // `jq_numeric_cmp` (#2906, tested below). ( OwnedValue::from_number_literal("869389897822472004"), OwnedValue::Float(869389897822471936.0), @@ -3617,13 +3815,120 @@ mod tests { "numeric_repr_eq disagrees with OwnedValue::eq for {a:?} vs {b:?}" ); assert_eq!( - numeric_repr_cmp::(ra, rb) == core::cmp::Ordering::Equal, + numeric_repr_cmp(ra, rb) == core::cmp::Ordering::Equal, are_eq, "numeric_repr_cmp disagrees with OwnedValue::eq for {a:?} vs {b:?}" ); } } + /// #2906: `cmp_decimal_literals` is jq's `decNumberCompare` on two + /// literal spellings -- exact, so doubles never enter into it. + #[test] + fn test_cmp_decimal_literals_is_exact_2906() { + use core::cmp::Ordering::{Equal, Greater, Less}; + for (a, b, want) in [ + ("869389897822472004", "869389897822472004.0", Equal), + ("869389897822472004", "869389897822471936.5", Greater), + ("869389897822472004", "869389897822472000.5", Greater), + ("869389897822472004", "869389897822472004.5", Less), + ("9007199254740993", "9007199254740992.0", Greater), + ("1.00000000000000001", "1.0", Greater), + ("1.0", "1", Equal), + ("1.50", "1.5", Equal), + ("1e2", "100", Equal), + ("1E+2", "99.999999999999999999", Greater), + ("0.1", "1e-1", Equal), + (".5", "0.5", Equal), + ("-.5", "-0.5", Equal), + ("-0", "0", Equal), + ("0.0", "-0.0e5", Equal), + ("-1", "1", Less), + ("-2", "-10", Greater), + ("-2.5", "-2.25", Less), + ("12", "123", Less), + ("123", "12", Greater), + ("1e999", "1e1000", Less), + ("1e-400", "0", Greater), + ("-1e-400", "0", Less), + ("1e999999999999999999999", "2e999999999999999999999", Less), + ] { + assert_eq!(cmp_decimal_literals(a, b), want, "{a} vs {b}"); + assert_eq!(cmp_decimal_literals(b, a), want.reverse(), "{b} vs {a}"); + } + } + + /// #2906: jq-mode number comparison -- two literals exactly, a literal + /// against a computed double through the 17-digit rounding. Every + /// expectation was captured live against `/usr/bin/jq` 1.7.1. + #[test] + fn test_jq_numeric_cmp_literal_pairs_exact_and_computed_pairs_rounded_2906() { + use core::cmp::Ordering::{Equal, Greater, Less}; + let lit = OwnedValue::from_number_literal; + let computed = |f: f64| OwnedValue::Float(f); + // `869389897822472004 + 0` in jq: the literal rounds to ...000, whose + // nearest double is ...1936. + let sum = computed(869389897822471936.0); + for (a, b, want) in [ + // literal vs computed: widen through the literal rounding + (lit("869389897822472004"), sum.clone(), Equal), + (OwnedValue::Int(869389897822472004), sum.clone(), Equal), + (lit("869389897822472000"), sum.clone(), Equal), + (lit("869389897822472100"), sum.clone(), Greater), + (lit("869389897822471936.0"), sum.clone(), Equal), + // literal vs literal: exact decimals, no rounding on either side + ( + lit("869389897822472004"), + lit("869389897822471936.0"), + Greater, + ), + ( + lit("869389897822472004"), + lit("869389897822472004.0"), + Equal, + ), + ( + lit("869389897822472004"), + lit("869389897822471936.5"), + Greater, + ), + ( + lit("869389897822472004"), + lit("869389897822472000.5"), + Greater, + ), + (lit("869389897822472004"), lit("869389897822472064.0"), Less), + ( + OwnedValue::Int(869389897822472004), + lit("869389897822472004.0"), + Equal, + ), + (lit("9007199254740993"), lit("9007199254740992.0"), Greater), + (lit("1.00000000000000001"), lit("1.0"), Greater), + (lit("1.50"), lit("1.5"), Equal), + (lit("1e999"), lit("1e1000"), Less), + (OwnedValue::Int(3), lit("3.0"), Equal), + (OwnedValue::Int(3), OwnedValue::Int(4), Less), + // computed vs computed, and the NaN rule + (computed(1.5), computed(1.5), Equal), + (computed(f64::NAN), computed(f64::NAN), Less), + (lit("1"), computed(f64::NAN), Greater), + (computed(-0.0), lit("0"), Equal), + ] { + assert_eq!(jq_numeric_cmp(&a, &b), Some(want), "{a:?} vs {b:?}"); + assert_eq!( + owned_value_eq::(&a, &b), + want == Equal, + "jq-mode == disagrees with jq_numeric_cmp for {a:?} vs {b:?}" + ); + } + assert_eq!(jq_numeric_cmp(&OwnedValue::Null, &OwnedValue::Int(1)), None); + assert_eq!( + jq_numeric_cmp(&lit("1"), &OwnedValue::String("1".into())), + None + ); + } + /// #2906: jq converts an integer literal to a double by first rounding /// its decimal digits to 17 significant digits (half-even), then /// rounding that to the nearest double. Every expectation here is the @@ -3675,45 +3980,17 @@ mod tests { } } - /// Hand-round the decimal spelling of `n` to 17 significant digits - /// (half-even) and parse the result -- an independent, string-based - /// route to the same double [`jq_literal_int_to_f64`] computes with - /// integer arithmetic, mirroring jq's own `decNumberToString` + - /// `strtod` shape. + /// Round the decimal spelling of `n` to 17 significant digits and parse + /// the result -- an independent, string-based route to the same double + /// [`jq_literal_int_to_f64`] computes with integer arithmetic, mirroring + /// jq's own `decNumberToString` + `strtod` shape. `core`'s `{:.16e}` + /// formatter rounds an integer's coefficient half-even, exactly the + /// decNumber rule. fn round_decimal_string_to_17_digits(n: i64) -> f64 { - let digits: Vec = n.unsigned_abs().to_string().into_bytes(); - if digits.len() <= 17 { - return n as f64; - } - let (kept, dropped) = digits.split_at(17); - let mut kept: Vec = kept.to_vec(); - let first_dropped = dropped[0]; - let rest_nonzero = dropped[1..].iter().any(|d| *d != b'0'); - let last_kept_odd = (kept[16] - b'0') % 2 == 1; - let round_up = - first_dropped > b'5' || (first_dropped == b'5' && (rest_nonzero || last_kept_odd)); - let mut exponent = dropped.len(); - if round_up { - let mut i = 16; - loop { - if kept[i] == b'9' { - kept[i] = b'0'; - if i == 0 { - kept.insert(0, b'1'); - kept.pop(); - exponent += 1; - break; - } - i -= 1; - } else { - kept[i] += 1; - break; - } - } - } let sign = if n < 0 { "-" } else { "" }; - let text = format!("{sign}{}e{exponent}", String::from_utf8(kept).unwrap()); - text.parse::().unwrap() + format!("{sign}{:.16e}", n.unsigned_abs()) + .parse::() + .unwrap() } /// #2906: property check of [`jq_literal_int_to_f64`] over seeded @@ -3723,20 +4000,15 @@ mod tests { /// on, monotonicity, and yq mode's untouched plain cast. #[test] fn test_jq_literal_int_to_f64_properties_2906() { - // xorshift64*, seeded; no external crate needed here. - let mut state: u64 = 0x2906_2906_2906_2906; - let mut next = || { - state ^= state >> 12; - state ^= state << 25; - state ^= state >> 27; - state.wrapping_mul(0x2545_F491_4F6C_DD1D) - }; + use rand::{Rng, SeedableRng}; + use rand_chacha::ChaCha8Rng; + let mut rng = ChaCha8Rng::seed_from_u64(0x2906_2906_2906_2906); let mut previous: Option<(i64, f64)> = None; for i in 0..20_000 { // Spread the samples across the 18/19-digit band and below it, // rather than letting a uniform u64 land past 10^18 nearly every // time. - let raw = next(); + let raw = rng.next_u64(); let n = match i % 4 { 0 => (raw % 1_000_000_000_000_000_000) as i64, 1 => (raw % 100_000_000_000_000_000) as i64 + 100_000_000_000_000_000, diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index 59e4af503..d39ab68ce 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -53728,22 +53728,23 @@ fn test_large_int_literal_rounds_to_17_digits_before_f64_arith_2906() -> Result< Ok(()) } -/// #2906: the same literal-to-double rule applies when an `Int` is widened -/// for `==`/`!=` and the ordering operators, so a literal compares equal to -/// the double jq's own arithmetic produced from it. Two literals still -/// compare exactly (`869389897822472004 == 869389897822472000` stays -/// `false`, as in jq's `decNumberCompare`). Captured live against -/// `/usr/bin/jq` 1.7.1. -/// -/// Only the binary operators are pinned here. `sort`/`unique`/`min`/`max` -/// on an array built in the filter cross the reindex bridge, which -/// re-parses the computed double `869389897822472000` as an *integer* -/// literal, and from then on it compares exactly against the real literal -/// (`[869389897822472004, (869389897822472000+0), 5] | sort` is -/// `[5,869389897822472000,869389897822472004]` here and -/// `[5,869389897822472004,869389897822472000]` in jq -- identical before -/// and after #2906, a separate bridge gap tracked as #2938 and recorded in -/// `docs/compliance/jq/limitations.md`). +/// #2906: jq's comparison (`jvp_number_cmp`) has two rules. A literal +/// against a *computed* double widens the literal through the same +/// 17-digit rounding arithmetic uses, so a literal compares equal to the +/// double jq's own arithmetic produced from it. Two *literals* compare +/// exactly as decimals (`decNumberCompare`) -- no rounding on either side, +/// so `869389897822472004` is unequal to the float literal `...1936.0` +/// (its own rounded double) but equal to `...2004.0`, whatever double that +/// spelling parses to. Captured live against `/usr/bin/jq` 1.7.1. +/// +/// Only the binary operators are pinned for the literal-vs-computed rule. +/// `sort`/`unique`/`min`/`max` on an array built in the filter cross the +/// reindex bridge, which re-parses the computed double `869389897822472000` +/// as an *integer* literal, and from then on it compares exactly against +/// the real literal -- identical before and after #2906, a separate bridge +/// gap tracked as #2938 (see `..._characterize_preexisting_bug_2906`). +/// Literal-vs-literal pairs survive the bridge unchanged, so they are +/// pinned through the ordering builtins too. #[test] fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<()> { for (filter, want) in [ @@ -53764,11 +53765,117 @@ fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<() "[(869389897822472000+0) < 869389897822472004, 869389897822472004 < 869389897822472100]", "[false,true]", ), + // Two literals: exact decimals. `...1936.0` is the literal's own + // rounded double and `...2064.0` is its nearest double; neither is + // equal to it, while `...2004.0` is. + ("869389897822472004 == 869389897822471936.0", "false"), + ("869389897822472004 == 869389897822472064.0", "false"), + ("869389897822472004 == 869389897822472004.0", "true"), + ("869389897822472004 > 869389897822471936.5", "true"), + ("869389897822472004 < 869389897822472000.5", "false"), + ("9007199254740993 == 9007199254740992.0", "false"), + ("1.00000000000000001 == 1.0", "false"), + ("0.1 == 0.10", "true"), + ("1e999 < 1e1000", "true"), + ( + "[869389897822472004, 869389897822471936.5] | sort", + "[869389897822471936.5,869389897822472004]", + ), + ( + "[869389897822472004.0, 869389897822472004] | unique", + "[869389897822472004.0]", + ), + ( + "[869389897822472004, 869389897822471936.0] | unique", + "[869389897822471936.0,869389897822472004]", + ), + ("[1.50, 1.5] | unique", "[1.50]"), + ("[869389897822472004] | bsearch(869389897822471936.0)", "-1"), + ("[869389897822472004.0] | index(869389897822472004)", "0"), + ("869389897822471936.0 | IN(869389897822472004)", "false"), + ( + "[869389897822472004, 869389897822471936.0] | group_by(.) | length", + "2", + ), + ( + "[869389897822472004, 869389897822471936.5, (869389897822472000+0)] | min", + "869389897822471936.5", + ), ] { let (output, code) = run_jq_null(filter, &["-c"])?; assert_eq!(code, 0, "`{filter}`"); assert_eq!(output.trim(), want, "`{filter}`"); } + + // The same literal pairs arriving as document input, through the + // generic evaluator. + let (output, code) = run_jq_stdin( + "[.a == .b, .a == .c, .a > .c, .a > .d, ([.a, .d] | sort)]", + r#"{"a":869389897822472004,"b":869389897822472004.0,"c":869389897822471936.0,"d":869389897822471936.5}"#, + &["-c"], + )?; + assert_eq!(code, 0); + assert_eq!( + output.trim(), + "[true,false,true,true,[869389897822471936.5,869389897822472004]]" + ); + Ok(()) +} + +/// #2906 left three same-mechanism gaps open (tracked as #2936, #2937 and +/// #2938); this pins their *current* answers so a later change cannot move +/// them unnoticed, and so the eventual fixes have a test to flip. Each +/// row's jq 1.7.1 answer is in the comment; succinctly's output was +/// verified identical before and after #2906. +#[test] +fn test_large_int_literal_gaps_characterize_preexisting_bug_2906() -> Result<()> { + for (filter, current, jq_says) in [ + // #2938: the reindex bridge re-parses the computed double + // `869389897822472000` as an integer literal, which then compares + // exactly (`000 < 004`) instead of equal to the literal. + ( + "[869389897822472004, (869389897822472000+0), 5] | sort", + "[5,869389897822472000,869389897822472004]", + "[5,869389897822472004,869389897822472000]", + ), + ( + "[5, 869389897822472004, (869389897822472000+0)] | max", + "869389897822472004", + "869389897822472000", + ), + // #2936: a >17-digit float literal is parsed with one correct + // rounding, not jq's 17-digit pre-rounding. + ( + "2.7293109604053567083 + 0", + "2.729310960405357", + "2.7293109604053565", + ), + // #2937: the math builtins widen with a bare cast, and the floor + // family (and `length`) print exact digits where jq has a double. + ( + "869389897822472004 | sqrt", + "932410798.8555647", + "932410798.8555645", + ), + ( + "869389897822472004 | floor", + "869389897822472064", + "869389897822472000", + ), + ( + "869389897822472004 | length", + "869389897822472004", + "869389897822472000", + ), + ] { + let (output, code) = run_jq_null(filter, &["-c"])?; + assert_eq!(code, 0, "`{filter}`"); + assert_eq!( + output.trim(), + current, + "`{filter}` moved -- if it now prints {jq_says} (jq's answer), update this row and close the tracked issue" + ); + } Ok(()) } diff --git a/tests/jq_evaluator_parity_tests.rs b/tests/jq_evaluator_parity_tests.rs index 1d9f86dd4..60eaa8daa 100644 --- a/tests/jq_evaluator_parity_tests.rs +++ b/tests/jq_evaluator_parity_tests.rs @@ -807,22 +807,22 @@ fn test_parity_number_literal_reaches_numeric_arg_builtins_387() { fn test_parity_number_literal_ordering_agrees_with_equality_387() { // `compare_values`'s first cut at a `NumberLiteral` ordering arm tried an // exact `i64` comparison before falling back to `f64`, while `==` - // (`OwnedValue::PartialEq`) always widens a mixed pair to `f64`. Above + // (`OwnedValue::PartialEq`) always widened a mixed pair to `f64`. Above // 2^53 the two representations of "the same number" disagree about // whether an `i64` round-trips through `f64` exactly, so `==` and `>` // could both report `true` for the same pair -- e.g. `sort`/`unique` // disagreeing with `==` about whether two values are the same number. - // This is an internal-consistency property, not a jq-parity one: this - // crate already documents (`OwnedValue`'s `PartialEq` doc comment) that it - // widens to `f64` here where jq 1.7 keeps full decimal precision, so `==` - // itself already diverges from jq for this pair -- what must not diverge - // is `==` from `>`/`<`/`sort` about the *same* values. + // The property is internal consistency: `==` must never diverge from + // `>`/`<`/`sort` about the *same* values. Since #2906 both go through + // jq's own rule (`jq_numeric_cmp`: two literals compare exactly as + // decimals), so the answers below are also jq 1.7.1's own -- captured + // live: `false`, `true`, `false`. let json = br"[9007199254740993, 9007199254740992.0]"; for filter in [".[0] == .[1]", ".[0] > .[1]", ".[0] < .[1]"] { assert_parity(json, filter); } - assert_eq!(as_strs(&full_outputs(json, ".[0] == .[1]")), ["true"]); - assert_eq!(as_strs(&full_outputs(json, ".[0] > .[1]")), ["false"]); + assert_eq!(as_strs(&full_outputs(json, ".[0] == .[1]")), ["false"]); + assert_eq!(as_strs(&full_outputs(json, ".[0] > .[1]")), ["true"]); assert_eq!(as_strs(&full_outputs(json, ".[0] < .[1]")), ["false"]); } From cce458961f80ef0a7bfdbba3c7c1e0862f41a6a9 Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 14 Sep 2026 13:14:22 +1000 Subject: [PATCH 4/5] fix(jq): address /code-review findings on #2906 Five findings from a high-effort /code-review of the #2906 branch: - jq_numeric_cmp now fast-paths any Int/NumberLiteral(Int) pair to a plain i64 comparison before building a Literal/Side for either operand -- the common case for sort/unique/min/max/bsearch over ordinary parsed-JSON integer arrays, which previously always paid for the full decNumber-literal machinery even though it always resolved to the same x.cmp(&y). - decompose_decimal_literal now reuses strip_leading_sign and parse_literal_exponent/ExpParse instead of a third independent sign-stripping and exponent-saturation implementation, matching the precedent #106/#1304 set for exactly this duplication pattern. - The x86_64 dtoi/INT64_MIN claim in limitations.md and the matching eval.rs comment is reworded as unverified (no x86_64 jq 1.7.1 was available to capture it live), instead of stated as fact. - materialize_lazy_keys's hardcoded compare_values:: now cites the actual parse-time enforcement (try_parse_builtin's ParserMode::Yq check) it relies on, rather than an unexplained comment; threading S through the whole streaming lazy-materialization pipeline for an already parse-time-enforced invariant wasn't worth the additional plumbing. - arith_mod's float-truncation gate is reordered so its condition is a single negation matching its own comment, instead of needing De Morgan's law to map one to the other. --- docs/compliance/jq/limitations.md | 10 +++-- src/jq/eval.rs | 21 ++++----- src/jq/eval_generic.rs | 21 ++++++--- src/jq/value.rs | 74 ++++++++++++++++++------------- 4 files changed, 75 insertions(+), 51 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 9ffc90ddb..47604e752 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -5835,9 +5835,13 @@ is otherwise untouched: real yq's `int64` arithmetic is exact (`8693898978224720 plain cast (`EvalSemantics::DECNUMBER_LITERALS`). Two things #2906 makes *visible* without changing: jq's `dtoi` on exactly `2^63` is a C -cast that saturates on the arm64 oracle (`as i64` does the same) but is undefined -behaviour and yields `INT64_MIN` on x86_64 jq, so `9223372036854775807 % 10` is `7` on the -pin and `-8` there; and jq parses `-x % y` as `-(x % y)` (unary minus binds looser than +cast that saturates on the arm64 oracle (`as i64` does the same), so +`9223372036854775807 % 10` is `7` on the pin -- but the cast is undefined behaviour in C, +and **the x86_64 case is not live-verified**: no x86_64 jq 1.7.1 was available to capture +this session, so whether it saturates the same way there, yields `INT64_MIN`, or differs by +compiler/libc is an open question, not a claim to build on (flag for whoever closes +#2936/#2937/#2938 with x86_64 hardware in reach); and jq parses `-x % y` as `-(x % y)` +(unary minus binds looser than `%`) where succinctly parses `(-x) % y`, a pre-existing precedence gap that only shows at this magnitude (`-9223372036854775807 % 10` is `-7` in jq, `-8` here; the parenthesised and data-sourced spellings agree on `-8`). diff --git a/src/jq/eval.rs b/src/jq/eval.rs index cf1902414..de4a6ed47 100644 --- a/src/jq/eval.rs +++ b/src/jq/eval.rs @@ -9600,23 +9600,24 @@ fn arith_mod( } else { Err(EvalError::divisor_is_zero(&left, &right, BinOp::Modulo)) } - } else if !S::MOD_TRUNCATES_FLOATS - || (jq_int_within_exact_f64_range(a) && jq_int_within_exact_f64_range(b)) + } else if S::MOD_TRUNCATES_FLOATS + && !(jq_int_within_exact_f64_range(a) && jq_int_within_exact_f64_range(b)) { + // jq (#2906): `binop_mod` is `dtoi(a) % dtoi(b)` on the + // operands' *doubles* -- a literal's 17-digit-rounded one -- + // with the remainder held as a double again, which is + // exactly `mod_floats`' truncating model. (`dtoi` saturates + // like `as i64` on the arm64 oracle; a C cast of exactly + // 2^63 is undefined behaviour, so this is unverified on + // x86_64 jq -- not live-tested there.) + mod_floats::(int_to_f64::(a), int_to_f64::(b), &left, &right) + } else { // yq: exact int64 remainder. jq: the same whenever both // operands are exact doubles -- the literal rounding is the // identity there, the `intmax_t` truncation round-trips, and // `|a % b| < |b| <= 2^53` keeps the result an exact `Int` // (#2906). `wrapping_rem`: `i64::MIN % -1` must not panic. Ok(OwnedValue::Int(a.wrapping_rem(b))) - } else { - // jq (#2906): `binop_mod` is `dtoi(a) % dtoi(b)` on the - // operands' *doubles* -- a literal's 17-digit-rounded one -- - // with the remainder held as a double again, which is - // exactly `mod_floats`' truncating model. (`dtoi` saturates - // like `as i64` on the arm64 oracle; the C cast of exactly - // 2^63 is undefined behaviour and differs on x86_64 jq.) - mod_floats::(int_to_f64::(a), int_to_f64::(b), &left, &right) } } (Some(NumberRepr::Float(a)), Some(NumberRepr::Float(b))) => { diff --git a/src/jq/eval_generic.rs b/src/jq/eval_generic.rs index 81aae9e44..5e050ca0d 100644 --- a/src/jq/eval_generic.rs +++ b/src/jq/eval_generic.rs @@ -1728,12 +1728,21 @@ fn materialize_lazy_keys( ) -> Result { let mut keys = effective_key_values(fields, collapse)?; if sorted { - // `sorted` is only ever `true` in jq mode: the yq parser lowers - // `keys` to `Builtin::KeysUnsorted` (real yq returns keys in - // document order), so this streaming entry point -- which has no - // `S` of its own -- never sorts under yq semantics, and jq's number - // model (#2906) is the only one it can need. (YAML keys are typed, - // so "keys are always strings" would be the wrong reason.) + // `sorted` is only ever `true` in jq mode -- not by convention, but + // because the parser decides which `Builtin` variant a `keys` call + // compiles to at parse time, keyed on `self.mode == ParserMode::Yq` + // (`try_parse_builtin` in `parser.rs`): yq mode always lowers to + // `Builtin::KeysUnsorted` (real yq returns keys in document order), + // jq mode always to this `sorted: true` `Builtin::Keys`. A single + // compiled query is only ever evaluated under the mode it was + // parsed in, so a `sorted: true` `LazyKeys` reaching this streaming + // entry point -- which has no `S` of its own -- structurally cannot + // have come from a yq-mode query; jq's number model (#2906) is the + // only one it can need. (YAML keys are typed, so "keys are always + // strings" would be the wrong reason.) `S` isn't threaded through + // just to make that static rather than structural: `into_lazy_items` + // and every caller up to `materialize_lazy_keys` would need it too, + // for an invariant already enforced one layer up, at compile time. keys.sort_by(compare_values::); } Ok(OwnedValue::Array(keys)) diff --git a/src/jq/value.rs b/src/jq/value.rs index efcb5a881..e67b90c86 100644 --- a/src/jq/value.rs +++ b/src/jq/value.rs @@ -2928,19 +2928,15 @@ pub(crate) fn cmp_decimal_literals(a: &str, b: &str) -> core::cmp::Ordering { /// zeros (empty means zero). Parsing stops at the first byte that is not /// part of a number, so a stray suffix cannot panic. fn decompose_decimal_literal(text: &str) -> (bool, Vec, i128) { - let bytes = text.as_bytes(); + // Sign-stripping and exponent-digit parsing/saturation each already have + // exactly one implementation elsewhere in this module + // ([`strip_leading_sign`], [`parse_literal_exponent`]/[`ExpParse`]) -- + // reuse both instead of adding a third copy of either (#2906 code + // review; see [`strip_leading_sign`]'s own doc comment for why that + // pattern keeps getting reintroduced). + let (negative, rest) = strip_leading_sign(text); + let bytes = rest.as_bytes(); let mut i = 0; - let negative = match bytes.first() { - Some(b'-') => { - i = 1; - true - } - Some(b'+') => { - i = 1; - false - } - _ => false, - }; let mut digits = Vec::new(); let mut exponent: i128 = 0; while i < bytes.len() && bytes[i].is_ascii_digit() { @@ -2956,27 +2952,22 @@ fn decompose_decimal_literal(text: &str) -> (bool, Vec, i128) { } } if i < bytes.len() && (bytes[i] == b'e' || bytes[i] == b'E') { - i += 1; - let exponent_negative = match bytes.get(i) { - Some(b'-') => { - i += 1; - true - } - Some(b'+') => { - i += 1; - false - } - _ => false, - }; - let mut e: i128 = 0; - while i < bytes.len() && bytes[i].is_ascii_digit() { - // Saturate far beyond any exponent a literal can hold, so an - // absurdly long exponent still orders correctly without - // overflowing the arithmetic above. - e = (e * 10 + i128::from(bytes[i] - b'0')).min(1 << 62); - i += 1; + let exp_start = i + 1; + let mut j = exp_start; + if matches!(bytes.get(j), Some(b'-' | b'+')) { + j += 1; + } + let digits_start = j; + while j < bytes.len() && bytes[j].is_ascii_digit() { + j += 1; + } + // No digits at all (`1e`, `1e+`) contributes nothing, matching the + // rest of this function's "stop at the first byte that isn't part + // of a number" leniency -- `parse_literal_exponent` only runs once + // there's an actual digit string for it to parse and saturate. + if j > digits_start { + exponent += parse_literal_exponent(&rest[exp_start..j]).value(); } - exponent += if exponent_negative { -e } else { e }; } let leading_zeros = digits.iter().take_while(|d| **d == b'0').count(); digits.drain(..leading_zeros); @@ -3006,6 +2997,25 @@ fn decompose_decimal_literal(text: &str) -> (bool, Vec, i128) { pub(crate) fn jq_numeric_cmp(left: &OwnedValue, right: &OwnedValue) -> Option { use core::cmp::Ordering; + // Two int-bearing operands (bare `Int`, or a `NumberLiteral` carrying an + // int repr -- the shape every parsed-JSON integer takes) always end up + // at `cmp_literals`'s own `x.cmp(&y)` fast path below regardless of + // text or magnitude, so check for that pairing directly rather than + // building a `Literal`/`Side` for each operand (and a wasted `as f64` + // cast per side) first. This is the hot case: document numbers are + // always `NumberLiteral`, so ordinary `sort`/`unique`/`group_by`/ + // `min`/`max`/`bsearch` over a plain JSON integer array hits it on + // every comparison (#2906 code review). + fn literal_int(value: &OwnedValue) -> Option { + match value { + OwnedValue::Int(n) | OwnedValue::NumberLiteral(NumberRepr::Int(n), _) => Some(*n), + _ => None, + } + } + if let (Some(a), Some(b)) = (literal_int(left), literal_int(right)) { + return Some(a.cmp(&b)); + } + struct Literal<'a> { double: f64, int: Option, From 2f95331d20d36614f1bf568681adb1e91d5804b1 Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 14 Sep 2026 13:51:24 +1000 Subject: [PATCH 5/5] fix(jq): clamp saturated literal exponents and pin the bridge-closed rows (#2906) `decompose_decimal_literal` now reuses `parse_literal_exponent`, which saturates an over-long exponent to `i128::MIN`/`MAX`; the fraction and digit-count adjustments around that value then overflowed (a debug-build panic on `1e<45 digits> == 2e<45 digits>`, reachable because both doubles are infinite and the tie falls through to the decimal comparison). Clamp the parsed exponent to +/-2^100 first, and pin three such spellings. Rebasing onto main also picked up #2902, whose reindex bridge now hands a computed float back as a bare `Float` instead of re-parsing its printed digits as an integer literal -- which is exactly the gap #2938 was filed for. Its `sort`/`unique`/`max`/`==` rows now match jq, so they move out of the characterization test into the positive comparison rows, and the limitations entry records #2938 as closed by #2902. --- docs/compliance/jq/limitations.md | 21 ++++++------ src/jq/value.rs | 28 ++++++++++++++- tests/jq_cli_tests.rs | 57 +++++++++++++++++-------------- 3 files changed, 68 insertions(+), 38 deletions(-) diff --git a/docs/compliance/jq/limitations.md b/docs/compliance/jq/limitations.md index 47604e752..7d3a68536 100644 --- a/docs/compliance/jq/limitations.md +++ b/docs/compliance/jq/limitations.md @@ -5846,7 +5846,7 @@ compiler/libc is an open question, not a claim to build on (flag for whoever clo this magnitude (`-9223372036854775807 % 10` is `-7` in jq, `-8` here; the parenthesised and data-sourced spellings agree on `-8`). -**Still open, same mechanism, out of #2906's scope** (tracked as #2936, #2937 and #2938): +**Still open, same mechanism, out of #2906's scope** (tracked as #2936 and #2937): a *float* literal with more than 17 significant digits, or an integer literal beyond `i64`, is still parsed with one correct rounding (`2.7293109604053567083 + 0` is `2.7293109604053565` in jq, `2.729310960405357` here) — closing it needs the mode plumbed @@ -5855,16 +5855,15 @@ builtins (`floor`, `sqrt`, `pow`, …) still widen a large `Int` with a bare cas (`869389897822472004 | sqrt` is `932410798.8555645` in jq, `…647` here); and `floor`/`ceil`/`round`/`trunc` (and `length`, which is `fabs(jv_number_value(x))` in jq) of such a value print their exact integer digits where jq prints the double -(`869389897822472000`) (all #2937). And a computed double that crosses the reindex bridge -into a document-input builtin (`sort`, `unique`, `min`, `max`, `group_by` on an array -built in the filter) is re-parsed from its printed digits as an *integer* literal, so it -then compares exactly against a real literal instead of equal: -`[869389897822472004, (869389897822472000+0), 5] | sort` is -`[5,869389897822472000,869389897822472004]` here and -`[5,869389897822472004,869389897822472000]` in jq (stable, the two are equal there) — -identical before and after this fix, since the comparator is right and the bridge is what -changes the operand (#2938). The `range` entry above and the unary-minus entry (#2357) are -unaffected — both stay on the exact `i64` path they document. +(`869389897822472000`) (all #2937). A third gap this fix's review found — the reindex +bridge re-parsing a computed double's printed digits as an *integer* literal before a +document-input builtin (`sort`, `unique`, `min`, `max`, `group_by` on an array built in +the filter), so that it then compared exactly against a real literal instead of equal — +was filed as #2938 and closed by #2902 landing first: the bridge now hands a computed +float back as a bare `Float`, so `[869389897822472004, (869389897822472000+0), 5] | sort` +is jq's `[5,869389897822472004,869389897822472000]` here too. The `range` entry above and +the unary-minus entry (#2357) are unaffected — both stay on the exact `i64` path they +document. ### `--argjson`/`--jsonargs` still reject a bare trailing decimal point with no exponent (`1.`) — accepted divergence, ADR-0018 rule 4c (#2240) diff --git a/src/jq/value.rs b/src/jq/value.rs index e67b90c86..e874b427c 100644 --- a/src/jq/value.rs +++ b/src/jq/value.rs @@ -2966,7 +2966,16 @@ fn decompose_decimal_literal(text: &str) -> (bool, Vec, i128) { // of a number" leniency -- `parse_literal_exponent` only runs once // there's an actual digit string for it to parse and saturate. if j > digits_start { - exponent += parse_literal_exponent(&rest[exp_start..j]).value(); + // `parse_literal_exponent` saturates an over-long exponent to + // `i128::MIN`/`MAX`; clamp far beyond any exponent a double can + // distinguish so the digit-count and fraction adjustments around + // it cannot overflow (a debug-build panic on + // `1e<45 digits> == 2e<45 digits>`, which reaches this comparison + // because both doubles are infinite). + const EXPONENT_CLAMP: i128 = 1 << 100; + exponent += parse_literal_exponent(&rest[exp_start..j]) + .value() + .clamp(-EXPONENT_CLAMP, EXPONENT_CLAMP); } } let leading_zeros = digits.iter().take_while(|d| **d == b'0').count(); @@ -3862,6 +3871,23 @@ mod tests { ("1e-400", "0", Greater), ("-1e-400", "0", Less), ("1e999999999999999999999", "2e999999999999999999999", Less), + // Exponents too long for `i128` saturate; the fraction and + // digit-count adjustments must not overflow around them. + ( + "1.5e999999999999999999999999999999999999999999999", + "2e999999999999999999999999999999999999999999999", + Less, + ), + ( + "1.5e-999999999999999999999999999999999999999999999", + "0", + Greater, + ), + ( + "-0.25e-999999999999999999999999999999999999999999999", + "0", + Less, + ), ] { assert_eq!(cmp_decimal_literals(a, b), want, "{a} vs {b}"); assert_eq!(cmp_decimal_literals(b, a), want.reverse(), "{b} vs {a}"); diff --git a/tests/jq_cli_tests.rs b/tests/jq_cli_tests.rs index d39ab68ce..ad36d2cdf 100644 --- a/tests/jq_cli_tests.rs +++ b/tests/jq_cli_tests.rs @@ -53737,14 +53737,11 @@ fn test_large_int_literal_rounds_to_17_digits_before_f64_arith_2906() -> Result< /// (its own rounded double) but equal to `...2004.0`, whatever double that /// spelling parses to. Captured live against `/usr/bin/jq` 1.7.1. /// -/// Only the binary operators are pinned for the literal-vs-computed rule. -/// `sort`/`unique`/`min`/`max` on an array built in the filter cross the -/// reindex bridge, which re-parses the computed double `869389897822472000` -/// as an *integer* literal, and from then on it compares exactly against -/// the real literal -- identical before and after #2906, a separate bridge -/// gap tracked as #2938 (see `..._characterize_preexisting_bug_2906`). -/// Literal-vs-literal pairs survive the bridge unchanged, so they are -/// pinned through the ordering builtins too. +/// The ordering builtins (`sort`/`unique`/`min`/`max`) on an array built in +/// the filter cross the reindex bridge; since #2902 that bridge hands a +/// computed double back as a bare `Float` rather than re-parsing its +/// printed digits as an integer literal, so the literal-vs-computed rule +/// holds through them too (the row set that #2938 was filed for). #[test] fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<()> { for (filter, want) in [ @@ -53801,6 +53798,24 @@ fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<() "[869389897822472004, 869389897822471936.5, (869389897822472000+0)] | min", "869389897822471936.5", ), + // Literal vs computed through the bridge: equal, so a stable sort + // keeps input order, `unique` keeps the first, `max` the last. + ( + "[869389897822472004, (869389897822472000+0), 5] | sort", + "[5,869389897822472004,869389897822472000]", + ), + ( + "[869389897822472004, (869389897822472000+0)] | unique", + "[869389897822472004]", + ), + ( + "[5, 869389897822472004, (869389897822472000+0)] | max", + "869389897822472000", + ), + ( + "[869389897822472004, (869389897822472000+0)] | .[0] == .[1]", + "true", + ), ] { let (output, code) = run_jq_null(filter, &["-c"])?; assert_eq!(code, 0, "`{filter}`"); @@ -53822,27 +53837,17 @@ fn test_large_int_literal_rounds_to_17_digits_in_comparisons_2906() -> Result<() Ok(()) } -/// #2906 left three same-mechanism gaps open (tracked as #2936, #2937 and -/// #2938); this pins their *current* answers so a later change cannot move -/// them unnoticed, and so the eventual fixes have a test to flip. Each -/// row's jq 1.7.1 answer is in the comment; succinctly's output was -/// verified identical before and after #2906. +/// #2906 left two same-mechanism gaps open (tracked as #2936 and #2937); +/// this pins their *current* answers so a later change cannot move them +/// unnoticed, and so the eventual fixes have a test to flip. Each row's jq +/// 1.7.1 answer is in the comment; succinctly's output was verified +/// identical before and after #2906. (A third gap, #2938 -- the reindex +/// bridge re-parsing a computed double as an integer literal -- closed when +/// #2902 landed; its rows now live in +/// `test_large_int_literal_rounds_to_17_digits_in_comparisons_2906`.) #[test] fn test_large_int_literal_gaps_characterize_preexisting_bug_2906() -> Result<()> { for (filter, current, jq_says) in [ - // #2938: the reindex bridge re-parses the computed double - // `869389897822472000` as an integer literal, which then compares - // exactly (`000 < 004`) instead of equal to the literal. - ( - "[869389897822472004, (869389897822472000+0), 5] | sort", - "[5,869389897822472000,869389897822472004]", - "[5,869389897822472004,869389897822472000]", - ), - ( - "[5, 869389897822472004, (869389897822472000+0)] | max", - "869389897822472004", - "869389897822472000", - ), // #2936: a >17-digit float literal is parsed with one correct // rounding, not jq's 17-digit pre-rounding. (