Skip to content

fix: keep error paths trim-verifier-clean#472

Open
quinnj wants to merge 3 commits into
masterfrom
trim-verifier-fixes
Open

fix: keep error paths trim-verifier-clean#472
quinnj wants to merge 3 commits into
masterfrom
trim-verifier-fixes

Conversation

@quinnj

@quinnj quinnj commented Jul 11, 2026

Copy link
Copy Markdown
Member

fix: keep error paths trim-verifier-clean

Three error-message constructions made JSON.jl responsible for the majority of --trim verifier errors in downstream juliac builds. Measured on 1.14-DEV (juliac --trim=safe --experimental):

  • an isolated parse+write workload (materializing parse, typed parse, json of Dict and struct): 474 → 56 verifier errors
  • a real HTTP-server application graph: 374 → 205

The three sites (all in src/parse.jl)

1. jsonreadstylerepr(::Symbol) in a throw: 165 errors from one line.
show(::Symbol) routes through the @nospecialized show_unquoted_quote_expr, which makes Base's entire Expr-show machinery (show_unquoted, show_block, show_generator, …) reachable from this error path — each with its own unresolvable dynamic call. Plain symbol interpolation (:$(unknown_fields)) prints identically for standard symbols and lowers to print(::IO, ::Symbol), a plain write.

2. unknownfielderrorrepr(key) where key::PtrString: generic struct-show fallback.
PtrString has no print/show methods, so both repr and plain interpolation fall back to show_default (dynamic sizeof/fieldname sites under trim). Now converts PtrString → String explicitly — which also improves the message: the actual member name instead of a PtrString(Ptr{UInt8}…) struct dump. Keys can also be Int (array-into-struct with extra elements — covered by the existing @test_throws cases), hence the string(key) fallback method.

3. make fallback — "cannot parse $x" interpolated a LazyValue.
Its show renders through the array-display machinery (print_matrix etc.). This branch is unreachable in practice (all JSONTypes are covered above it); it now uses the same "cannot parse json" message as the sibling error paths at lines 360/506.

Behavior change

Error-message text only, in the three messages above. No API or parsing behavior changes. Full test suite passes, including the trim compile tests.

Not addressed here (follow-ups)

The 56 remaining workload errors are not JSON error paths: StructUtils field-matching (findfield/indexed_iterate) and write-closure dispatch (applyeach), and the Parsers BigFloat overflow-fallback digits machinery reached from parsenumber. Those want fixes in StructUtils/Parsers respectively — with them, materializing parse could graduate into the trim test workload's covered entrypoints (the carve-out documented in test/json_trim_public_entrypoints.jl).

…ring, LazyValue)

Three error-path constructions made JSON.jl responsible for the majority of
--trim verifier errors in downstream juliac builds (measured: 418 of 474 in
an isolated parse+write workload; 169 of 374 in a real HTTP-server graph):

- jsonreadstyle: `repr(::Symbol)` routes through the @nospecialize'd
  show_unquoted_quote_expr, making Base's entire Expr-show machinery
  reachable — hundreds of unresolvable dynamic calls from one throw.
  Plain symbol interpolation prints identically for standard symbols.
- unknownfielderror: keys arrive as PtrString (no print/show methods), so
  `repr(key)` fell back to the generic struct show. Convert PtrString to
  String explicitly (also a nicer message: the actual key text instead of
  a PtrString struct dump); string(key) for Int/other scalar keys.
- make fallback: `"cannot parse $x"` interpolated a LazyValue, whose
  show renders via the array-display machinery; use the same "cannot
  parse json" message as the sibling error paths.

No behavior change outside these three messages; full test suite green.
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.45%. Comparing base (e5ef310) to head (f27001d).

Files with missing lines Patch % Lines
src/parse.jl 66.66% 1 Missing ⚠️
src/write.jl 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
+ Coverage   90.41%   90.45%   +0.04%     
==========================================
  Files           7        7              
  Lines        1419     1436      +17     
==========================================
+ Hits         1283     1299      +16     
- Misses        136      137       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

quinnj and others added 2 commits July 14, 2026 12:32
string(::TimeType) routes through Dates' DateFormat machinery (dynamic
lpad/repeat) under juliac --trim; the fixed-width renderings are equal to the
string() forms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compiles test/json_trim_workload.jl with juliac --trim=safe (JuliaC.jl, error
budget zero) and runs the executable, so the trim-verifier fixes in this PR
can't silently regress. Same harness shape as StructUtils/HTTP.

The workload asserts runtime values across typed parse (@defaults incl.
defaulted fields, Vector/Dict targets), the rewritten error paths (the
unknown-field message with its PtrString key formatting, and the bad
unknown_fields-option message — both triggered and matched at runtime),
untyped parse with the isa-narrowing pattern trim-compiled consumers need,
JSON writing incl. escapes and struct roundtrips, and the hand-formatted
Date/DateTime/Time lowers (exact renderings incl. millisecond padding).

The trim env temporarily pins Parsers#trim-verifier-fixes: JSON's number
parsing pulls Parsers' float path into the verify set, and the registered
release fails verification until JuliaData/Parsers.jl#207 ships (TODO in the
setup script).

Skips on Julia < 1.12 and on prerelease builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant