Inline small values with a decimal number representation (#24)#43
Merged
Conversation
Rework the IValue representation to store null, booleans, small decimal numbers, and short strings entirely inline, behind a 3-bit tag scheme. - ALIGNMENT is now 8, giving an 8-variant TypeTag: an `Inline` family (0) plus dedicated pointer tags for i64/u64/f64 numbers (with a reserved slot), string, array, and object. - Inline numbers are stored as a decimal `mantissa * 10^exp` (56-bit mantissa on 64-bit, 24-bit on 32-bit). This inlines most JSON numbers — integers, timestamps/ids, and short decimals like 0.5 and 63.5 — with no allocation or indirection. The 4-bit exponent field also carries the `has_decimal_point` flag using two exponent-0 codes. Values that don't fit fall back to a bare 8-byte heap payload whose type is in the tag. - Representation is canonical (fits-inline => stored inline), so inline and heap value-sets are disjoint and numeric comparison stays exact. - Short strings move into the inline family (tag 0, string sub-family); interned strings, arrays, and objects get dedicated pointer tags. - Deletes STATIC_NUMBERS, NumberType, and the i24/heap number header. The all-zero pattern is reserved as the NonNull niche (the exponent is biased so integer zero is non-zero). Validated across the platform matrix: native x86-64, i686 (32-bit), and Miri with no UB on native, s390x (big-endian 64-bit), and powerpc (big-endian 32-bit).
Drop, Clone, Hash, and Debug now match on the raw representation tag rather than the semantic type_(). Inline values own no heap storage, so they are dropped/copied uniformly without consulting type_(); only the pointer representations delegate to their module. This structurally rules out the class of footgun that caused the earlier infinite recursion (type_() re-entering type classification during Drop), and is more efficient (no semantic-type computation on teardown). Heap clone_impl/drop_impl are simplified since inline is handled by IValue.
Restructure so modules are organised by *representation* (storage form)
rather than by JSON type, and so the root IValue owns the logic while the
type wrappers delegate up to it.
Representation modules, each owning the low-level ops on IValue:
- inline/ the inline family (mod), with number and string submodules
for the inline decimal and short-string codecs
- scalar.rs the heap 8-byte number payload (NumberI64/U64/F64)
- interned.rs the heap interned string (header, cache, refcount)
- array.rs / object.rs unchanged (type == representation)
IValue's tag-dispatched trait impls (Drop/Clone/Hash) now call into the
representation modules directly (scalar_clone/drop, interned_clone/drop,
number_hash). The number- and string-type logic (to_i64, has_decimal_point,
as_str, ...) lives as `impl IValue` methods that pick the representation by
tag, and INumber/IString are thin wrappers that delegate up to them. The
inline constants and detection move from value.rs into the inline module.
No behaviour change. Validated across native x86-64, i686 (32-bit), and
Miri with no UB on native, s390x (BE 64-bit) and powerpc (BE 32-bit).
The representation modules previously implemented methods on IValue and
value.rs inspected inline bits directly. Invert this properly:
- inline/scalar/interned now expose free functions that operate on the
raw representation (a `usize` for inline, a `NonNull<u8>` for the heap
representations) and know nothing about IValue. The inline module owns
the sub-family classification (value_type / is_number / is_string) and
the null/false/true bit constants.
- IValue delegates *down* to them: type_() -> inline::value_type, Clone/
Drop -> scalar::{alloc,read,free} / interned::{bump_rc,release}, the
constants and is_null/is_bool via inline. The is_inline_number/string
helpers combine the tag (IValue's concern) with inline classification.
- The number/string type methods on IValue orchestrate across the
representations by calling the free functions with raw bits/pointers.
value.rs no longer contains representation-specific bit logic. No
behaviour change. Validated across native x86-64, i686 (32-bit), and Miri
with no UB on native, s390x (BE 64-bit) and powerpc (BE 32-bit).
is_inline_number()/is_inline_string() leaked the inline sub-classification up into value.rs. Instead, value.rs matches TypeTag and delegates the entire Inline case to the inline module: - Hash: the Inline arm calls inline::hash(bits), which dispatches number-vs-(string/null/bool) itself and hashes numbers by value. - is_number/is_string go through type_() (which delegates inline classification to inline::value_type); Debug likewise dispatches on the semantic type_. - value.rs keeps only a plain is_inline() (a pure tag check); the inline sub-family predicates and constants live entirely in the inline module. - inline::number now exposes high-level per-bits operations (value_i128, to_f64_exact/lossy, has_decimal_point, hash) so the number type methods no longer poke at mantissa/code; the low-level decoders are private. No behaviour change. Validated across native x86-64, i686 (32-bit), and Miri with no UB on native, s390x (BE 64-bit) and powerpc (BE 32-bit).
Complete the inversion: - No module other than value.rs implements methods on IValue. The number and string type logic is now free functions in their modules (number::to_i64, string::as_str, ...) taking &IValue. - IValue's trait impls (Clone/Drop/Hash/PartialEq/PartialOrd/Debug) and its public number accessors now delegate to the module free functions (number::/string::/array::/object::/scalar::/interned::) instead of reaching into the INumber/IString/IArray/IObject wrappers. - array.rs/object.rs gained representation free functions (clone/drop/hash/eq/cmp/debug) that value.rs delegates to; IArray/IObject are used only inside their own module. - The I-type wrappers delegate to the same free functions, so the flow is wrapper -> module free function -> representation, with IValue as the only thing that owns the tag scheme. No behaviour change. Validated across native x86-64, i686 (32-bit), and Miri with no UB on native, s390x (BE 64-bit) and powerpc (BE 32-bit).
Reorganize so every value representation lives as a submodule of `value`
and the public wrapper types are thin facades that delegate down into
them. Previously the low-level operations `value` needed (clone/drop/
hash/eq/cmp/debug for arrays and objects) routed back through the public
`IArray`/`IObject` types via `as_array()`/`as_object()`, mixing the type
with its representation and making the delegation direction hard to see.
Now:
- `value::{inline, scalar, interned}` (moved under `value/`), plus new
`value::{number, string, array, object}` submodules, hold all the
representation and per-type logic as free functions on `&IValue`.
- `IValue`'s trait impls delegate only to these submodules; they never
reference the public wrapper types for a low-level operation.
- The public `INumber`/`IString`/`IArray`/`IObject` types (and the
object `Entry`/iterator/index API) stay in the top-level modules and
delegate *down* to the matching `value::` submodule.
- `value::array`/`value::object` operate on the header representation
directly (object clone/hash/eq/debug reimplemented at the header
level) so they no longer reach back up through `IArray`/`IObject`.
No public API or behavior change. Validated with cargo test, clippy,
fmt, and Miri on x86-64 (LE-64), s390x (BE-64), powerpc (BE-32) and
i686 (LE-32) — no UB on any target.
Now that the representations are submodules of `value`, they can see `value`'s private items directly, so the low-level `IValue` accessors they use no longer need `pub(crate)`. Make `ALIGNMENT`, `TypeTag`, `new_inline`, `new_ptr`, `new_ref`, `ptr_usize`, `ptr`, `set_ptr`, `set_ref`, `raw_copy` and `type_tag` private to the `value` module. Only `raw_eq`, `raw_hash` (used by the `IString` facade) and `is_inline` (used by the `INumber`/`IString` facade tests) remain `pub(crate)`.
…der value A JSON number/string is not a representation — each is a type that spans two representations (inline decimal vs heap scalar; inline short string vs interned string). So the misnamed value::number / value::string modules are gone; their cross-representation dispatch now lives directly on IValue as new_*/number_*/string_* methods, which pick a representation as early as possible and defer to it: - new_string asks inline::string::try_encode (new) whether the string fits inline, and only interns when it does not; new_i64/new_u64/new_f64 likewise try inline::number first, then fall back to a heap scalar. - number_*/string_* accessors dispatch on the tag and immediately defer to the owning representation (inline::number/scalar, inline::string/ interned), with the small amount of genuinely cross-representation logic (numeric NumVal comparison) kept as private helpers. value/ now contains only representations: inline, scalar, interned, array, object. INumber/IString are unchanged thin facades that delegate to the new IValue methods. No public API or behavior change; validated with cargo test, clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686 — no UB on any target.
…eros The 4-bit exponent code previously placed the two exponent-0 encodings adjacent (code 7 = plain integer, code 8 = "N.0"), which forced a piecewise `code_exp`/`code_has_dot`. A decimal point only ever needs distinguishing at exponent 0 (a larger integer that had a decimal point cannot fit the mantissa in fractional form and spills to a heap f64). So reserve a single code for the plain integer at exp 0 and make every other code linear: `exp = code - 7` (EXP_BIAS). Codes 0..=7 are exp -7..=0 and all carry a decimal point, so `has_decimal_point` collapses to `code <= 7`; codes 8..=14 are integers at exp 1..=7. The reserved code is the maximum (15), not 0: the plain-integer path is the only one that emits a zero mantissa, so putting it at code 0 would make integer zero the all-zero bit pattern and collide with the NonNull niche. Keeping it at 15 preserves the niche. Added a unit test covering the exponent round-trip, the exp-0 split, and the niche property. Internal encoding only (inline numbers are never serialized); no public API or behavior change. Validated with cargo test, clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686 — no UB.
`has_decimal_point()` drives serialization (serialize_f64 vs serialize_i64), so the decimal-point flag a code carries is observable, not cosmetic. Positive exponents (codes 8..=14) previously meant "plain integer, no decimal point" — used to factor large round integers inline. Repurpose them for floats instead: code 0..=6 -> exp -7..=-1 fraction (decimal point) code 7 -> exp 0 integer-valued float (decimal point) code 8..=14 -> exp 1..=7 e-notation float (decimal point) code 15 -> exp 0 plain integer (no decimal point) Now every code but the reserved 15 carries a decimal point, so `has_decimal_point` collapses to `code != 15`. Consequences: - `encode_int` no longer factors trailing zeros into a positive exponent (that would mislabel an integer as a float and serialize it back as one). A large round integer that exceeds the mantissa spills to a heap i64/u64 instead — still exact, just an allocation. - integer-valued floats now factor trailing zeros into a positive exponent, so e.g. `1e18` stays inline (mantissa 1e11, exp 7) rather than going to a heap f64. Net effect: the inline optimization shifts from large round integers to large e-notation floats, and integer-vs-float round-trips through serde are preserved either way. Added tests covering the code layout and the distinct serialization of `1000000000000000000` vs `1e18`. Internal encoding only; no public API change. Validated with cargo test, clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686 — no UB.
`f64_as_integer` and `f64_scaled_integer` used `u128::checked_shl` to guard the exponent shift, but `checked_shl` only rejects shift *amounts* >= 128 — it silently drops the high bits when the shifted *value* overflows the 128-bit width. For a large float such as 2^127, the fractional probe computed `(5 * 2^52) << 76`, which wraps to 0 in a u128, so the encoder concluded the value was the exact decimal `0 * 10^-1` and stored it inline as zero. `IValue::from(2e127)` then reported `0.0` and serialized as `"0.0"`. Replace the two `checked_shl` calls with `shl_checked`, which returns `None` when the shift would overflow (i.e. when the shift exceeds the value's leading-zero count). Large magnitudes that cannot be represented exactly inline now correctly spill to a heap f64 instead of encoding to a wrong value. Found by the numeric edge-case harness (2^127 and other >= ~2^76 magnitudes). Added a regression test in the inline-number module.
Thorough testing of the numeric paths starts from the discontinuities in how numbers are represented, not from random sampling. Add a shared `numeric_edge_cases` module (test-only) with one list per input flavour — `i64`, `u64`, `f64` (finite and non-finite), and raw JSON strings — each targeting a specific class of boundary: - inline mantissa fit (2^23 / 2^55) and the 2^53 f64-exactness cliff - every integer type limit and the i64/u64 seam at 2^63 - float->int conversion thresholds at 2^63 / 2^64 - dyadic short decimals (exp -1..=-7) and near-miss "short-looking" ones - positive-exponent factoring of integer-valued floats - the int-vs-float (`has_decimal_point`) distinction - special floats (0.0/-0.0, NaN/Inf, subnormals, MIN/MAX, 2^127) - big-int literals that overflow u64 and reparse as floats Alongside the lists, a foundational harness runs each through core invariants (exact round-trips, conversion consistency, decimal-point correctness, and agreement with serde_json's own pipeline). This already surfaced the u128 shift-overflow bug fixed in the previous commit. Note: the f64/JSON round-trip invariants only require ijson to lose no more than serde_json's *own* pipeline, because serde_json's default float parser is not exact-round-trip (its precise parser is behind the `float_roundtrip` feature, which is not enabled here).
Extend the numeric edge-case harness to cover ordering, hashing, and the normalisation guarantee that numerically-equal values are indistinguishable regardless of how they were built or which representation they land in: - normalisation_makes_equal_values_indistinguishable: curated groups of the same number reached via different types/representations (e.g. -0.0, +0.0 and integer 0; 1 and 1.0; heap 10^18 and inline 1e18; the i64/u64 seam at 2^63). Every member of a group must compare equal AND hash equal; representatives of different groups must differ. - sorting_matches_reference_order: a hand-verified strictly-increasing sequence spanning representations and the precision-trap adjacencies (a large integer just above the float that rounds to it, the i64/u64/float seams). `cmp` must recover exactly this order and never conflate distinct values. - hash_respects_equality_across_the_pool / comparison_is_a_consistent_ total_order: exhaustive O(n^2) checks over every value from the lists — the Hash/Eq contract (equal implies hash-equal, which cross-checks the separate `inline::number::hash` and `number_hash` implementations) and antisymmetry / eq-cmp agreement. Gated out of Miri for run time; the curated tests exercise the same unsafe paths there. All pass, confirming the two hash implementations agree and equality is consistent across representations. Validated with cargo test, clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
Equal values were checked to compare and hash equal, but not to be stored identically. Add that missing invariant: the same number reached different ways must have the *identical internal representation*, with the decimal point as the sole allowed difference. - Add a test-only `IValue::number_repr_key` returning a number's tag plus its inline bits or 8-byte heap payload; two numbers with equal keys are bit-for-bit the same storage. - `representation_is_canonical`: integers built via i64 / u64 / JSON land on the same bits; grouped equal magnitudes share one representation within a decimal-point class. In particular -0.0, +0.0 and integer 0 collapse as expected (the signed zeros are literally the same bits). - Extend the exhaustive pairwise pool check: every pair that is equal and agrees on `has_decimal_point` must also share `number_repr_key` — so no value has two distinct canonical encodings. Confirms canonicalisation holds across construction paths and the inline/heap boundary. Validated with cargo test, clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
Two related changes. Round-trip test (round_trips_through_inumber_exactly): converting any number from the edge-case lists into an INumber and back with the matching accessor returns exactly the original value — including the *exact* `to_f64` accessor, not just the lossy one. i128 reduction: the runtime comparison/hash path used i128 only to span `i64 ∪ u64 ∪ (up to ~3.6e23)`. But a value beyond `u64` is always an integer-valued float that factored inline, and so is exactly representable as `f64` — it can be a `Float`, not an exact integer. Only heap `u64`s genuinely exceed `i64`. So: - `NumVal` becomes `Int(i64) | UInt(u64) | Float(f64)` (was `Int(i128)`), and `cmp_int_f64(i128, _)` splits into `cmp_i64_f64` / `cmp_u64_f64` with range guards — no 128-bit arithmetic in comparison or hashing. - `value_i128`/`decimal_to_i128`'s fractional case shrink to `i64` (`value_i64`), and `new_u64` no longer probes the inline encoder for values it can never fit. - The duplicate `inline::number::hash` is gone: the `Hash` impl now routes inline numbers through the same `number_hash` as heap numbers, which also removes the only place the two hash paths could disagree. `i128`/`u128` now appears only in the f64 exact-decimal *encoding* path (`f64_as_integer`, `f64_scaled_integer`, `encode_int_float`, `decimal_to_i128`, `i128_fits_f64`), where intermediate products genuinely exceed `u64`. No public API or behaviour change. Validated with cargo test (the round-trip, hash-contract, total-order and canonicalisation harness), clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
The Clone/Drop/PartialEq/PartialOrd/Debug impls each had their own tag/type_ match. Replace them with a single dispatch: `repr()` classifies the value once and returns a `&'static dyn ValueRepr`, and each impl delegates to a trait method. Every operation is fundamentally per-representation — cloning an inline value is a bit-copy, a scalar allocates, an interned string bumps its refcount — so the trait is keyed on representation, with the inline and heap forms of number and string as distinct markers (`InlineNumberRepr` vs `ScalarRepr`, `InlineStringRepr` vs `InternedRepr`, plus `NullRepr`, `BoolRepr`, `ArrayRepr`, `ObjectRepr`). The binary `eq`/`partial_cmp` are the one complex case: the impls do the type check, then the representation's method handles any cross-representation comparison (an inline decimal vs a heap scalar, via `number_cmp`). `repr()` returns a `&'static dyn` built from a match of concrete zero-sized markers, so once it inlines the optimizer devirtualizes each arm back to a direct call — no indirection on the hot inline clone/drop. `Hash` stays a direct match: its `<H: Hasher>` method can't be a trait-object method, and erasing the hasher to `&mut dyn Hasher` would make every write virtual (and, unlike the others, un-devirtualizable). No public API or behaviour change. Validated with cargo test (the full numeric + type harness), clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
Push the rest of IValue's per-value behaviour down into ValueRepr, so value/mod.rs is delegation rather than matches. The trait now carries value_type, hash, destructure/destructure_ref/destructure_mut, to_bool, to_i64/to_u64/to_f64/to_f64_lossy and len alongside clone/drop/eq/ partial_cmp/debug — most with default implementations, so a representation only overrides what it actually supports: - Defaults encode the inline/common behaviour: clone = copy the pointer word, drop = nothing, hash = the canonical pointer word, eq = raw bits, partial_cmp = unordered, and every accessor = None. The inline constants and string reps ride almost entirely on the defaults. - The number and string value operations (identical across their inline and heap representations) are factored into `number_repr_ops!` / `string_repr_ops!` macros; the two number reps and two string reps then override only clone/drop. value/mod.rs's type_/destructure*/to_*/len/is_empty now just call `self.repr().method(self)`; the only remaining `match self.type_tag()` are `repr()` itself (the single dispatch point) and a heap scalar decoding its own payload by tag. hash also moves into the trait via `&mut dyn Hasher`: IValue::hash_dyn erases the concrete hasher once (so the top-level writes still devirtualize when inlined), and array/object recurse through hash_dyn so the same &mut dyn Hasher threads through without nesting. No public API or behaviour change. Validated with cargo test (the full numeric + type harness, including the exhaustive hash/eq/cmp/repr contract), clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
The previous commit forced `hash` into `ValueRepr` via `&mut dyn Hasher`, which required an `IValue::hash_dyn` helper that the array representation called to recurse — a representation delegating back *up* into `IValue`. Delegation must only run I-type -> IValue -> representation [-> more specific representation], never upward. The root cause is that `Hash::hash` is generic over the hasher, so it cannot be an object-safe trait method; erasing the hasher to `&mut dyn Hasher` is what created the upward bounce. So hashing stays generic on `IValue`: - `IValue: Hash` dispatches on `type_()` and calls the generic `number_hash`/`array::hash`/`object::hash` (or hashes the bit pattern); - `array::hash`/`object::hash` recurse into their elements through the elements' own `Hash` impls (`slice.hash(state)`, `(&k, &v).hash(h)`) — exactly how `clone`/`eq`/`cmp`/`debug` already recurse through the elements' standard trait impls, never through a bespoke `IValue` helper. `ValueRepr` keeps clone/drop/eq/partial_cmp/debug plus the accessors; `hash` is the one operation it cannot hold, and the trait docs say why. No public API or behaviour change. Validated with cargo test (full numeric + type harness incl. the hash/eq contract), clippy, fmt, and Miri on x86-64, s390x (BE-64), powerpc (BE-32) and i686.
IValue::hash now erases the concrete hasher once and dispatches to self.repr().hash(), matching every other operation. The value-based number hash (which keeps 1e18 and 10^18 agreeing across the inline/heap boundary) moves into the shared number representation; the standalone IValue::number_hash is gone. Arrays recurse element-by-element via each element's repr(); objects recurse through their entries' Hash impls.
value/mod.rs kept the ValueRepr trait, all eight ZST markers, their impls, and the number/string dispatch methods that branched on inline-vs-heap. That put every representation's behaviour in one file and hid the per-rep code behind number_repr_ops!/string_repr_ops! macros. Now each representation owns its marker and impl next to its storage code: InlineNumberRepr in inline/number.rs, ScalarRepr in scalar.rs, InlineStringRepr in inline/string.rs, InternedRepr in interned.rs, ArrayRepr/ObjectRepr in array.rs/object.rs, and NullRepr/BoolRepr in a new inline/constant.rs. mod.rs keeps only the trait and the repr() dispatch. Both macros are gone. Each number representation reduces its own storage to a NumVal (inline::number::num_val decodes the decimal; scalar::num_val reads the heap payload) and the shared value logic lives in standalone num_to_i64/u64/ f64/f64_lossy/num_hash/num_debug/number_cmp utility functions. The branching IValue::number_* accessors are removed; the facade and public API reach numbers through repr()-dispatched trait methods (to_i64, to_f64_lossy, has_decimal_point). Strings get the same treatment via a new as_bytes trait method, so each string representation supplies its own bytes and string_cmp/string_debug are shared. The only inline-vs-heap dispatch that remains is resolving an operand of unknown representation when comparing two numbers or two strings (num_val_of / string_as_str) — the acknowledged binary-operation case. cargo test (57 + doctest, single-threaded), clippy --all-targets --all-features, fmt --check, and Miri on x86-64/s390x/powerpc/i686 all pass.
The four inline types (null, bool, inline number, inline string) each had their own ValueRepr impl, and repr() matched on the family bits to pick among them. Since every inline value clones/drops identically (a bit-copy with nothing to free) and is never a collection, that split duplicated the trivial cases and spread the family decode across repr(). Now a single inline::InlineRepr implements ValueRepr for the whole family: repr() returns it for every inline value, and each of its methods decodes the family bits once (InlineRepr::inner) and delegates to an inline sub-repr through a new inline-only InlineValue trait. InlineValue mirrors ValueRepr's value operations but drops clone/drop/len, which InlineRepr supplies uniformly. The NullRepr/BoolRepr/InlineNumberRepr/InlineStringRepr markers now implement InlineValue with unchanged bodies. ValueRepr::value_type gains a "v" argument so the one inline impl can decode the type; the single-type representations ignore it. cargo test (57 + doctest, single-threaded), clippy --all-targets --all-features, fmt --check, and Miri on x86-64/s390x/powerpc/i686 all pass.
num_val decoded an inline number and, when it was not an i64, unwrapped to_f64_exact with an expect that assumed every non-i64 inline number is exactly an f64. That held only because today's constructors all go through encode_f64/encode_int; the inline format itself can represent any mantissa * 10^exp, including exact decimals that are not exact f64s (e.g. 0.1 == 1 * 10^-1). A decimal-aware or arbitrary-precision constructor would turn that expect into a live panic. num_val now decodes exactly when it can and no longer panics on the rest, so it is total over the whole representable domain rather than the current constructor set. Behavior is unchanged for every value that exists today (to_f64_exact is still Some for all of them). Adds a regression test that builds the inline encoding of 0.1 directly and checks num_val handles it.
The previous commit made num_val total by rounding a non-i64, non-f64 inline
number (e.g. the fraction 0.1) to the nearest f64. That removed the panic but
threw away the exact value. NumVal now carries that value exactly.
NumVal gains a Decimal { mantissa, exp } variant for an exact mantissa * 10^exp
that is neither an i64 nor an exact f64. num_to_i64/u64 extract the integer when
the decimal is integer-valued (so it stays equal to the matching Int/UInt),
num_to_f64 reports None (a Decimal is by construction not an exact f64), and
comparison is exact against integers and other decimals.
Comparison against a Float goes through the decimal's nearest f64 rather than an
exact tie-break: normalisation keeps each number in one representation, so a
Decimal and a Float are never the same number, and only distinct values need
ordering there. num_hash routes a non-integer Decimal through the same nearest
f64, so Hash and Eq stay consistent.
Also fixes the twin latent panic in INumber's Serialize impl, where
has_decimal_point implied to_f64() was Some; it now falls back to the nearest
f64 for an exact decimal that is not an exact f64.
No constructor produces a Decimal yet, so the new paths are exercised by direct
unit tests. cargo test (61 + doctest, single-threaded), clippy, fmt, and Miri on
x86-64/s390x/powerpc/i686 all pass.
Implements std::str::FromStr for INumber so a string can be parsed into a number with "...".parse::<INumber>(). Strings are validated against the JSON number grammar from json.org (optional minus, no leading zeros, a fraction needs a digit after the dot, an exponent needs a digit, no bare '+', no inf/nan, no surrounding whitespace) before parsing, since Rust's own i64/f64 FromStr are more permissive than JSON. Classification mirrors serde_json: a number written with a fraction or exponent, or a plain integer beyond u64, is stored as a float (and reports a decimal point); a plain in-range integer keeps its integer representation. An out-of-range magnitude (a float that would be infinite) is rejected so the INumber stays finite. The new public error type ParseNumberError is exported. Tested: valid integers/floats, the beyond-u64 integer-to-float path, a broad set of rejected malformed inputs, out-of-range magnitudes, and agreement with serde_json deserialization (value and decimal-point flag). cargo test (67 + doctests), clippy, fmt, and Miri all pass.
Mirrors the i64/u64/f64/JSON case lists with string_number_cases(): the curated JSON number strings plus every i64, u64 and f64 boundary rendered as text. Wires the string inputs through the same invariants used for the other lists. number_pool() now also includes each string parsed via INumber::from_str, so the exhaustive pairwise hash/representation and total-order checks cover the parser's output alongside the directly-constructed numbers. Two new per-input tests: - string_inputs_are_consistent: each string is a number with the expected decimal-point flag; an exact integer matches serde_json bit-for-bit; a float matches a direct std f64 parse (serde_json's default float parser rounds some large magnitudes differently, so it is not used as the float oracle); and the value round-trips through serialize + reparse. - string_parsing_matches_direct_construction: rendering each i64/u64/f64 case to text and parsing it back reproduces the directly-constructed number. Documents the one intentional divergence from serde_json: the token "-0" parses as the integer 0 (no decimal point), faithfully to the JSON grammar, whereas serde_json keeps the sign as -0.0. cargo test (69 + doctests), clippy, fmt, and Miri on x86-64/s390x/powerpc/i686 all pass.
Found by fuzzing (the new fuzz/number_value target). An exact f64 stored inline as `mantissa * 10^exp` with a scaled mantissa above 2^53 — e.g. 949288156749637.5 == 9492881567496375 * 10^-1 — decoded wrongly through to_f64_lossy: `mantissa as f64 * 10^exp` rounds the mantissa to the nearest f64 *before* scaling, so 949288156749637.5 came back as 949288156749637.6. to_f64_lossy now decodes exactly (via decimal_to_f64_exact, which divides out 5^k first so nothing exceeds 2^53) whenever the value is exactly an f64 — which every inline decimal a constructor can produce is — and only falls back to the naive conversion for a non-f64 inline decimal, which nothing produces today. to_f64 (the exact accessor) already used this path and was correct. Adds the fuzzer-found value and two constructed siblings to f64_cases. Full test suite, clippy, fmt, and Miri on x86-64/s390x/powerpc/i686 pass.
Three coverage-guided libFuzzer targets that check the numeric invariants over machine-generated inputs, complementing the curated edge-case tests: - number_str: INumber::from_str over arbitrary strings (parse ⟹ number, serde acceptance, serialize round-trip, integer-value agreement). - number_value: f64 construction over the whole bit space (exact round-trip via to_f64/to_f64_lossy, integer-conversion consistency, non-finite rejection). - number_ord: comparison/hashing of numbers built through different constructors (total order + Hash/Eq contract across representations). number_value found the to_f64_lossy rounding bug fixed in the previous commit. fuzz/README.md documents how to run them. On Windows, coverage-guided fuzzing works with the default AddressSanitizer build (per the cargo-fuzz book) once the ASan runtime is available — either the VS "C++ AddressSanitizer" component or a standalone LLVM's compatible clang_rt on the LIB/PATH.
from_str previously ran every float-shaped string through parse::<f64>(), so
"0.1" became the f64 approximation (and spilled to the heap) rather than the
exact decimal it denotes — defeating the reason the string constructor exists.
It now parses the digits into an exact mantissa * 10^exp and stores that inline
when it fits (inline::number::encode_decimal, via IValue::new_decimal), so "0.1"
is the exact 1 * 10^-1. Only a value too large or too precise to hold exactly
falls back to the nearest f64. The encoding is canonical: bit-identical to
encode_f64 for a value that is an exact f64, so the two constructors never
disagree.
Because exact non-f64 decimals are now reachable, the earlier premise for the
lossy Decimal-vs-f64 comparison ("normalisation keeps each number in one
representation") no longer holds: the exact decimal 0.1 and the f64 0.1 are
different numbers that coexist. So the exact Decimal-vs-f64 comparison is
reinstated (0.1 != 0.1_f64, and 0.1 < 0.1_f64), a non-integer Decimal hashes by
its canonical mantissa/exp, and comparison is once again a proper total order.
decimal_to_f64_lossy now rounds correctly for a mantissa above 2^53 (reachable
via a Decimal): an integer through the i128 -> f64 cast, a fraction through the
correctly-rounded f64 string parser.
Consequence: serialize (serde emits the shortest decimal, which rounds an f64)
followed by from_str (which now reads decimals exactly) is no longer an exact
round-trip for a heap f64 — it preserves the value only up to f64 precision. The
round-trip tests and the number_str fuzz target assert that weaker property;
exact decimal round-trips are covered directly.
cargo test (70 + doctests), clippy, fmt, Miri on x86-64/s390x/powerpc/i686, and
coverage-guided fuzzing of all three targets (number_str now reaches the decimal
paths) all pass.
`kinds_classify_and_avoid_the_niche` used the literal "inline" as its inline string, which only fits inline on a 64-bit target: an inline string holds 7 bytes there but 3 on a 32-bit one. On 32-bit the string was interned instead, so `kind()` was reading a pointer as inline bits — the i686 Miri jobs have been red on this branch since the test landed. Derive it from the representation's own `CAPACITY` instead, which both fixes the classification and makes the case the *longest* inline string on whichever target is being built, so it tests the boundary rather than an arbitrary short string.
`F64Repr` was given a decimal-point flag so that an integer past `u64` which happens to be exactly an `f64` (`1e20`, `2^64`) could be stored as one without becoming a float. That put cross-group canonicalisation in the storage layer, which is the wrong home for it. A number belongs to one of two groups — written with a decimal point, or without — and that is not a property of its value. Choosing a representation may only pick the cheapest home *within* a group; it must never move a number between them. Deciding that `1e20`, `100000000000000000000` and the `f64` `1e20` are one number is `NumVal`'s job, and `NumVal` has no decimal point to preserve. So `F64Repr` goes back to a bare `f64` (it is always a float), an integer beyond `i64`/`u64` always goes to the decimal representation, and the reduction in `NumVal` becomes *complete*: `reduce_fixed` now also recognises an exact `f64`, so a value decodes to the same variant however it was stored. `canonicalise` and `from_big` share that one reduction. Without it, `2^64` parsed as an integer literal and the same value handed in as a Rust `f64` compared *equal* — the cross-variant comparison is exact — but hashed differently, so a `HashMap` would have held the same number twice. That is now a test. The exactness test is a cheap rejection (significant bits, or divisibility by a power of five) in front of the existing correctly-rounded-conversion-and-compare, so the reduction stays quick on the path it actually runs on: every comparison and hash of a stored decimal. Tests, clippy and Miri pass in both configurations, on 64- and 32-bit.
`codegen.rs` asserts a coarse property across the whole library (nothing dispatches through a vtable). This is its counterpart at the other end of the scale: a handful of basic operations, checked down to the instruction. Constructing a short string, a small number, a bool or `null` should be *free* — the value lives in the pointer word, so the constructor is pure arithmetic and, for a compile-time input, ought to fold away entirely. Each probe must compile to a single `ret` of the encoded word: no call, no branch, no allocation. Pinning the words also pins the bit layout from the outside. Until now the encoding was only ever asserted by code sharing the very constants it was testing, so it could not catch a layout that agreed with itself; here the word is read back out of the compiler and decoded against the layout as documented. It records some things worth not losing quietly — that the integer `0` encodes as 0xF8 rather than the reserved all-zero niche, that the empty inline string is likewise non-zero, and that the two inline number representations differ only in the float (`0.5` as `1 * 2^-1` or `5 * 10^-1`). Needs fat LTO, because the constructors are not `#[inline]` and so export no MIR: without it the probe crate can only call them and there is nothing to fold. So the test writes a small `cdylib` probe crate (whose `#[no_mangle]` exports survive LTO as roots), builds it against this one, and reads the emitted IR. Deliberately strict, and says so: a failure that shows a different constant means the layout moved, and anything other than a lone `ret` means building a small value now costs real work at run time.
Both codegen tests work by asking the compiler to emit IR for a fresh build and reading it back, so that IR has to be the library's own. Anything the outer build injects through the environment ends up in it — and under `cargo llvm-cov` (the coverage job) `RUSTFLAGS` carry `-C instrument-coverage`, which puts a profiling counter at the top of every function. The nested build inherited it, so the constructions that should fold to a constant began with an `atomicrmw` on a profiling counter instead, and the new test reported the instrumentation as a regression. Strip the inherited flags when spawning the nested `cargo`, in one shared helper both tests use, so neither can pick the habit back up.
Removing RUSTFLAGS/CARGO_ENCODED_RUSTFLAGS was not enough: the coverage job's instrumentation still reached the nested build. Set RUSTFLAGS empty rather than removing it (which also shadows any config-file build/target rustflags), drop the encoded and CARGO_BUILD_* forms, the per-target form, and any rustc wrapper. And make the failure legible if flags get in regardless: the test now detects instrumented IR — every function bumping a profiling counter, so nothing folds — and reports it as that, along with the environment it actually saw, instead of listing every probe as a codegen regression.
Each integration test compiles tests/common separately, so a helper only one of them uses is dead code in the other.
The constant-folding test only covers what the compiler can see at
compile time. These operations — converting a number, asking a value's
type — take a value whose representation is not known, so they cannot
fold away. What they can do is stay free of the costs that make them
slow, and this asserts they do: no vtable, no allocation, no panic path,
and nothing left out of line that should be inline.
It is an allow-list, not a list of forbidden things. A fast path should
call nothing at all, the exceptions are few enough to name, and naming
what is *allowed* means a new kind of cost cannot slip through unlisted.
The exceptions are `INumber::to_f64_lossy`'s unwrap (an assertion of the
type's own invariant, cold, and better than a default that would paper
over the bug) and, with `arbitrary_precision` only, the numeric
conversions calling into the bignum reduction — real work, rightly out of
line.
Writing it turned up three regressions, all invisible from behaviour:
- `ReprTag::with` panicked on the decimal tag when the feature is off.
Unreachable, but the compiler cannot know that, so it put a panic and
the `Arguments` it formats into *every* value operation. A
`debug_assert!` over a total fallback says the same thing and
generates nothing; `has_decimal_point` goes from 41 instructions to
23, and the numeric conversions lose their panic paths and stack
slots entirely. (I introduced this one, chasing purity without
looking at the output.)
- The same mistake, pre-existing, in the inline constant decode — whose
impossible arm is on the path of `IValue::type_`, so `is_number` and
`type_` carried a panic too.
- `From<f64>`/`From<f32>` used `unwrap_or(IValue::NULL)`, which builds
the `NULL` whether or not it is wanted and then, since an `IValue`
owns what it points at, has to *drop* it again on the ordinary path.
A live temporary, and enough to stop the conversion folding at all.
Each is verified to fail the test when put back.
CI (Linux, where more got inlined into view) showed `to_f64_lossy` calling `panic_const_div_by_zero`. Every division in the numeric code took a plain integer divisor, and the compiler cannot see through `pow` that a power of ten or five is non-zero — so each one carried a divide-by-zero check, and dragged the panic machinery into conversions that cannot fail. Five such panics in the arbitrary-precision build. Say it in the type instead: `bigint::div_small`/`rem_small` take a `NonZeroU64`, and the `Decimal` arithmetic takes its divisors from `const` tables of `NonZero`s. The fact becomes structural and the checks disappear — zero divide-by-zero references in the emitted IR now, in either configuration. The signed divisions go through unsigned and put the sign back afterwards. A signed `NonZero` would not have helped: `Div` for a `NonZero` divisor is only defined on the unsigned integers, and it still admits `-1`, so the `MIN / -1` overflow check and its panic would have survived anyway. Also relaxes one assertion the same CI run showed to be platform-dependent: whether LLVM inlines the small arms of the numeric model differs by target, so the fast-path test no longer pins that. It still pins what is durable — no vtable, no allocation, no panic, and nothing called outside the numeric model.
Without `arbitrary_precision` there is no decimal representation to dispatch to, and the arm I left there sent the tag to the `f64` representation instead. That is not a harmless choice among unreachable options — it is a *wrong* one: were the tag ever to appear, it would decode some other representation's allocation as a float and hand back a plausible-looking number. A wrong answer is worse than none. It cannot `panic!` either: this is the single dispatch every value operation goes through, and an unreachable arm that panics puts a cold panic block, and the `Arguments` it formats, into every one of them — which is what the previous commit removed. So state what is true. The tag is unreachable, and provably so: a value's tag is only ever *named*, since `new_usize`/`new_ptr` take a `ReprTag` argument and `set_ptr`/`set_usize` keep the tag already there. The only site naming `NumberDecimal` is `DecimalRepr::store`, in a module compiled out without the feature. Nothing left in the build can produce it, whatever the input. The `debug_assert!` keeps the check where checks are affordable — the tests and Miri run with them on — and the codegen is a shade better than the fallback besides, since the tag drops out of the switch entirely.
The fast-path test only said what must *not* be in the generated code —
no vtable, no allocation, no panic. Within that, anything could be
emitted, and something was.
An operation's fast path cannot be seen from outside the crate: the
representation is a run-time fact, so the compiler emits the whole
dispatch and every arm's code together. So tell it. `ijson::codegen_probes`
(behind `--cfg codegen_probes`, which only the tests' nested build sets,
so it does not exist in an ordinary build) hands the compiler the one fact
it is missing — *which* representation, never *which value* — with
`assert_unchecked`. The dispatch then folds away, and what is left is the
fast path alone, asserted instruction by instruction. The slow paths stay
free to be anything.
Reading a small integer out of an `INumber` is now pinned to exactly:
to_i64 load, ptrtoint, ashr, insertvalue, ret
to_f64_lossy load, ptrtoint, ashr, sitofp, ret
is_number ret (a constant)
type_ ret (a constant)
has_decimal_point ret (a constant: an integer has none)
Two things were wrong, both only under `arbitrary_precision`, and both
invisible from behaviour:
- Reading a plain inline integer — the commonest operation there is —
went through a *call* to `NumVal::from_decimal`, returning a 24-byte
`NumVal` through a stack slot. The function is large, so LLVM would
not inline it, and the hot branch (a plain integer, at exponent zero)
never folded. `#[inline]` on it and on `decimal_to_f64_lossy` collapses
both to the shift above.
- `to_f64_lossy` tried the *exact* conversion first and fell back to the
correctly-rounded one. But correctly rounded already means the nearest
`f64`, which for a value that is exactly one *is* that value — so the
exact attempt could only reach the same answer by a longer road, and
it is the more expensive of the two: it analyses the significant bits
of an `i128`, on every call, including the integers that are nearly all
of them.
The probes also had to be split into two crates. A function LLVM sees
called from one place is inlined unconditionally; from two it weighs the
cost. Two probes sharing a callee therefore change each other's codegen —
and when `to_i64` stopped inlining into the older probe as a result, that
test went quietly *vacuous*: a probe that merely calls out has nothing in
it to object to.
Both regressions above are verified to fail the test when reintroduced.
Miri has been warning on every run — and CI printed the warnings and passed. Two things wrong. The casts. Building an `IValue`'s word did `(tag | payload) as *mut u8`, and reading it back did `ptr as usize`. But that word is not a pointer: it is an inline value's bits, or the empty form of a collection, and it is never dereferenced. An integer-to-pointer cast claims the opposite — it asks for whatever provenance happens to be lying around — and that is not a pedantic distinction, because once Miri has to *guess* which allocation a pointer came from, it stops being able to tell a genuine aliasing violation from our tagged-pointer arithmetic. The warning was Miri saying it had been switched off. So say what is true: `without_provenance_mut` to build the word (it points at nothing and may alias nothing), and `addr` to read it (the bits, and nothing else — an `as` cast additionally *exposes* the provenance, offering it up to some later cast, which an inline value does not even have). And the gate. `-D warnings` cannot reach this: it is a run-time Miri diagnostic, not a lint. `-Zmiri-strict-provenance` makes it a hard error instead, and is now set on all three Miri jobs — verified to fail if the cast is put back. `RUSTFLAGS: -D warnings` covers the ordinary lints at the workflow level; cargo caps lints at `allow` for registry dependencies, so it denies our own code and stays quiet about theirs. Miri also runs twice as fast now, which is the same fact from the other side: it no longer has to track an exposed-provenance set at all.
Under a 32-bit Miri run both conditions held — Miri cannot shell out to a compiler, and the pinned words are the 64-bit little-endian encoding — so the test carried two `#[ignore]`s, and the second did nothing. That is a warning, and CI now denies those; it is what `-D warnings` caught the moment it was turned on.
Each was correct today but rested on a fact held somewhere other than where it is relied on — so a reasonable future edit could break it silently. Pull each guarantee into the structure. - Array allocation was sized `Layout::array::<usize>` while it stores and writes `IValue`s (the object layout already uses its element type). It works only because `IValue` is one word — a fact that lived in a doc comment. Size it as `IValue`, and add `const _: () = assert!(size==word)` at the type, so adding a field is a compile error, not a heap overflow. - `InternedRepr::intern` had no length precondition, yet string equality and hashing are pointer comparisons that are correct *only* because a short string is always inline and a long one always interned. Interning a short string would make two `IString`s for it compare unequal. Add a `debug_assert` that the string is too long to fit inline. - The `Decimal` variant's domain was declared in `numeric` but guaranteed by `number_decimal`, linked by prose. And `from_decimal` indexed a divisor table *before* its `fits_decimal` check. Export the bound from the value model that owns it, assert the check *first* (it holds on the raw input iff on the canonical form), and add a `const` assertion in the inline representation proving its encoding stays in-domain — so widening the mantissa or exponent fails to compile. - `is_true`/`is_false` compared the tag-masked word, resting on "no allocation lives at address 96". Compare the raw word instead: a heap value's tag is non-zero, so it can never collide with an inline constant. Strictly stronger, and removes the assumption. - `IObject::header` was a *safe* method wrapping an unsafe read whose real precondition (allocated, not the empty form) went unstated, while its `header_mut` sibling was `unsafe`. Make it `unsafe` too with the precise contract, so a caller cannot reach for it on an empty object unawares.
`arbitrary_precision` enabled only serde_json's write side (`raw_value`),
so exactness was a property of *which door* a number came through, not of
the value. `"0.1".parse::<INumber>()` was the exact decimal; but
`serde_json::from_str::<INumber>("0.1")` was the rounded `f64` — unequal,
and hashing differently, from identical text. And a magnitude past `f64`'s
range (`1e400`) parsed by `from_str` could be *written* but not read back:
`serde_json::from_str` rejected it outright.
The cause: serde_json, without its own `arbitrary_precision`, rounds every
float to `f64` before ijson ever sees it. Enable `serde_json/arbitrary_precision`
so it hands the raw literal over instead — as a one-entry map keyed by its
private token — and intercept that on both sides:
- Deserialize: `NumberVisitor`/`ValueVisitor` peek the token and re-parse
the literal with `INumber::from_str`, the same parser `str::parse` uses.
Integers in `i64`/`u64` range still arrive through `visit_u64`/`i64`.
- Serialize-into-ijson (`to_value`): a `serde_json::Number` serializes
*itself* as that token map, so the `Serializer` mirrors the
interception — a one-field `{token: "<digits>"}` becomes a number, not
an object with a magic key. Without this, round-tripping a
`serde_json::Value` turned every number into an object.
- `From<serde_json::Number>`: routed through `from_str` on the literal
too, so a direct conversion keeps the value exact and the dead
`f64::MAX` clamp (which would have silently corrupted an out-of-range
magnitude the moment such a `Number` existed) is now an unreachable
backstop.
The single invariant, now tested in both configs: `str::parse` and
`serde_json::from_str` are two doors to one parser and must agree, and
every value round-trips through JSON unchanged. Under the feature that
means exact; without it, both round to `f64` as before. (The one
remaining, pre-existing difference — the JSON token `-0`, which the
grammar-strict parser reads as the integer 0 and serde_json as `-0.0` —
is a difference of presentation, not value, and is called out where it
is asserted.)
Running the `json` fuzz target under `arbitrary_precision` — the round-trip and cross-parser surface the previous commit reworked — turned up a panic on the first corpus entry with an absurd exponent. `JsonNumber::significand` computed `written_exp - frac_digits.len()` to place the decimal point, and `written_exp` is already saturated near `i64::MIN` for a literal like `1e-99999999999999999999`, so the subtraction overflowed (a debug panic; a wrong, wrapped exponent in release). Saturate the subtraction. The `MAX_EXP` bound rejects the result either way — such a value is not representable and falls back to `f64` — so saturation only keeps the intermediate well-defined. Regression test covers both configs, since the grammar parser underlies both.
…sion Fuzzing found `441044444333116.125` failing its round trip. It is an exact `f64`, so it is stored as a float and was serialized with `serialize_f64` — which emits the *shortest* decimal that rounds back to the same `f64`, here `441044444333116.1`. That is correct only when the reader also parses through `f64`. Under `arbitrary_precision` the reader is exact, so it read `441044444333116.1` literally, as a different number (the decimal, not the `f64`). Under the feature, a float must therefore be written from its *exact* value too — the unique decimal that an exact reparse maps back to that `f64`. `exact_json` now renders `Float` (not just `Decimal`/`Big`), and `Serialize` routes every non-integer through it before falling back to `serialize_f64` (which stays correct without the feature, where the reader is `f64`-based). Two details the exact rendering needs: an `f64`'s exact magnitude carries many factors of ten (`0.5` is `5 * 10^52 * 10^-53`), so they are divided out or the text would be a run of trailing zeros; and zero — always `Int(0)`, and the one value `render_decimal`'s non-zero-magnitude contract excludes — is left to serde, which writes `0`/`0.0` exactly. Regression cases (`441044444333116.125`, `0.0`, `-0.0`) added to the deserializer round-trip test.
The `is_true`/`is_false` fix compared the raw word against an inline constant, and I had added a `raw_word()` accessor for it. But that is exactly what `raw_eq` already does — bit equality of the whole word — so `self.raw_eq(&Self::TRUE)` says the same thing through machinery that already exists, and reads as what it means: "is this the TRUE value". It keeps the strictly-stronger property (a heap value's non-zero tag can never match an inline constant's word), needs no new method, and compares against the `IValue::TRUE` constant rather than the lower-level `inline::TRUE` payload. `raw_word()` is removed; `repr_tag` reads the address directly again.
From an independent audit of the crate's comments, especially the safety
documentation. Each finding was verified against the code.
Safety contracts that were missing:
- `alloc.rs`'s three `unsafe fn` primitives (`alloc_infallible`,
`realloc_infallible`, `dealloc_infallible`) had no `# Safety` section at
all, yet each has a real precondition every representation relies on
(non-zero layout size; `ptr` currently allocated with the given layout;
matching alignment on realloc). Their callers were discharging a
contract the callee never stated. Now documented.
- `IValue::raw_copy` is `unsafe` because it duplicates the tagged word
without adjusting ownership — for a heap-owning value the two copies
must not both be dropped — which its siblings (`new_usize`/`set_usize`/
`ptr`) all state and it did not. Now documented.
- The `unsafe` blocks in the array/object `alloc`/`realloc`/`dealloc`
carried no `// Safety:` note, unlike the identical ones in
`scalar`/`interned`/`decimal`. Added, to match.
Comments that were inaccurate:
- `number_repr_key`'s doc claimed equal keys mean bit-for-bit identical
storage; for a multi-limb `arbitrary_precision` decimal it reads only
the 8-byte header, not the limbs. Scoped the claim to the inline and
heap-scalar reps the tests actually feed it, and said so.
- The crate-level feature list documented only `ctor`, omitting
`arbitrary_precision` — a headline feature that changes number
semantics — and `indexmap`/`broken-borrow-impl-compat`/`tracing`. Added.
- Two module docs linked to the private `crate::value::{array,object}`
modules, a broken public-doc link (`cargo doc` with
`-Drustdoc::private_intra_doc_links` errors). Made them code spans.
And one real bug the audit turned up: `impl Debug for VacantEntry` called
`debug_struct("OccupiedEntry")`, so a vacant entry printed under the wrong
name. A copy-paste from the `OccupiedEntry` impl; corrected.
From an independent privacy/redundancy audit; each change is compiler-verified (`-W unreachable_pub`, and both feature configs build, test, and lint clean). - `alloc`, `thin`, and `ser` are private modules but declared their contents with bare `pub` — 14 items (`alloc_infallible` and friends, the `ThinRef`/`ThinMut` machinery, the serde `Serializer` plumbing). They were already crate-capped by the private module, so the `pub` was misleading, not a leak: it reads as public API. `unreachable_pub` flags exactly these; now `pub(crate)`, matching the rest of the crate. (`to_value`, genuine public API, stays `pub`.) - `scalar::read` was the lone `pub(crate)` among its siblings (`alloc`/`free`/`layout` are private). The scalar reps that use it are child modules that see it regardless; the only other caller is a `#[cfg(test)]` method in the parent `value`. `pub(super)` serves both and says so. - `JsonNumber`'s `int_digits`/`frac_digits`/`written_exp`/`shape` are read only inside `inline/number.rs` (through `significand`/`shape`); only `negative` is read by a caller. The four are now private, matching the crate's private-fields-plus-accessor pattern. Two audit findings deliberately left: tightening `bigint`'s `pub(crate)` to `pub(super)` (already crate-internal — churn without real surface reduction), and deduping the one-line `i64_fits_f64` predicate (would widen `numeric`'s API and couple the deliberately-independent inline number representations to remove a single line).
The reference count — a hand-rolled `Arc` — used `Relaxed` on every
operation, including the decrements and the final decrement that frees.
The freeing thread must see every other owner's reads of the shared bytes
happen-before it deallocates them, and `Relaxed` provides no such edge.
Reachable from safe code (`IString`/`IValue` are `Send + Sync`): one
thread reads `as_str()` then drops via the lock-free fast path while
another drops the last reference and frees; with all-`Relaxed` the read
and the free are unordered — a data race of the use-after-free class.
x86-TSO masks it; ARM/POWER need not.
The fix uses the shard lock, which a plain `Arc` does not have:
- Fast-path decrementers never take the lock, so their reads are ordered
before the free only by making the decrement `Release` and having the
freeing decrement `Acquire` it — that one pair.
- Slow-path decrementers run under the shard lock, and so does the free,
so their reads are already published to the freeing thread by the
lock; those decrements need no atomic ordering of their own (including
the resurrection case, where a concurrent `intern` upgraded the entry
and `fetch_sub` returns > 1).
So: the fast-path CAS decrements with `Release`, the slow-path `fetch_sub`
with `Acquire`, and the increments stay `Relaxed`. No API change.
Also fixes five doc-comment typos surfaced by the API review (thie,
numberic, accomodate, unimporant, returend).
The README's four graphs are rendered from comparison.csv, which had drifted to an older corpus and an older representation. Regenerate both from the current branch: fresh dummy-json data run through the comparison example, and the four PNGs replotted from that CSV. The clone-allocations graph is where the inline-numbers work shows: ijson's clone allocations now stay roughly halved across the corpus, since a clone of an inline number is a bit-copy rather than an allocation.
`to_value`/`from_value` are a public API, but the suite only ever drove the dynamic serde path (serializing a serde_json::Value, deserializing back into one) — both go through serialize_map/deserialize_any and nothing else. The typed methods (serialize_struct, the four serialize_*_variants, ObjectKeySerializer, and on the read side deserialize_struct/deserialize_enum, the typed deserialize_i8..f64, EnumDeserializer, ArrayAccess, ObjectAccess) only fire when a derived type drives the (de)serializer, so they had almost no coverage: ser.rs and de.rs sat near 28% line. Add a round-trip test module over a derived type touching every scalar width, options, tuples, tuple/newtype/unit structs, all four enum variant shapes, sequences, nested structs and string-keyed maps, plus targeted tests for non-string map keys, rejected keys, ignored unknown fields, and type-mismatch errors. This lifts de.rs to 71% and ser.rs to 65% line (merged over both number features), and gives the round-trip API a real correctness check. serde's `derive` feature is added as a dev-dependency only; it unions onto the existing serde dependency for test builds and leaves the published library's footprint unchanged.
The wrapper types' public surface was only incidentally exercised: whatever the deserializer happened to call. Whole method families had no test — IObject's Entry API, iterators, retain, clear, Index/IndexMut, map conversions and Extend; IArray's slice traits (Deref/Borrow/AsRef), clear, PartialOrd, From<&[T]>, by-ref iteration and Default; IString's From/PartialEq conversions, Ord/Hash and Debug/Display; and every trait value_subtype_impls! generates (AsMut/Borrow/BorrowMut/TryFrom), plus the ijson! construction macro. Add focused tests for each. The IObject/IArray/IString tests are in-module and use mockalloc, so the unsafe data-structure paths are also checked for leaks; the conversion/macro tests are a small integration module over the public API. Merged over both number features this takes array.rs and macros.rs to 100% line, string.rs to 97%, object.rs to 93%, and the crate from ~83% to ~88% line. All new tests pass under Miri in both feature configurations.
The existing `json` target drives only the dynamic serde path — `IValue`'s own `Serialize`/`Deserialize` through `serde_json` — so it never reaches the typed machinery (`ValueSerializer`, `ObjectKeySerializer`, `Deserializer for &IValue` and its enum/variant/map/seq access) that only a derived type exercises. A corpus of valid JSON structurally cannot cover those paths. Add a `serde_roundtrip` target whose payload is both `Arbitrary` (so the fuzzer builds it directly) and `Serialize`/`Deserialize` (so it drives the typed path), asserting `to_value` then `from_value` recovers an equal value. The payload spans every integer width, bool/char/string, option, tuple, sequence, a nested struct, all four enum-variant shapes, and a string-keyed map. Types are chosen for an exact round trip: no floats (a non-finite f64 is unrepresentable), and only string-keyed maps (integer keys serialize but do not parse back). Map values are integers and every single-key object is an enum variant, so the arbitrary_precision number-token interception cannot misfire — the target is correct under both feature configurations. Ran 200k iterations clean. serde's `derive` feature is added to the fuzz crate's dependencies for the payload type.
The typed serde surface was thin: nothing serialized an IValue (or IString/
IArray/IObject) to JSON text through serde, deserialized the subtypes back,
used an IValue as a Deserializer for arbitrary types, exercised the enum and
type-mismatch error arms, or touched the bytes paths — so ser.rs/de.rs sat
around 65%/71% line.
Add:
- tests/serde_value.rs: serializes an all-types IValue to JSON text (every
Serialize-for-IValue arm plus the subtype impls), deserializes each subtype
back (including the escaped-string path and each visitor's `expecting`),
uses from_value for every JSON type and typed targets, and drives the enum
and MaybeUnexpected error arms.
- tests/serde_bytes.rs: the bytes paths, which JSON never reaches —
serialize_bytes (bytes -> array of byte values), deserialize_bytes/byte_buf
dispatch, the ObjectKeySerializer bytes-key rejection, and StringVisitor's
visit_bytes/visit_byte_buf including the invalid-UTF-8 rejection (reached
with byte-oriented deserializers).
- serde_derive.rs: the map-key shapes that legitimately stringify (assorted
integer widths, a unit enum variant, a newtype struct), a representative
rejection per non-scalar key shape, and struct-from-scalar errors.
Merged over both number features this takes de.rs to 85% and ser.rs to 83%
line, and the crate to ~91% line / ~91% region. What remains is the identical
one-line ObjectKeySerializer arms for key types that cannot occur, and a few
serde visitor methods reached only by binary formats. serde's `derive` and
`serde_bytes` are dev-dependencies only. All new tests pass under Miri in both
configurations.
The corpus was gitignored (the cargo-fuzz default), so every `cargo fuzz run` and any CI fuzzing started cold. Track a seed corpus instead: it gives regression coverage (interesting inputs found over time are preserved and re-checked) and lets a fresh run reach deep states immediately. Commit the *minimized* corpus (`cargo fuzz cmin`), not the raw one: cmin keeps a minimal set of inputs that preserves the same coverage, cutting the on-disk corpus from ~21k files / 47 MB to ~3.4k files / 1.2 MB. This includes a seed for the new `serde_roundtrip` target. Corpora are arbitrary bytes, so a .gitattributes marks `fuzz/corpus/**` as `binary` (no EOL/text normalization, which would corrupt them regardless of a contributor's autocrlf setting) and `linguist-generated` (GitHub collapses them in diffs and excludes them from language stats).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #24. Stacked on #42 (base is the inline-short-strings branch); the two are intended to merge together.
Reworks the
IValuerepresentation so thatnull, booleans, small numbers, and short strings are stored entirely inline in the pointer-sized value, with a widened 3-bit tag. The exact-decimalarbitrary_precisionmode is included, and the surrounding representation, correctness, test-coverage, and fuzzing work landed alongside it.Tag scheme (
ALIGNMENT = 8)The
Inlinefamily (0) holds null/bool/number/short-string; tags 1–7 are dedicated pointers. Tag 4 (heap decimal) is used only witharbitrary_precision.STATIC_NUMBERS,NumberType, and the i24/heap number header are deleted.Inline numbers =
mantissa × base^expA small number packs directly into the pointer-sized
IValue(tag 0, number bit set) asmantissa × base^exp, with a 56-bit signed mantissa (24-bit on 32-bit) and a 4-bit exponent code. The base depends on the feature, and the two inline representations (number_binary.rs/number_decimal.rs) deliberately share no code so their bit layouts can diverge:exp ∈ -7..=7, so every inline number is exactly anf64. Anf64is decomposed tomantissa × 2^exp(trailing zeros factored into the exponent) and inlines when both fit; there is no non-f64inline value.arbitrary_precision: base 10 — an exact decimalmantissa × 10^exp(see the section below).Either way this inlines the overwhelming majority of real JSON numbers — integers, timestamps/ids (which used to cost a heap
i24/i64), and short fractions like0.5,2.5,63.5— with no allocation and no indirection. What doesn't fit spills to a bare 8-byte heap payload whose type is the tag: an integer beyond the mantissa toi64/u64(tags 1–2), and a float outside the inline exponent range (0.1,π) tof64(tag 3, or aDecimalat tag 4 under the feature) — down from the old 16-byte header+payload.Key design points (co-designed in the issue thread):
has_decimal_pointis carried in the exponent field via two exponent-0 codes (2vs2.0), using the fact that a decimal-pointed integer can only ever fit inline at exponent 0.NonNullniche — every inline number is kept non-zero (the base-2 rep sets a dedicated number bit) — preservingsize_of::<Option<IValue>>() == size_of::<IValue>().arbitrary_precision(opt-in) — now includedThe opt-in feature stores a JSON number's exact decimal value rather than rounding to
f64, by swapping the inline base from 2 to 10: the same inline slot instead holdsmantissa × 10^exp, so0.5→5×10⁻¹and0.1→1×10⁻¹both inline exactly. A larger or more precise number spills to a heap arbitrary-precision decimal (tag 4). With the feature on,"0.1"is the exact tenth — a different number from thef640.1— and magnitudes beyondf64's range are representable.The load-bearing invariant is that all number text passes through one parser. Deserialization routes through serde_json's own
arbitrary_precision(its raw-number token hands over the literal instead of eagerly rounding), which is then parsed by the sameINumber::from_strthatstr::parseuses — soserde_json::from_strandstr::parsecannot disagree, and an exactly-representable value stays exact through a round trip. Serialization mirrors it: an exact-f64float is written from its exact digits (not its shortestf64rounding) so an exact reparse reads back the same number. Without the feature the default is already correct — short decimals inline exactly via thef64decomposition, andhas_decimal_pointfollows the int-vs-float deserialize path.Representation & dispatch refactor
The value dispatch is split into per-type
Reprtraits and routed by continuation-passing over the tag (ReprTag::withhands each arm's concrete representation to a closure), which keeps every operation a direct call — returning a single&dyn ValueReprfrom a match instead merges every arm's vtable into a phi the optimizer can't see through, silently costing an indirect call per operation. A codegen test builds the library to LLVM IR and asserts the dispatch stays devirtualized (no ijson function calls through a function pointer, save the one deliberate&mut dyn Hashererasure); companion probes pin the exact fast-path IR for constructing and reading small values, so a regression to a vtable or an extra load fails CI rather than passing silently.Correctness hardening
Arc; its free is now correctly ordered against the lock-free fast-path decrement (Release on the unprotectedcompare_exchange, Acquire on the shard-lockedfetch_sub), with the shard lock publishing slow-path decrementers' reads to the freer. Verified by two independent reviews and under Miri.IObject'sPartialOrd/PartialEqsoa == b ⇒ a.partial_cmp(b) == Some(Equal)holds for value-equal objects in distinct allocations — including nested inside an array, where the break also panickedsort_by.INumberfiniteness is enforced at thenew_f64boundary; the numberDeserializeris total over the numeric model; divisors are taken asNonZeroso scaling carries no panic.unsafesafety contracts, and an exponent-overflow fix in the exact-decimal parser found by fuzzing.Testing & fuzzing
IObject/IArray/IStringAPIs and generated conversions, and the full serde bridge — serializing anIValueto JSON text,to_value/from_valuefor derived types, the enum and type-mismatch error arms, and the bytes paths (which JSON never reaches).serde_jsoncross-check), number parsing/ordering/f64construction, and a newserde_roundtriptarget that fuzzes the typedto_value/from_valuepath with anArbitraryderived payload. A ~213M-execution session across all targets found no crashes. A minimized seed corpus (cargo fuzz cmin, ~3.4k files / 1.2 MB) is committed so runs start warm;fuzz/corpus/**is markedbinary linguist-generated.Validation
Full platform/endianness matrix, in both feature configurations:
clippy/fmtcleanNew number tests cover the inline/heap boundaries, integer factoring,
i64/u64extremes, negative short decimals,2 == 2.0with differinghas_decimal_point, and ordering across all representations. (Miri surfaced a real fragility — reliance onf64::powi, which it makes non-deterministic — now replaced with exact integer-power scaling.)Notable landmine fixed
Materialising
IValue::NULL/TRUE/FALSEinside thetype_()/Dropdispatch path caused infinite recursion (the temporary'sDropre-enterstype_). The hot path now compares against plain bit constants.