Fix applyeach dispatch, split unions, and default absent fields - #73
Merged
Merged
Conversation
…t absent fields
- `applyeach(f, style, x)` (the do-block order) now requires `f::Function`, so it no
longer collides with style-first overloads a package defines for its own types
(`applyeach(::MyStyle, f, ::MyType)`); those were ambiguous before.
- Array, set and tuple indices pass through `applyeach` unlowered; only object keys go
through `lowerkey`. With JSON's `lowerkey(::Real) = string(x)` every element index of
an array was a String allocation (2M allocations for a 1e6-element vector), and
multi-dimensional `make` under such a style received String indices.
- Union-typed struct fields and array elements reach `f` one member at a time through
an `isa` ladder generated from the field/element type, so unions wider than four
members, or ones met inside a recursive write, are static calls.
- Absent fields: a `Union{Missing,T}` field with no default reads as `missing`; a
required field that is absent throws an `ArgumentError` naming the field instead of a
bare `TypeError`. The message holds no type, so it is safe under `juliac --trim`.
- Union selection in `make` is unrolled over the members, so `arraylike(::DataType)` is
no longer a dynamic call under `--trim` for unions of three or more members.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
quinnj
changed the title
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.
Follow-ups from JuliaGeo/GeoJSON.jl#128. Missing fields use the null their types admit, union selection remains statically resolvable under safe trimming, and callback-first
applyeachaccepts callable structs without conflicting with style-first overloads.Every built-in key and index continues to pass through
lowerkey, including arrays, tuples, sets, generators, simple vectors, union elements, and undefined slots. JuliaIO/JSON.jl#486 removes discarded numeric-index string allocations within JSON while preserving the hook calls.Only union element types use the generated union helper. Wide struct unions share a non-inlined helper to avoid copying the dispatch ladder into every field.
Validation at
4f4e220e91b474bcd3d2cdc6bb74f9cd13aa40c1:benchmarks/applyeach.jl; the companion JSON PR contains runtime, first-call, load, and precompile benchmarks.The earlier review found remaining tradeoffs versus the package bases: smaller wide-union structs can take longer on first use, and focused Julia 1.13 warm wide-struct writes were about 4–6% slower with unchanged allocations. The hook restoration does not claim to remove those tradeoffs.
Co-authored by Codex