test(json): adversarial QuickCheck roundtrip and parser-robustness tests by bobzhang · Pull Request #4045 · moonbitlang/core

bobzhang · 2026-08-14T08:21:22Z

Summary

Adversarial QuickCheck property tests for the json parser and stringifier, complementing json/quickcheck_test.mbt. This suite found the bugs now tracked issue-first and fixed in dedicated PRs:

json: parser aborts on strings containing lone surrogates combined with escapes #4049 — raw lone surrogates aborted the process (fixed in fix(json): reject raw unpaired surrogates in string lexing instead of aborting #4056, base main)
json: parser constructs ill-formed strings from unpaired \uXXXX escape sequences #4062 — unpaired \uXXXX surrogate escapes manufactured ill-formed strings (fixed in fix(json): reject unpaired \uXXXX surrogate escapes in strings #4064, base main)
json: parse("-0") loses the sign of negative zero #4053 — parse("-0") lost the sign of zero (fixed in fix(json): preserve the sign of -0 in the integer fast path #4061, already merged)

Merge order: this PR should merge after BOTH #4056 and #4064. It is based on agent/fix-json-lexer (#4056) and retargets to main when that merges. On the current branch (which has #4056's fix and the merged #4061, but not #4064) exactly two tests fail, both needing #4064:

"lone surrogates are rejected in every position" — the escaped (\uXXXX) spelling of a lone surrogate is still accepted without fix(json): reject unpaired \uXXXX surrogate escapes in strings #4064;
"surrogate handling pins" — the escaped-rejection assertions.

All other 209 tests are green on wasm-gc, js, and native; the suite goes fully green once #4064 is in the base history.

New property families (`json/quickcheck_adversarial_test.mbt`)

Adversarial strings — every control character, JSON syntax characters, BMP boundaries, astral pairs (Unicode scalar values only) — roundtrip through stringify/parse as values and object keys, across indent/escape_slash. The AdvString shrinker drops whole characters so candidates stay well-formed.
Fully \uXXXX-escaped spellings parse back to the exact original (pairs split across two escapes, mixed-case hex).
Lone-surrogate rejection (unicode-safe policy): a lone surrogate injected at any position — raw via stringify or spelled as an escape, with escapes/astral pairs/control chars nearby — is always rejected cleanly: parse raises the documented error, valid is false, nothing ever aborts. Deterministic pins cover raw, escaped, reversed-pair, and mixed raw/escaped-half spellings, plus still-accepted well-formed pairs.
Zero literals preserve the sign of zero bitwise (fix(json): preserve the sign of -0 in the integer fast path #4061, merged — passing).
Int64/UInt64 literals roundtrip textually (repr preserved beyond 2^53).
Whitespace insensitivity; duplicate keys last-wins; mutation totality (single UTF-16 code-unit deletion/replacement keeps parse consistent with valid, successful mutants are restringify-reparse fixed points); depth-1024 boundary pins.

Verification

moon check clean; moon info && moon fmt — no .mbti changes.
Current branch: 209/211 on wasm-gc, js, and native (identical two failures everywhere — see merge-order note).
Properties stress-run locally with seeds 1..20 and counts up to 2000 per property on all three backends; committed tests use CI-friendly default counts.

🤖 Generated with Claude Code

coveralls · 2026-08-14T08:24:52Z

Coverage Report for CI Build 6069

Warning

No base build found for commit ca31837 on agent/fix-json-lexer.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 90.663%

Details

Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	17671
Covered Lines:	16021
Line Coverage:	90.66%
Coverage Strength:	151096.45 hits per line

💛 - Coveralls

Copilot

Pull request overview

This PR strengthens the json package’s correctness by fixing two parser edge cases uncovered via adversarial property testing (lone-surrogate handling in string lexing and -0 sign preservation in number lexing), and adds a new suite of deterministic, CI-friendly QuickCheck properties to prevent regressions.

Changes:

Fix lex_string_slow to avoid aborting on lone trailing-surrogate boundaries by using bounds-checked view(...) instead of checked slicing.
Fix integer fast-path number lexing to preserve IEEE-754 negative zero by applying the sign after Int64 -> Double conversion.
Add adversarial property tests (strings/escapes/whitespace/dup keys/mutations/nesting limit) plus a targeted unit test for negative-zero parsing.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
json/quickcheck_adversarial_test.mbt	New adversarial QuickCheck suite exercising parser/stringifier robustness and edge cases.
json/lex_string.mbt	Prevents aborts when slow-path string flushing crosses lone-surrogate boundaries.
json/lex_number.mbt	Preserves `-0.0` sign in the integer fast path by negating after conversion to `Double`.
json/lex_number_test.mbt	Adds a regression unit test asserting negative-zero sign preservation across spellings/underflow.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

bobzhang · 2026-08-15T01:21:35Z

+  let deep_array = "[".repeat(1024) + "0" + "]".repeat(1024)
+  assert_true(@json.valid(deep_array))
+  fn outcome(text : String) -> String {
+    try {
+      ignore(@json.parse(text))
+      "parsed"
+    } catch {
+      DepthLimitExceeded => "depth limit"
+      _ => "other error"
+    }
+  }
+
+  let deeper_array = "[".repeat(1025) + "0" + "]".repeat(1025)
+  assert_eq(outcome(deeper_array), "depth limit")
+  let deep_object = "{\"k\":".repeat(1025) + "0" + "}".repeat(1025)
+  assert_eq(outcome(deep_object), "depth limit")


Fixed in 29009f6: the test now asserts depth-1024 documents parse successfully for both arrays and objects (assert_eq(outcome(deep_array), "parsed") / assert_eq(outcome(deep_object), "parsed")), and the 1025-deep variables are renamed too_deep_array / too_deep_object. The test now pins the documented boundary exactly on both sides for both shapes.

bobzhang · 2026-08-15T01:21:37Z

+///|
+/// Deleting or replacing one code unit of a valid document must keep the
+/// parser total: it either succeeds or raises a parse error (`valid` agrees
+/// with `parse`), and when the mutant still parses, the parsed value is a
+/// fixed point of restringify-and-reparse.
+test "parse stays total under single code-unit deletion and replacement" {


Fixed in 29009f6 by taking the stronger option: the test now mutates genuine UTF-16 code units via code_units() instead of Char-level to_array(). Deletion/replacement can now split an astral surrogate pair, and the replacement unit is drawn from the full 16-bit range (including lone surrogates). Verified green on wasm-gc, js, and native.

Every append in `Buffer` already guards capacity before writing, but the indexed writes that followed still emitted a full `idx < 0 || idx >= len` check plus a panic branch — the compiler does not propagate the guard. Replace those writes with `FixedArray::unsafe_set` in the 15 encoders where a preceding guard establishes the bound, and hoist `data`/`offset` into locals so each function loads the fields once instead of per byte. The same pattern was already used in `uleb128.mbt` / `sleb128.mbt`. Safety for each site rests on the documented buffer invariant `0 <= len <= data.length()`: the guard leaves `len + k < data.length()` for every `k` written, and `grow` aborts via `buffer_growth_capacity` when the size computation overflows, so a negative `required` cannot reach a write. `write_string_utf16le`/`be` additionally rely on `code_units()` yielding exactly `length()` items, which is what `required` reserves. Bulk `blit_*` calls and the `data[0:len]` slices in `contents()` / `to_bytesview()` are left untouched. Generated C for the package (native, release): 59 `moonbit_panic()` sites removed across the 35 emitted functions, total body size 2005 -> 1492 lines. Adds `buffer/write_bench_test.mbt` covering the append paths.

`Buffer(size_hint=n)` inside the timed closure charged every run a malloc plus a zero-fill of the whole capacity (128KB for the utf16le case), on freshly handed-back memory. That dominated the short benchmarks and made `write_char_utf8` on the ASCII branch read as a 50% regression, even though its inlined loop is 9 instructions per iteration against the baseline's 13. Allocate the buffer once outside the closure and rewind it with `reset()`. `from_array` keeps its allocation, since allocating and filling is what it does.

`buf.length()` is derivable from the fixed loop bounds, so keeping only that leaves the byte stores unobservable in principle and invites the backend to drop the loop under test. Keep the buffer itself instead. The emitted code shows the stores did survive in both baseline and patched builds — hoisting the buffer out of the closure already made it escape — and re-running the A/B moves every figure by less than two points. This is to stop the benchmarks resting on that.

The js to_octets ran its zero shortcut before validating the requested length, so 0N.to_octets(length=0) silently returned b"" while the documented contract (and every other target) panics on a non-positive length. Validate the length first, and align the docstring with the now-uniform panic contract. Fixes #4052 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The non-js to_uint/to_uint64 (and hence to_int/to_int64) computed the truncation of a negative value by adding one modulus and reading magnitude limbs. That is wrong whenever the sum is still negative, i.e. whenever |x| > 2^32 (resp. 2^64) — already -(2^32 + 1) was mishandled: (-(1N << 32) - 1N).to_uint() returned 1 instead of 4294967295, and (-(1N << 33) - 5N).to_uint() returned 5 instead of 4294967291; only exact modulus multiples and scattered coincidental residues came out right. js was already correct via BigInt.asUintN and is unchanged. Truncation to 2^k is a ring homomorphism, so for a negative value it is the wrapping negation of the magnitude's low bits; compute it that way. Fixes #4050 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Add a deterministic deep property suite for BigInt covering the large-operand paths quickcheck_test.mbt does not reach: - multiplication straddling the Karatsuba threshold (dense 47..53-limb sweep), cross-checked against sub-threshold chunk reassembly and shift-and-add closed forms with maximal carry chains - Knuth division with adversarial divisor top limbs (0x80000000 / 0xffffffff patterns) and all-ones quotient digits, plus the truncated division law across mixed size classes for all four sign combinations - shifts vs multiplication and floor division across limb boundaries, radix round-trips for every base 2..36, pow/modpow reduction, octet round-trips, parse-language agreement across targets All inputs derive deterministically from literal SplitMix64 seeds, so every target runs the identical value stream and any cross-target divergence fails the suite. Hunting with this suite found the two defects fixed in #4054 (issues #4050, #4052); this change is tests-only and stacks on that fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Pins `decode`, `decode_lossy`, and `encode` against an oracle transcribed from the Unicode 16.0 core spec rather than from the implementation: `lead_class` is Table 3-7 (the well-formed UTF-8 byte sequences), `scan` is D93b (the maximal-subpart rule that fixes how many U+FFFD a lossy decoder emits and where a strict decoder reports the failure), and `push_scalar` is the closed-form encoder. An independent oracle matters here because the package carries two implementations selected by target -- a hand-written scanner (decode_nonjs.mbt) and the platform TextDecoder/TextEncoder (decode_js.mbt) -- and the scanner and encoder are additionally `#intrinsic`, so a backend may substitute its own code generation for the MoonBit body. The spec is the only thing all of them must agree with. Coverage is exhaustive where the input space allows -- every 1- and 2-byte string (settling every overlong lead, every out-of-range lead, every bare continuation byte and every truncated 2-byte prefix), and every 3- and 4-byte string over an alphabet containing all of Table 3-7's range boundaries. Elsewhere it is property-based, with a generator biased toward near misses (boundary bytes, class-edge scalars, truncated encodings) rather than uniform random bytes, which would almost never form a valid multi-byte sequence and so would exercise only the reject-immediately path. Properties: oracle agreement for both decoders; encode/decode and decode/encode round-trips (the latter pins injectivity, which an accepted overlong form would break); decode_lossy output is always itself well-formed; lossy agrees with strict wherever strict succeeds; per-scalar byte lengths; concatenation homomorphism; BOM emission and ignore_bom as inverses; the `Malformed` offset contract under truncation; and view-offset independence. All 13 tests pass on wasm, wasm-gc, js and native. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`@range.iter`'s specification is "from, from + step, from + 2 * step, ... while the value is on the correct side of `to`", and the whole difficulty is in the words the implementation has to add to make that terminate on a fixed-width type: it stops as soon as `current + step` fails to make progress past `current`. The oracle here computes the sequence in exact, unbounded arithmetic (BigInt) and stops when the next value would leave the type's range. That is a genuinely independent statement of the same rule -- for a two's-complement type, "the exact next value is outside [min, max]" and "the wrapped next value fails to make progress" are the same condition, but the oracle never performs the wrapping arithmetic whose corner cases are under test. A reference written with the same fixed-width `+` would reproduce an overflow bug rather than catch it. Cases are generated in BigInt space and then narrowed to each Step type, so all eight integral impls -- Int, Int64, UInt, UInt64, Int16, UInt16, Byte and BigInt itself -- are checked against the one oracle. Generation concentrates where the overflow stop actually engages: `from` is drawn from the ends of the type's range and their neighbours, `to` is either a near neighbour or another anchor, and `step` is small, or scaled so a few hundred additions cross the range, or an anchor (which is how step = MIN_VALUE and a bare step = 1 across a full-width range get covered). Both sides are capped at 300 values so an unbounded case degrades to a prefix comparison rather than a hang. Three oracle-free properties restate the contract directly: the values form an arithmetic progression, strictly monotonic and never past `to`; an inclusive range is the exclusive one plus at most its endpoint; and negating the step retraces the same values in reverse. Float and Double cannot use the exact oracle -- repeated addition is not `from + k * step` -- so they are specified by invariant, plus a cross-check that Float and Double agree on exactly representable values, and regression cases for the two ways the loop could fail to terminate: a sub-precision step, and a NaN endpoint. All 15 tests pass on wasm, wasm-gc, js and native. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A lazy list has two specifications that must hold at once, and a bug in either is invisible to a test that only checks the other. The extensional half — what elements come out — is settled by an Array model: every combinator is mirrored by a three-line strict definition, and the two must agree. The properties drive whole randomly generated *pipelines* rather than single combinators, because the interesting failures in a lazy structure live in the composition (flat_map feeding take, concat feeding zip) and not in any one operation. Alongside that sit the algebraic laws: functor identity and composition, flat_map's singleton and annihilator cases, concat as a monoid, take/drop as a partition, take_while/drop_while splitting at the same point, filter idempotence and commutativity, and zip's two projections. The intensional half — how much is forced, and when — is what makes the structure worth having, and the doc comments state it precisely: take forces "exactly the cells of the prefix -- no look-ahead", drop forces "up to n cells", head "does not force the tail", tails memoize so a thunk runs "at most once", and concat does not touch its right-hand side until the left is exhausted. Those are pinned here as *exact thunk counts* against an instrumented infinite source. An implementation that quietly forced one cell too many would still pass every extensional test, but would turn take on an infinite list from a total function into a hang -- so the counts are asserted as equalities, not bounds, and a final property re-runs every lazy combinator against an infinite source to confirm it stays total. All 20 tests pass on wasm, wasm-gc, js and native. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…k DP The package had no test file at all -- only the doc examples. The definition of Levenshtein distance is a three-line dynamic program, and the oracle here is exactly that: the full (m+1) x (n+1) table, no trimming, no banding, no saturation. The implementation is none of those things. It trims the common prefix and suffix, rolls two rows instead of the table, swaps its arguments so the longer side comes first, and for the `*_within` entry points restricts the search to a diagonal band with Ukkonen bail-out and saturating arithmetic. Each of those is a chance to be wrong on a particular shape of input, so the suite checks the optimized code against the definition rather than against itself. Two things drive that comparison. Exhaustive sweeps settle whole input spaces outright -- every ordered pair of binary words up to length 6 (16129 pairs) and of ternary words up to length 4, each checked at *every* distance bound from -1 to one past the maximum, from both argument orders. That is the only way to be confident no band-edge case was missed, since the `lo`/`hi` clamping and the row sealing are where an off-by-one would hide. Property tests then carry the same checks to longer inputs. Alongside those sit the axioms that hold independently of the algorithm: Levenshtein is a metric, so it is zero exactly on equal inputs, symmetric, and obeys the triangle inequality; it is bounded below by the length difference and above by the longer length; a single insertion, deletion or substitution costs exactly one; and it is invariant under a shared prefix and suffix, which is precisely the assumption the trimming step rests on. Finally, `edit_distance_str` is pinned as the array API over `code_units()`, with the documented astral-character behaviour as explicit cases. Generators project onto a small alphabet on purpose: over unrestricted Ints two random sequences almost never share an element, the distance collapses to max(m, n), and the substitution-versus-insert/delete choice is never exercised. All 16 tests pass on wasm, wasm-gc, js and native. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`decode` has more than one implementation and picks between them at run time. On the non-JS backends it first runs a V128 pre-scan (utf16_needs_scalar_*_v128) looking for any surrogate code unit; if it finds none it takes a vectorized fast path that reinterprets or byte-swaps the buffer wholesale, and otherwise falls back to the scalar matcher. JS always takes the scalar path. The same input can therefore be decoded by three different pieces of code depending on backend and content, and they must all agree. The pre-scan is the delicate part: it decides correctness for the fast path, it processes sixteen bytes at a time with a scalar tail, and it runs from the *view's* start offset, which need not be even relative to the backing store. A pre-scan that missed a surrogate would hand a lone surrogate to the fast path and produce an ill-formed String rather than the Malformed the caller expects. So the oracle is an independent code-unit walk written from the UTF-16 definition, and the generators land on every boundary the pre-scan cares about. Coverage is exhaustive where it can be -- every 1- and 2-byte input in both byte orders (settling the whole single-code-unit space, including every lone surrogate and every truncated unit), and every ordered pair drawn from a surrogate-boundary alphabet, with and without a trailing odd byte. Two targeted sweeps sit alongside: a surrogate planted at every lane of every block for lengths 1..40, which is what would catch a pre-scan that skipped a position; and surrogate-free runs of every length in the same range, which drive the vectorized path and its scalar tail. Property tests add view-offset independence at paddings 0, 1, 2 and 15, so the pre-scan's blocks land on both parities of the code-unit grid. Round-trips pin encode/decode both ways, that the two byte orders are byte-swapped images of each other, that decode_lossy output is always itself well-formed, and that a BOM is stripped only when it matches the declared byte order (FF FE read as big-endian is U+FFFE, not a mark). Verified to have teeth: narrowing the oracle's low-surrogate bound by a single value is caught by three separate properties. All 14 tests pass on wasm, wasm-gc, js and native. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`decode_v128` recovered the offending input offset by dividing the *output* offset (`bytes[result_offset / 2:]`), even though the matched view already starts at the offending byte. Bind the vector arm with `as current` and let the catch-all bind the view directly, so both `Malformed` payloads come from the view being matched rather than from arithmetic on the write cursor. This is what `decode_scalar` already does. No behavior change — the existing quickcheck in `decode_v128_wbtest.mbt` compares the vector and scalar error views on arbitrary payloads at arbitrary view offsets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013wPGhicD5xMW9xH5Uk56iK

`utf16_needs_scalar_le_v128` and its BE twin each tested the first code unit for a surrogate before entering the scan loop. That test can never fire: - with >= 16 bytes left the first arm checks all eight code units of the block, including the first, via `utf16_v128_has_surrogate`; - with fewer, the `[u16le(0xD800..=0xDFFF), ..]` arm catches the first code unit itself. The check predates the bitstring rewrite, where the loop's own arms took over the job. Remove it from both functions. No behavior change; the oracle tests in `quickcheck_test.mbt` — including "a surrogate at every position of a vectorized block" — cover the first-code-unit case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013wPGhicD5xMW9xH5Uk56iK

ryu_to_string ran the full shortest-representation search for every non-zero double. Integral values in Int range - the common case in real workloads, JSON especially - now return Int::to_string directly, which renders the identical shortest form 3.46x faster (311.1us -> 89.9us per 10k values, native release). Json::stringify on a 10k-number document drops 345.2us -> 152.2us with no change to the json package. The guard admits exactly the doubles whose value round-trips through Int: NaN and infinities fail the range comparisons, -0.0 is already handled by the zero check above, and fractional values fail i.to_double() == val. JS is unaffected (ryu_to_string there is native number.toString()). Equivalence was verified differentially: a checksum over ~260k renderings (dense band, power-of-two neighbourhoods across the Int boundary, random integral/fractional values) is byte-identical to main. The new wbtest pins the boundary, special values, and nearest-to-integral doubles; removing the integrality check breaks 41 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014z81qJ5vdYoxZ48JTKCdAy

Extends the integer fast path from Int range to MAX_EXACTLY_REPRESENTABLE_INT (2^53), the largest double below which every integral value's shortest decimal form is its exact integer digits. This covers millisecond timestamps and 64-bit-ish IDs, common in real JSON: 350.8us -> 158.2us per 10k values. Two tiers keep the dominant small-integer band on Int formatting (93.2us, vs 103.9us through Int64 alone and 311.1us on main). The bound is load-bearing, not conservative: above 2^53 an integral double's shortest form is generally NOT its exact digits (2^62 renders as 4611686018427388000, its exact value ends ...87904), so the new wbtest pins 2^62 and 2^60 to fail if the bound is ever widened. Also pins both sides of 2^53, tier-2 fractional values, and the differential checksum over ~330k values (including 2^53 +/- 2 and trailing-zero mantissas) is byte-identical to main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014z81qJ5vdYoxZ48JTKCdAy

Codex re-review: the const doc claimed 2^53 is the largest double round- tripping through Int64 (2^62 also does - the real property is that every integer through 2^53 is exactly representable), and a wbtest comment still said values just outside Int range fall back to the full algorithm (they now take the Int64 tier). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014z81qJ5vdYoxZ48JTKCdAy

…ched view" This reverts commit 220e85f.

Array::copy, Deque::to_array, SortedSet::to_array and HashMap::to_array have no in-repo benchmark, so any performance claim about them is only reproducible from a scratch package. Add coverage first. Each function gets an Int and a Ref (String) variant, following array_make_blit_bench_test.mbt. The element type is not incidental here: it decides whether the per-element store carries reference-counting traffic, and the spread is large enough that an Int-only benchmark would hide it entirely. Array::copy Int n=1000000 56.14 µs Array::copy Ref n=1000000 1.70 ms Deque::to_array Int n=50000 35.91 µs Deque::to_array Ref n=50000 101.43 µs SortedSet::to_array Int 213.39 µs SortedSet::to_array Ref 250.09 µs HashMap::to_array Int 495.85 µs HashMap::to_array Ref 865.90 µs deque and sorted_set did not import the bench package for tests; add it.

…t it `Array::make_uninit` hands back an array that already reports its full length while its slots hold nothing meaningful, so the caller carries an obligation the type does not express. Rename it to `Array::unsafe_make_uninit` to put that in the name, and export it so packages outside builtin can build an array without going through `push`. Marked `#doc(hidden)`, so it is callable cross-package but stays out of pkg.generated.mbti and the generated documentation. The interface file is unchanged by this commit. The doc comment states the caller's obligation and points at `Array::makei` as the better default. No behavior change: this is the rename, the visibility, and the comment.

`Array::make(len, elem)` is `Array::unsafe_make_uninit(len)` followed by `len` `unsafe_set` calls; for a reference element type each of those also increfs `elem` and decrefs the slot's previous contents. Where the array is immediately overwritten, all of that is dead. - Array::copy wrote every element twice: `Array::make(len, self[0])` to fill, then `unsafe_blit` over the whole range. The js backend already avoided this via `slice(0)`. - Deque::to_array, SortedSet::to_array and HashMap::to_array reserved capacity and pushed. Allocate at final length and write with `unsafe_set`, replacing an out-of-line call per element with an inlined store. This also removes the seed-value plumbing: HashMap::to_array loses the while/break/nobreak block whose only job was finding a value to seed `Array::make`, and SortedSet::to_array loses `padding` and `unwrap`. The value is bound to a local before each `unsafe_set`. Written inline as `xs.unsafe_set(i, self[i])`, the call sits between the buffer load and the store, and the reference-counting pass brackets the buffer borrow inside the loop, emitting an incref/decref pair per element that costs more than the bounds check it saves. Measured on native/release with the benchmarks from the first commit. Every A/B varies one file only, with all others pinned, rebuilding each round. Array::copy, 4 rounds, ranges disjoint. FixedArray::copy is a control: its code is identical in both arms. Array::copy Int n=1000000 104.7/111.5/111.1/112.9 -> 53.8/55.6/51.6/53.5 us -50.7% 4/4 Array::copy Ref n=1000000 2240/2200/2260/2250 -> 1620/1700/1640/1780 us -26.4% 4/4 FixedArray::copy Ref (control) 1110/1060/1110/1080 -> 1060/1110/1080/1120 us 0.0% 2/4 to_array, 3 rounds, builtin identical in both arms. Min of rounds: Deque::to_array Int 35.2 -> 27.5 us -21.9% 3/3 Deque::to_array Ref 99.9 -> 91.7 us -8.2% 3/3 HashMap::to_array Int 496.0 -> 462.4 us -6.8% 3/3 HashMap::to_array Ref 783.1 -> 845.0 us -- ranges overlap SortedSet::to_array Int 205.8 -> 204.4 us -0.7% no signal SortedSet::to_array Ref 240.5 -> 236.5 us -1.7% no signal The gain tracks how little else the loop does per element. Deque only indexes and stores. HashMap allocates a tuple per pair and increfs both halves when they are references, which dominates. SortedSet is dominated by its dfs recursion. HashMap Ref is reported as no signal rather than a regression: its own push arm spans 14% across rounds, wider than the difference between arms. SortedSet and HashMap keep the conversion so all four paths build their result the same way. The `len == 0` fastpaths are kept. `unsafe_make_uninit(0)` has no `[]` shortcut, an empty SortedSet would still allocate its dfs closure, and dropping the guards costs 6-31% on empty input. HashMap::to_array gains one it never had, worth -17.7% on an empty map.

…nvalid memory Taking a view from an `Array` shares the underlying buffer, and every shrinking operation cleared the slots it vacated with `%fixedarray.set_null`. A view created before such a mutation then read a null slot: SIGSEGV on native for reference element types, `null` on wasm-gc, `undefined` on JS. `Array::view` is plain `pub`, so this was undefined behaviour reachable from entirely safe code. Vacated slots are now left alone, so a view can only ever observe valid values of `T`. Mutating an array while a view of it is alive stays a program error, but what the view yields is merely unspecified rather than invalid. The cost is that a removal retains what it removes, and that applies uniformly: `clear` empties an array the same way `pop` shortens it, and neither writes to the slots it gives up. No existing signature changes. The removed elements are released once a later push reuses the slot, once the buffer grows, or once the array is dropped, so clear-and-refill self-heals -- each push releases one old occupant, and repeated fill/drain cycles on one array hold flat. Reclaiming on demand is explicit. The new `Array::release_unused(placeholder~)` overwrites every slot from `length()` to `capacity()` with the placeholder, which is exactly the region any removal leaves behind: one pass, no allocation, capacity kept. `shrink_to_fit` was already releasing those elements by letting the old buffer go, at the cost of an allocation plus a copy of every survivor; that is now documented rather than incidental. Writing into that region needs no high-water mark, because it is always either NULL or a live reference, never garbage. `%fixedarray.make_uninit` NULL-fills for reference element types -- it has to, since `moonbit_drop_object` walks a REF_ARRAY's full capacity and skips slots with `if (!obj) continue` -- and `moonbit_make_ref_array_with_blit` NULL-fills everything outside the blitted range when a buffer grows. It is the same region `Array::push` writes into on every push past the previous high-water mark. `Array::resize` shrinks like `truncate` and no longer releases on that branch. Two things differ from the code in #4135. `Array::resize_buffer` now copies only the live prefix into the new buffer instead of the whole old capacity: when vacated slots were NULL the difference was a memcpy of zeros, but with retention it decided whether `reserve_capacity` carried the retained elements into the new buffer, which would have made the "released once the buffer grows" promise false on that one path. And `Array::release_unused` goes through the same `%fixedarray.fill` intrinsic, with the same `#owned` argument, that `Array::fill` and `Array::resize` already use for this region, instead of a hand-written `unsafe_set` loop. The JavaScript backend is unchanged and still shrinks the underlying JS array, so a view reaching past the current length observes `undefined` there; the `ArrayView` documentation now says so explicitly, and `Array::release_unused` is a documented no-op on that backend. This is the `Array` half of #4135, split out so that each container can be reviewed on its own; `Deque` gets the same treatment in a separate PR, and `%fixedarray.set_null` loses its last user in core once both have landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WjN1QF3HbJohHocMPBRcuw

…emory `Deque::as_views` hands out `ArrayView`s that share the deque's buffer, and every shrinking operation cleared the slots it vacated with `%fixedarray.set_null`. A view created before such a mutation then read a null slot: SIGSEGV on native for reference element types, `null` on wasm-gc, `undefined` on JS. `Deque::as_views` is plain `pub`, so this was undefined behaviour reachable from entirely safe code. Vacated slots are now left alone, so a view can only ever observe valid values of `A`. Mutating a deque while a view of it is alive stays a program error, but what the view yields is merely unspecified rather than invalid. The cost is that a removal retains what it removes, and that applies uniformly: `clear` empties a deque the same way `pop_back` shortens it, and neither writes to the slots it gives up. No existing signature changes. The removed elements are released once a later push reuses the slot, once the buffer grows, or once the deque is dropped, so clear-and-refill self-heals -- each push releases one old occupant, and repeated fill/drain cycles on one deque hold flat. Reclaiming on demand is explicit. The new `Deque::release_unused(placeholder~)` overwrites every slot not currently holding an element with the placeholder -- the complement of the occupied run, which may wrap around the end of the buffer -- and that is exactly the region any removal leaves behind: one pass, no allocation, capacity kept. `shrink_to_fit` was already releasing those elements by letting the old buffer go, at the cost of an allocation plus a copy of every survivor; that is now documented rather than incidental. Writing into that region needs no high-water mark, because it is always either NULL or a live reference, never garbage: `UninitializedArray::make` (`%fixedarray.make_uninit`) NULL-fills for reference element types, growing blits into a fresh NULL-filled buffer, and the pushes already store into exactly such slots. `Deque::clear` and `Deque::truncate` collapse to length assignments, and `Deque::truncate(0)` no longer routes through `Deque::clear`. `Deque::drain` keeps its blits and loses only the nulling passes; the branch that existed solely to null an emptied back run is folded away. This is the `Deque` half of #4135, split out so that each container can be reviewed on its own; `Array` gets the same treatment in a separate PR, and `%fixedarray.set_null` loses its last user in core once both have landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WjN1QF3HbJohHocMPBRcuw

The parsing API moved to `@string` long ago; `moonbitlang/core/strconv` has been a deprecated compatibility layer over `internal/strconv` since then, and no package in this module imports it anymore. Delete the package together with its duplicated tests, README and generated interface, and drop it from the README dependency graph. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

… comments Follow-up to #4189 from its review. `Deque::drain` handled a drain ending at the end of the front run with one `else` arm: shift the front survivors right by the drained length and advance `head`. That is needed when something survives after the drained range -- the rest of the front, or a non-empty back run -- so the survivors stay contiguous with it. When the deque is unwrapped and the drain takes its tail, nothing survives after the range, the survivors already sit at `[head, head + start)`, and the shift was O(start) work that moved every survivor one slot for no reason. The arm is now guarded on `len < front_max_drain || back.length() != 0` and the tail case falls through with `head` untouched. A new test pins the behaviour through a view taken before the drain: with the survivors left in place the view still reads them at their original offsets, whereas the old shift made it read the first survivor twice. Reverting the guard fails that test and nothing else. Two comments in `clear` and `truncate` called "an emptied deque restarts at offset zero" an invariant. It is not one: `pop_front` and `extract_if` can empty a deque without resetting `head`, and nothing depends on it. They now say the three functions arrange it, which is what they do. Also adds a `release_unused` test where the unused capacity sits on both sides of an unwrapped run, so each of the two `fill_slots` calls in the unwrapped branch is pinned by a test on its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WjN1QF3HbJohHocMPBRcuw

The tuple `Show` impls are deprecated, so the old tests could only exercise them under `#warnings("-deprecated")`, stringifying each tuple with `to_string()` and asserting through `@json.json_inspect`. Inspect the tuple's `Debug` representation directly with `debug_inspect` instead and drop the warning suppressions. Tuples wide enough to be pretty-printed on several lines (12+ elements here) use the multi-line expectation form. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

- `ParseError` variants and the deprecated `Json` `Show` render through `<+` templates; the conditional `Number(...)` arm delegates to a local writer function so its delimiters stay paired in one template - `JsonPath` JSON Pointer rendering streams each path step through a template with `\{cb => ...}` writer holes Inline-writer holes use `\{cb => ...}` so delimiters stay paired and the templates stay readable. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

…equences - `Show for Iter` and `Show for Map` write their delimiters and separators through `<+` templates with `\{cb => ...}` writer holes - base64 `==` tail and a FixedArray iterator-test builder collapse into single `<+` writes Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

…pers - `Range`/`HunkHeader` Show write through `<+>` templates; a lone range is written via `write_object` - `V128` hex formatting streams through a new `u64_hex_to` writer helper (the string `u64_hex` wrapper is kept for the Debug impl), and Show renders `V128(...)` through a `<+>` template Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

Range members are Ints, so plain `\{...}` interpolation writes them directly - no inline writer holes needed. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

`Show` for `HashMap` and linked-hash `Set` was deprecated in favor of `@debug.Debug`. Drop the impls (and their hidden `Show::{output, to_string}` extends plus the now-obsolete Show test), now that nothing in the tree renders these collections through `Show` anymore. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

…dixes Closes #3827. The generic radix path extracted one digit per full BigInt division (grade_school_div per output digit, O(n^2) overall). Generalize the radix-10 algorithm already used by to_string: convert limb-by-limb into base chunk = radix^chunk_len slots using only Int64 arithmetic, with chunk chosen as the largest power keeping (slot << RADIX_BIT_LEN) | limb inside Int64, then emit chunk_len digits per slot into a pre-sized buffer. Also drops the -self magnitude copy the old path made for negative inputs. Native release benchmarks on a ~4000-bit value (committed): - radix=7: 1.28 ms -> 83 us (~15x) - radix=36: 762 us -> 95 us (~8x) - both now match the optimized radix-10 path (~105 us) as expected New tests: known values for radixes 3/6/36, from_string round-trips for ten non-power-of-two radixes on a 700+ bit value (positive and negative), and radix-3 digit-length checks across chunk boundaries (3^e for e around 19 and 38). Reviewed by Codex CLI (codex-cli 0.144.1): "Approved; no findings" — with an explicit Int64 overflow bound derivation, slot-count and pos-underflow checks, sign-magnitude equivalence for negatives, and an independent 7,650-case differential check across all 30 applicable radixes. Signed-off-by: Codex CLI <codex@openai.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… aborting Fixes #4049. parse aborted (panic, not ParseError) on strings mixing a raw lone trailing surrogate with any escape sequence, e.g. the 5-code-unit text " U+DC00 \n ": lex_string_slow's flush sliced with the checked ctx.input[start:end], which aborts when the code unit at a slice boundary is a trailing surrogate. Meanwhile the escape-free fast path silently *accepted* raw lone surrogates, producing ill-formed strings. Per the design rule that MoonBit Strings stay Unicode well-formed (unsafe_to_char is indeed unsafe), well-formed input cannot contain a raw lone surrogate in the first place — but unsafe code can manufacture such a String, and robustness demands a clean error over a process abort or an ill-formed result. The string lexer now rejects raw unpaired surrogates with the documented ParseError (InvalidChar) on both the fast path and the slow path, and flush slices with the bounds-check-only view(start_offset~, end_offset~) so no abort path remains. Escaped surrogate sequences (\uXXXX) are out of scope here; unpaired escape handling is tracked in #4062 and fixed separately. Deterministic regression tests in lex_string_test.mbt cover escape-free and escape-adjacent raw lone surrogates (the former abort), the exact ParseError shape, and still-accepted well-formed pairs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

New property families complementing json/quickcheck_test.mbt: - adversarial strings (every control character, JSON syntax characters, BMP boundaries, astral pairs, and lone surrogates) roundtrip through stringify/parse as both values and object keys, across indent and escape_slash options; - the fully \uXXXX-escaped spelling of those strings parses back to the exact original, including surrogate pairs split across two escapes and lone-surrogate escapes; - every textual spelling of zero preserves the sign of zero bitwise; - Int64/UInt64 literals roundtrip textually (repr preserved past 2^53); - random legal whitespace inserted between tokens never changes the parsed value; - duplicate object keys: last occurrence wins; - single code-unit deletions/replacements of valid documents keep the parser total (parse agrees with valid, no panics) and successfully parsed mutants are fixed points of restringify-and-reparse; - deterministic pins for the default 1024 nesting-depth boundary and for surrogate handling. The AdvString generator shrinks at the UTF-16 code-unit level so counterexamples can minimize to half of a surrogate pair. These properties found the two parser bugs fixed in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…tate at code-unit level - The nesting-limit test now asserts that documents exactly 1024 levels deep parse successfully for BOTH arrays and objects (previously it only checked valid() for the array and only checked the 1025-deep failure for objects), and the 1025-deep variables are renamed too_deep_array / too_deep_object to reflect their depth. - The mutation-totality test now genuinely mutates single UTF-16 code units via code_units() instead of Char-level to_array(), so deleting or replacing a unit can split an astral surrogate pair, and the replacement unit is drawn from the full 16-bit range (including lone surrogates) — strictly stronger fuzzing that matches the test's name and doc comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…e policy The parser now keeps strings Unicode well-formed (#4056): unpaired surrogates — raw or as \uXXXX escapes — are rejected with a ParseError instead of being passed through. Update the adversarial suite to match: - the roundtrip and fully-escaped generators produce only Unicode scalar values (astral pairs still included), and the AdvString shrinker drops whole characters so candidates stay well-formed; - new property: a lone surrogate injected at any position of a hostile string — raw via stringify or spelled as a \uXXXX escape, with escapes/astral pairs/control characters nearby — is always rejected cleanly (parse raises, valid is false, never an abort); - the deterministic surrogate pins now assert rejection for raw, escaped, reversed-pair, and mixed raw/escaped-half spellings, while well-formed pairs (raw or split across two escapes) still parse. Note: "zero literals preserve the sign of zero" requires the parse(-0) fix from #4061 (based on main) and fails until that lands in this branch's history; all other tests are green on wasm-gc, js, and native. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings August 14, 2026 08:21

Copilot started reviewing on behalf of bobzhang August 14, 2026 08:21 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

bobzhang mentioned this pull request Aug 14, 2026

fix(json): reject raw unpaired surrogates in string lexing instead of aborting #4056

Open

bobzhang force-pushed the agent/quickcheck-json-deep branch from 8360ba6 to fc24c9c Compare August 14, 2026 08:31

bobzhang changed the base branch from main to agent/fix-json-lexer August 14, 2026 08:31

bobzhang force-pushed the agent/fix-json-lexer branch 2 times, most recently from 6af3721 to 5cf2ae6 Compare August 15, 2026 02:19

bobzhang mentioned this pull request Aug 15, 2026

fix(json): preserve the sign of -0 in the integer fast path #4061

Merged

bobzhang force-pushed the agent/quickcheck-json-deep branch from 29009f6 to c217fa5 Compare August 15, 2026 02:20

bobzhang changed the base branch from agent/fix-json-lexer to agent/fix-json-negative-zero August 15, 2026 02:20

bobzhang force-pushed the agent/fix-json-negative-zero branch from 15d2ac4 to b93a00e Compare August 15, 2026 02:55

bobzhang force-pushed the agent/quickcheck-json-deep branch from c217fa5 to 10d4e23 Compare August 15, 2026 03:02

bobzhang changed the base branch from agent/fix-json-negative-zero to agent/fix-json-lexer August 15, 2026 03:03

bobzhang mentioned this pull request Aug 15, 2026

json: parser constructs ill-formed strings from unpaired \uXXXX escape sequences #4062

Closed

bobzhang force-pushed the agent/fix-json-lexer branch from ca31837 to 41233d9 Compare August 15, 2026 03:17

Yu-zh added 3 commits August 15, 2026 11:19

bobzhang mentioned this pull request Aug 15, 2026

fix(json): reject unpaired \uXXXX surrogate escapes in strings #4064

Merged

bobzhang force-pushed the agent/quickcheck-json-deep branch from 10d4e23 to f9a3219 Compare August 15, 2026 03:20

bobzhang and others added 8 commits August 15, 2026 14:49

bobzhang and others added 24 commits September 1, 2026 16:44

Revert "refactor(encoding/ascii): derive Malformed views from the mat…

4566b75

…ched view" This reverts commit 220e85f.

perf(bigint): cache decimal division reciprocal (#4188)

3082191

fix async lexbuf test, adapt new cancellation semantic

2f816fc

refactor(diff): interpolate Range members directly in the template

195ab51

Range members are Ints, so plain `\{...}` interpolation writes them directly - no inline writer holes needed. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>

fix async lexbuf test, adapt new cancellation semantic

3aedb3c

fix(builtin): seed hashes from platform entropy

65b074b

bobzhang force-pushed the agent/quickcheck-json-deep branch from f9a3219 to 772f2a8 Compare September 4, 2026 03:09

bobzhang and others added 4 commits September 4, 2026 11:10

bobzhang force-pushed the agent/quickcheck-json-deep branch from 772f2a8 to ecf2106 Compare September 4, 2026 03:10

Navigation Menu

Uh oh!

test(json): adversarial QuickCheck roundtrip and parser-robustness tests - #4045

test(json): adversarial QuickCheck roundtrip and parser-robustness tests#4045
bobzhang wants to merge 199 commits into
agent/fix-json-lexerfrom
agent/quickcheck-json-deep

bobzhang commented Aug 14, 2026 •

edited

Loading

Uh oh!

coveralls commented Aug 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

bobzhang Aug 15, 2026

Uh oh!

bobzhang Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Uh oh!

Conversation

bobzhang commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

New property families (json/quickcheck_adversarial_test.mbt)

Verification

Uh oh!

coveralls commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 6069

Coverage: 90.663%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bobzhang Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

bobzhang Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

bobzhang commented Aug 14, 2026 •

edited

Loading

New property families (`json/quickcheck_adversarial_test.mbt`)

coveralls commented Aug 14, 2026 •

edited

Loading