Compare commits
4
Commits
v0.2.0
...
readplan-poc
| Author | SHA1 | Date | |
|---|---|---|---|
 glm-5.2
|
f7c71da9e5 | ||
|
glm-5.2
|
1037e68091 | ||
|
glm-5.2
|
3184818c08 | ||
|
glm-5.2
|
51cb552715 |
No files matched your search
Generated
+8
@@ -547,6 +547,14 @@ version = "5.3.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f"
|
||||
|
||||
[[package]]
|
||||
name = "readplan-poc"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"alktype",
|
||||
"serde_json",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "redox_syscall"
|
||||
version = "0.5.18"
|
||||
|
||||
+4
-1
@@ -19,4 +19,7 @@ default = []
|
||||
|
||||
[dependencies]
|
||||
jsonschema = { version = "0.46", default-features = false }
|
||||
serde_json = { version = "1", features = ["preserve_order"] }
|
||||
serde_json = { version = "1", features = ["preserve_order"] }
|
||||
|
||||
[workspace]
|
||||
members = ["poc/readplan"]
|
||||
@@ -45,6 +45,7 @@ format definition; the engine is generic.
|
||||
| [008](decisions/008-reject-tunion-in-aligned-mode.md) | Reject TUnion in Aligned Mode for v1 | Unions are the protocol pattern; aligned-mode union semantics were broken |
|
||||
| [009](decisions/009-builder-api.md) | Builder API for Schema Construction | Fluent Rust API producing `serde_json::Value`; covers BAST kinds + standard JSON Schema; resolves OQ-003. *Output format amended to BAST / standard JSON Schema by ADR-BAST.* |
|
||||
| [010](decisions/010-generalized-validation-validate-bytes.md) | Generalized Validation — `validate_bytes` on `AlkTypeEngine` | Single-call binary-buffer validation; materialize `Value` from bytes, then validate. *Validation step amended to the BAST-native validator by ADR-VAL-SPLIT.* |
|
||||
| [011](decisions/011-compiled-read-plan-for-packed-mode.md) | Compiled Read Plan for Packed Mode | `ReadPlan` — the packed read-side compiled form, symmetric to `OffsetMap` (aligned) and `PackedLayout` (packed write). Closes review #004's 400x read-path gap; retires ADR-007's "re-parse on demand" framing. *Accepted.* |
|
||||
|
||||
## Relevant Open Questions
|
||||
|
||||
|
||||
@@ -69,6 +69,18 @@ schema itself. This is cheap — a struct has a small number of fields
|
||||
(SFTP's largest packet has 5). The construction cost is negligible
|
||||
compared to the cost of reading a buffer.
|
||||
|
||||
> **Note**: The "re-parse on demand" framing below (the read loop
|
||||
> re-parsing `BastDoc::new` per field) is the root cause of the 400x
|
||||
> read-path gap measured in
|
||||
> [review #004](../../reviews/004-performance-review.md).
|
||||
> [ADR-011](011-compiled-read-plan-for-packed-mode.md) (Proposed)
|
||||
> retires this framing by giving the packed read path a compiled
|
||||
> `ReadPlan`; the "Cost" section here and the
|
||||
> `src/engine.rs:112-115` doc comment will be updated in the
|
||||
> implementation commit per ADR-011's recommended order. The factory
|
||||
> decision itself (`sequential_reader() -> Option<SequentialReader>`,
|
||||
> owned fresh reader, consumer-driven cursor) is retained.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
@@ -0,0 +1,490 @@
|
||||
# ADR-011: Compiled Read Plan for Packed Mode
|
||||
|
||||
## Status
|
||||
|
||||
Accepted. Closes review #004 H1 + M1 (packed side) + L1 + L2;
|
||||
retires the "re-parse on demand" framing from ADR-007. A derisking
|
||||
POC on branch `readplan-poc` confirmed the `ReadPlan` shape covers
|
||||
every `BastType` arm in the current read loop before implementation
|
||||
began (see "POC coverage" at the end).
|
||||
|
||||
## Context
|
||||
|
||||
Review #004 (`docs/reviews/004-performance-review.md`) measured the
|
||||
packed read path at **~400x slower per chunk** than a hand-rolled codec
|
||||
(2.27 µs/chunk vs 5.6 ns/chunk), with the cost fixed across payload
|
||||
sizes — the signature of per-field interpretive overhead, not
|
||||
payload-copy overhead. The write path is competitive (~1.1x at 4 KiB)
|
||||
because it uses a compiled form; the read path is not because it
|
||||
doesn't.
|
||||
|
||||
### The three layout-side compiled forms and the one gap
|
||||
|
||||
ADR-002 defines two layout modes. Each mode has a write-side and a
|
||||
read-side. Three of the four slots already have a **compiled form** —
|
||||
a data structure built once from the schema, held by the engine, and
|
||||
walked at access time without re-touching the schema:
|
||||
|
||||
| mode | write-side | read-side |
|
||||
|------|-----------|-----------|
|
||||
| aligned static | `OffsetMap` (used for both) | `OffsetMap` |
|
||||
| packed sequential | `PackedLayout` (`LayoutBuilder::build`) | *(none)* |
|
||||
|
||||
- **`OffsetMap`** (aligned, both sides) — a flat table of
|
||||
`(field_path, ByteRange)` pairs computed once via
|
||||
`OffsetMap::compute(&BastDoc)`. Read and write both look up a field's
|
||||
byte range and call `data_access::read_*`/`write_*` at the known
|
||||
offset. No schema walk at access time.
|
||||
- **`PackedLayout`** (packed, write-side) — a flat table of
|
||||
`(field_path, FieldPosition)` pairs computed once via
|
||||
`LayoutBuilder::build(&var_sizes)`. The write loop calls
|
||||
`data_access::write_*` at the precomputed offsets. No schema walk at
|
||||
write time.
|
||||
- **packed read-side** — `SequentialReader` walks `BastDoc`
|
||||
interpretively on every field read. There is no compiled form.
|
||||
|
||||
This is the structural reason the read path is 400x slow: it is the
|
||||
only access path in the engine with no compiled form. Every other
|
||||
mode/side pair compiles the schema once and reuses the result.
|
||||
|
||||
### Root cause: the `BastDoc<'a>` borrow constraint
|
||||
|
||||
`BastDoc<'a>` borrows `&'a Value` and `&'a str` throughout
|
||||
(`src/bast.rs:51-55`). The owning structs that need a parsed tree at
|
||||
read time — `SequentialReader` (owns a cloned `Value`),
|
||||
`AlkTypeEngine` (owns `bast_doc: Value`), `LayoutBuilder` (owns
|
||||
`doc_value: Value`) — cannot store a `BastDoc` that borrows from their
|
||||
own `Value` field. That would be a self-referential struct, which safe
|
||||
Rust cannot express.
|
||||
|
||||
The workaround chosen in ADR-007 was "re-parse on demand": the engine
|
||||
and reader retain a clone of the raw `Value` and reconstruct the
|
||||
`BastDoc` from it whenever the typed tree is needed. ADR-007's "Cost"
|
||||
section argued this was cheap because construction is a small `Vec` of
|
||||
field schemas. That is true for *construction* (once), but the decision
|
||||
did not account for `read_field_at` re-parsing `BastDoc::new` **per
|
||||
field read** — the cost that actually dominates. For an N-field struct,
|
||||
reading all fields is O(N²) in parse work (each of N reads re-parses
|
||||
all N fields).
|
||||
|
||||
### Why `OffsetMap` is a flat table but the packed read plan cannot be
|
||||
|
||||
`OffsetMap` works as a flat `(path, byte_range)` lookup table because
|
||||
aligned positions are **data-independent** — field N's offset depends
|
||||
only on the schema, not on the bytes of fields 0..N-1. Random access
|
||||
by path is free.
|
||||
|
||||
Packed positions are **data-dependent** — a variable-length field's
|
||||
extent is read from its length prefix at access time, and every
|
||||
subsequent field's position shifts accordingly. You cannot look up
|
||||
field N's offset without reading fields 0..N-1 first. So the compiled
|
||||
form for packed reads cannot be a flat lookup table; it must be a
|
||||
**read program** — a pre-resolved tree of read instructions that the
|
||||
read loop walks in order, advancing a cursor. The schema is compiled
|
||||
into the program once; the bytes are walked against it at read time.
|
||||
|
||||
This asymmetry is inherent to packed sequential layout (ADR-002) and is
|
||||
not a flaw in `OffsetMap`. The two modes need different compiled-form
|
||||
shapes because they have different position-computation semantics.
|
||||
|
||||
## Decision
|
||||
|
||||
**Introduce `ReadPlan` — the compiled read-side form for packed mode,
|
||||
symmetric to `OffsetMap` (aligned read-side) and `PackedLayout` (packed
|
||||
write-side).**
|
||||
|
||||
`AlkTypeEngine::compile` builds the `ReadPlan` once from the `BastDoc`
|
||||
(in packed mode) and holds it for the life of the engine.
|
||||
`sequential_reader()` hands out fresh `SequentialReader`s that share
|
||||
the engine's `Arc<ReadPlan>` — the plan is immutable; only the cursor
|
||||
state (`field_index`, `position`) is per-reader. The read loop walks
|
||||
the plan, never touching `BastDoc` or the raw `Value`.
|
||||
|
||||
The same `ReadPlan` is consumed by `materialize_packed` (the other
|
||||
byte-walking path), unifying the two packed read-side consumers on one
|
||||
compiled form — mirroring how `OffsetMap` unifies the aligned read and
|
||||
write sides.
|
||||
|
||||
### The `ReadPlan` shape
|
||||
|
||||
A pre-resolved tree of read instructions. Every `$ref` is resolved, every
|
||||
endianness is computed (field override or container default), every
|
||||
union variant is inlined. The read loop indexes into a `Vec`, matches
|
||||
on a `ReadKind`, and calls `data_access::read_*` with a precomputed
|
||||
`Endian` — no `resolve_typeref`, no `BastDef::parse`, no JSON node
|
||||
access on the happy path. (`format!` for error-path attribution may
|
||||
still occur on the error path; it does not run on the happy path and
|
||||
is not the cost being removed here.)
|
||||
|
||||
```rust
|
||||
pub struct ReadPlan {
|
||||
endian: Endian,
|
||||
fields: Vec<FieldPlan>,
|
||||
by_name: HashMap<String, usize>,
|
||||
}
|
||||
|
||||
pub struct FieldPlan {
|
||||
name: String,
|
||||
kind: ReadKind,
|
||||
endian: Endian,
|
||||
encoding: VariableEncoding,
|
||||
max_length: Option<usize>,
|
||||
body: Option<CompositePlan>,
|
||||
}
|
||||
|
||||
pub enum ReadKind {
|
||||
Primitive(AlkTypeKind),
|
||||
Enum,
|
||||
Struct,
|
||||
Union,
|
||||
Array,
|
||||
Record,
|
||||
}
|
||||
|
||||
pub enum CompositePlan {
|
||||
Struct(ReadPlan),
|
||||
Union {
|
||||
disc: DiscriminatorPlan,
|
||||
variants: Vec<(String, VariantPlan)>,
|
||||
},
|
||||
Array {
|
||||
element: Box<CompositePlan>,
|
||||
count: usize,
|
||||
element_stride: usize,
|
||||
},
|
||||
Record {
|
||||
value: Box<CompositePlan>,
|
||||
},
|
||||
}
|
||||
|
||||
pub enum DiscriminatorPlan {
|
||||
Byte { offset: usize, disc_type: AlkTypeKind },
|
||||
Field { name: String, field_index: usize },
|
||||
}
|
||||
```
|
||||
|
||||
Nested structs share the `ReadPlan` shape (a struct field's `body` is
|
||||
`CompositePlan::Struct(ReadPlan)`). Union variants are pre-resolved:
|
||||
each `(key, VariantPlan)` entry carries the variant's `ReadPlan`, so
|
||||
dispatch is a flat lookup + recurse — no `resolve_typeref_as_def` at
|
||||
read time. Array element strides are precomputed (`element_stride = 0`
|
||||
signals variable-length elements, same convention as today's
|
||||
`FieldValue::Array`).
|
||||
|
||||
### Construction
|
||||
|
||||
```rust
|
||||
impl ReadPlan {
|
||||
pub fn compile(bast_doc: &Value, root_name: &str) -> Result<Self, AlkTypeError>;
|
||||
}
|
||||
```
|
||||
|
||||
`compile` walks `BastDoc` once, resolves all `$ref`s eagerly, computes
|
||||
effective endianness at every node, inlines union variants, and
|
||||
builds the `FieldPlan`/`CompositePlan` tree. Malformed schemas surface
|
||||
as `AlkTypeError::Schema` — the same untrusted-input discipline
|
||||
(AGENTS.md §3) and overflow-safe arithmetic (AGENTS.md §4) as
|
||||
`BastDoc::new`.
|
||||
|
||||
### Engine integration
|
||||
|
||||
`AlkTypeEngine::compile` builds the `ReadPlan` in packed mode and
|
||||
stores `Arc<ReadPlan>`. `sequential_reader()` returns
|
||||
`SequentialReader { plan: Arc::clone(&self.plan), .. }` — an owned
|
||||
reader (ADR-007's factory decision is retained; the reader owns its
|
||||
cursor, shares the immutable plan).
|
||||
|
||||
`bast_doc: Value` (`src/engine.rs:85`) is retained on the engine
|
||||
unconditionally. In packed mode it becomes unused by the read path
|
||||
(both `sequential_reader` and `validate_bytes` consume the plan);
|
||||
in aligned mode it is still needed for `read_field`/`write_field`/
|
||||
`validate_bytes`. Keeping it always avoids a mode-conditional field
|
||||
and costs only a `Value` clone paid once at `compile`. `ReadPlan`
|
||||
must be `Send + Sync` so `Arc<ReadPlan>` can be shared from the
|
||||
`Send + Sync` engine (ADR-007); this falls out naturally from the
|
||||
plan being immutable owned data, but the implementation should add a
|
||||
`static` bound assertion test to lock it in.
|
||||
|
||||
### Public API change (breaking — version bump to 0.3.0)
|
||||
|
||||
- `SequentialReader::new(&Value, &str)` → `SequentialReader::new(Arc<ReadPlan>)`.
|
||||
The old constructor is replaced by `ReadPlan::compile(&Value, &str)`
|
||||
followed by `SequentialReader::new(Arc::from(plan))`.
|
||||
- `materialize_packed(&BastDoc<'_>, &[u8])` →
|
||||
`materialize_packed(&ReadPlan, &[u8])`.
|
||||
- `materialize_aligned` is unchanged (already takes `&OffsetMap`, a
|
||||
compiled form).
|
||||
- `ReadPlan` is a new public type, re-exported from `lib.rs`.
|
||||
- `BastDoc` and the `Bast*` types are **unchanged** — they remain the
|
||||
validation-side typed tree, borrowed, as today. This is a smaller
|
||||
breakage than review #004's Option A (which changed `BastDoc<'a>` →
|
||||
`BastDoc` and every `Bast*` signature).
|
||||
|
||||
The crate is pre-1.0 with two in-house downstream consumers
|
||||
(`alktty`, `alkcall`), both of which will be updated with the bump.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope (consumes the `ReadPlan`)
|
||||
|
||||
- **`SequentialReader`** — the read loop walks `FieldPlan`/`CompositePlan`
|
||||
instead of `&BastField`/`&BastType` + `&BastDoc`. The functions
|
||||
`read_field_value`, `read_typeref_value`, `walk_struct_size`,
|
||||
`read_union_value`, `read_array_value`, `read_record_value` are
|
||||
rewritten to take plan nodes. One walker, not two — the review's
|
||||
Option B concern ("duplicates the `BastType` matching logic") does
|
||||
not apply because the plan *replaces* the `BastType` matching, not
|
||||
parallels it.
|
||||
- **`materialize` (packed mode)** — `materialize_packed` takes
|
||||
`&ReadPlan` and walks it to produce `serde_json::Value`. Same read
|
||||
logic, same `data_access` calls, different input type. Unifies the
|
||||
two packed read-side consumers on one compiled form.
|
||||
- **`AlkTypeEngine::validate_bytes` (packed mode)** — calls
|
||||
`materialize_packed(&self.plan, buffer)` instead of reconstructing a
|
||||
`BastDoc`. Closes M1's `engine.rs:284` re-parse.
|
||||
|
||||
### Out of scope (stays on `BastDoc`)
|
||||
|
||||
- **`bast_validation`** — the BAST-native value-domain validator walks
|
||||
`BastDoc` to check constraints (`maxLength`, enum string values,
|
||||
union variant keys, integer ranges). These are value-domain checks,
|
||||
not byte-position walks; they don't benefit from a read plan and
|
||||
would require a separate "validation plan" with a different shape.
|
||||
Validation is not a per-chunk hot loop (AGENTS.md: validation is
|
||||
opt-in per operation). A future `ValidationPlan` is a two-way door
|
||||
if a bench motivates it; for now `bast_validation` keeps walking
|
||||
`BastDoc`, which remains the validation-side typed tree.
|
||||
- **`LayoutBuilder` / `PackedLayout`** — the packed write-side already
|
||||
has a compiled form (`PackedLayout`). `LayoutBuilder::build`
|
||||
(`src/layout_builder.rs:190`) re-parses `BastDoc::new` per `build()`
|
||||
call (M1), but the typical pattern is build-once-reuse, so this is
|
||||
not a hot loop. A future `WritePlan` that lets `LayoutBuilder` cache
|
||||
the typed tree (review #004 Option A's territory) is additive and can
|
||||
follow; it is not blocking the read-path fix.
|
||||
- **`OffsetMap` / aligned mode** — already a compiled form; unchanged.
|
||||
`materialize_aligned` already takes `&OffsetMap`.
|
||||
- **Aligned-mode `read_field` / `write_field`** (`engine.rs:334,467`)
|
||||
re-parse `BastDoc::new` per call (M1). These are one-shot paths, not
|
||||
hot loops; they can adopt a compiled form later without affecting
|
||||
the packed read-path decision. Left as-is for now.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Closes the 400x read-path gap (review #004 H1).** The read loop no
|
||||
longer touches `BastDoc` or the raw `Value`. Per-field work drops
|
||||
from "re-parse the typed tree + resolve_typeref + match" to "index
|
||||
into a `Vec` + match `ReadKind` + `data_access::read_*` with a
|
||||
precomputed `Endian`." The expected per-chunk cost is in the
|
||||
hand-rolled codec's ballpark (the `data_access` calls are the same
|
||||
ones the hand-rolled codec makes).
|
||||
- **Closes M1's packed-side re-parse (`engine.rs:284`, `validate_bytes`).**
|
||||
The aligned-side M1 sites (`engine.rs:334,467`,
|
||||
`layout_builder.rs:190`) are deliberately left as-is — they are not
|
||||
hot for the packed-codec use case, and Option A remains available as
|
||||
an additive later fix if an aligned-mode hot loop ever emerges. This
|
||||
is a reversible bet, not a claim that the aligned-side M1 is a
|
||||
non-issue.
|
||||
- **Unifies the two packed read-side consumers on one compiled form.**
|
||||
`SequentialReader` and `materialize_packed` walk the same `ReadPlan`,
|
||||
mirroring how `OffsetMap` unifies the aligned read and write sides.
|
||||
The "two parallel walkers" concern from review #004 Option B does
|
||||
not apply — the plan replaces the `BastType` matching, not
|
||||
duplicates it.
|
||||
- **Symmetric with the other compiled forms.** The engine now has a
|
||||
compiled form for every mode/side pair: `OffsetMap` (aligned R/W),
|
||||
`PackedLayout` (packed W), `ReadPlan` (packed R). The "compiled form
|
||||
of a BAST document" framing in ADR-004/validation.md becomes true for
|
||||
the read path, not just the write path.
|
||||
- **ADR-007's factory gets cheaper.** Today
|
||||
`sequential_reader()` clones `doc_value: Value` (the whole BAST
|
||||
document) per reader. With the plan, it clones an `Arc<ReadPlan>`
|
||||
(refcount bump). The plan is immutable and shared across all readers
|
||||
from one engine. ADR-007's "owned fresh reader" decision is retained;
|
||||
the reader owns its cursor, shares the plan.
|
||||
- **Deterministic compile.** `ReadPlan::compile` is a pure function of
|
||||
the BAST document + root name — same input, same plan. This makes the
|
||||
"compiled form" visibly deterministic, which is a prerequisite for
|
||||
future capabilities (fingerprinting the plan for cross-run caching,
|
||||
disk-cached compiled plans, or schema-version handshakes for
|
||||
`alkcall`'s hub/spoke topology). Not implemented in this ADR and not
|
||||
needed to justify the decision; listed here only so a future ADR
|
||||
doesn't re-derive the prerequisite. See "Future capabilities" below.
|
||||
- **Retires the "re-parse on demand" framing (L2).** ADR-007's "Cost"
|
||||
section and `src/engine.rs:112-115`'s doc comment framed re-parse as
|
||||
the intended design. With the plan, the read path never re-parses;
|
||||
the framing is retired. ADR-007's "Cost" section and the doc comment
|
||||
are updated in the same commit.
|
||||
- **L1 falls out.** The dead `_field_schema: &Value` parameter and the
|
||||
`Vec<(String, Value)>` field storage (where the `Value` half is
|
||||
unused) are replaced by `Vec<FieldPlan>`. No dead `Value` clones.
|
||||
|
||||
### Negative
|
||||
|
||||
- **Breaking public-API change (0.2.0 → 0.3.0).** `SequentialReader::new`
|
||||
and `materialize_packed` change signatures (take `ReadPlan` instead
|
||||
of `&Value`/`&BastDoc`). `ReadPlan` is a new public type. Per
|
||||
AGENTS.md, this is semver-relevant. The crate is pre-1.0 with two
|
||||
in-house downstream consumers, both updated with the bump. The
|
||||
breakage is smaller than review #004's Option A (no `Bast*` type
|
||||
changes — `BastDoc` stays borrowed, stays the validation-side tree).
|
||||
- **A parallel typed tree, not a flat lookup table.** This is the
|
||||
honest cost. `OffsetMap` and `PackedLayout` are flat `(path, range)`/
|
||||
`(path, position)` projections of `BastType`; `ReadPlan` is a full
|
||||
parallel hierarchy (`CompositePlan` mirrors `BastType`'s
|
||||
Struct/Union/Array/Record). The maintenance tax is real and higher
|
||||
than those: when schema semantics change, `BastDoc`/`BastType` and
|
||||
`ReadPlan`/`CompositePlan` move together. It is worth it because the
|
||||
perf win on composite-heavy schemas (the SFTP-shaped union-with-`$ref`
|
||||
-variants packet) justifies it — see the next bullet. This is a
|
||||
permanent tax accepted in exchange for a ~20–50x composite-dispatch
|
||||
win on top of the 400x re-parse fix, not a structural symmetry with
|
||||
the flat compiled forms.
|
||||
- **Eager `$ref` resolution at compile time.** `ReadPlan::compile`
|
||||
resolves all `$ref`s eagerly, including union variant refs. This is
|
||||
correct (the schema is fixed at compile time) and matches the
|
||||
review's Option B design, but it means a schema with a `$ref` cycle
|
||||
(which BAST forbids — refs are always `#/$defs/<name>`, no
|
||||
recursion) would loop forever. The meta-schema (`bast_meta`)
|
||||
already rejects recursive schemas; `ReadPlan::compile` inherits
|
||||
that guard. No new failure mode.
|
||||
- **Nested `Box<CompositePlan>` vs flat `Vec<Op>`.** The plan as
|
||||
specified uses nested `Box`es — idiomatic, debuggable, easy to
|
||||
build. A flat `Vec<Op>` with jump indices (true "bytecode") would be
|
||||
more cache-friendly but harder to build and read. Protocol headers
|
||||
are small N (SFTP's largest packet has 5 fields); the perf win is
|
||||
eliminating the re-parse and `resolve_typeref`, not SoA cache
|
||||
effects. Start nested; go flat only if a bench says otherwise (a
|
||||
two-way door — the plan is a private internal type; its shape can
|
||||
change without a semver bump as long as the public `ReadPlan` name
|
||||
and `compile`/`SequentialReader::new` signatures are stable).
|
||||
|
||||
## Scope Boundaries (What This Is Not)
|
||||
|
||||
- **Not a `BastDoc` replacement.** `BastDoc` stays as the
|
||||
validation-side typed tree, borrowed from `&Value`, unchanged. The
|
||||
`Bast*` types and their signatures are not touched. Validation
|
||||
(`bast_validation`), aligned one-shot reads/writes
|
||||
(`engine.rs:334,467`), and `LayoutBuilder::build` continue to walk
|
||||
`BastDoc` until they get their own compiled forms (additive, later).
|
||||
- **Not a flat lookup table.** Packed positions are data-dependent;
|
||||
the plan is a read program (instructions to walk), not a `(path,
|
||||
offset)` table. This is inherent to packed sequential layout
|
||||
(ADR-002), not a limitation of this design.
|
||||
- **Not a validation plan.** `bast_validation`'s value-domain checks
|
||||
(maxLength, enum values, union variant keys, integer ranges) are a
|
||||
different concern and a different shape. They stay on `BastDoc`. A
|
||||
future `ValidationPlan` is a two-way door.
|
||||
- **Not the review's Option A or Option B.** It is the "compiled form"
|
||||
path the review pointed at but did not name: Option A (make `BastDoc`
|
||||
own its data) kills the re-parse but leaves the read loop as a
|
||||
`BastDoc` tree walk; Option B (precompute an owned read plan in
|
||||
`SequentialReader::new`) is the surgical subset that closes H1 only.
|
||||
This ADR is the principled version of B — a public `ReadPlan` built
|
||||
at `compile` time, shared across readers and `materialize`, symmetric
|
||||
with `OffsetMap`/`PackedLayout` — and it closes H1 + M1 (packed side)
|
||||
+ L1 + L2.
|
||||
|
||||
## Recommended Order
|
||||
|
||||
1. **`ReadPlan` type + `compile`** — the `ReadPlan`/`FieldPlan`/
|
||||
`CompositePlan`/`ReadKind`/`DiscriminatorPlan` types and the
|
||||
`ReadPlan::compile(&Value, &str)` constructor. Pure addition; no
|
||||
existing code touched. Unit-tested against the same BAST fixtures
|
||||
the `BastDoc` tests use.
|
||||
2. **`SequentialReader` rewrite** — the read loop walks `&ReadPlan`
|
||||
instead of reconstructing `BastDoc`. `read_field_value`,
|
||||
`read_typeref_value`, `walk_struct_size`, `read_union_value`,
|
||||
`read_array_value`, `read_record_value` take plan nodes. The
|
||||
existing `sequential_reader.rs` tests (which drive `read_next`/
|
||||
`read_field`/`reset` over real buffers) pass unchanged — they
|
||||
exercise the read path through the public API, so they validate
|
||||
the rewrite without modification.
|
||||
3. **`materialize_packed` rewrite** — takes `&ReadPlan`, walks the
|
||||
plan to produce `Value`. The existing `validate_bytes` (packed)
|
||||
tests cover it end-to-end.
|
||||
4. **`AlkTypeEngine::compile` integration** — builds `Arc<ReadPlan>`
|
||||
in packed mode, stores it, `sequential_reader()` hands out
|
||||
`Arc::clone(&self.plan)`. `validate_bytes` (packed) calls
|
||||
`materialize_packed(&self.plan, buffer)`.
|
||||
5. **L2 — retire the "re-parse on demand" framing.** Update
|
||||
ADR-007's "Cost" section (replace the "re-parse on demand"
|
||||
paragraph with the `Arc<ReadPlan>` cost) and the
|
||||
`src/engine.rs:112-115` doc comment. ADR-007's status block stays
|
||||
"Accepted" for the factory decision; only the cost framing changes.
|
||||
6. **Public API bump (0.2.0 → 0.3.0).** `lib.rs` re-exports `ReadPlan`;
|
||||
`SequentialReader::new` and `materialize_packed` signatures change.
|
||||
Update `alktty`/`alkcall` in the same commit.
|
||||
7. **Verification block.** `cargo test --release`,
|
||||
`cargo clippy --all-targets -- -D warnings`, `cargo doc --no-deps`
|
||||
(new public type), `cargo build --target wasm32-unknown-unknown
|
||||
--release` (the plan touches `sequential_reader.rs` and
|
||||
`materialize.rs`, both wasm-relevant). Re-run the `alktty`
|
||||
`wire_vs_bast` bench to confirm the 400x gap closes.
|
||||
|
||||
Steps 1–2 close H1. Step 3 closes the `materialize` half of M1.
|
||||
Step 4 closes the `validate_bytes` half of M1 (packed side). Step 5
|
||||
closes L2. L1 falls out at step 2. The aligned-side M1 paths
|
||||
(`engine.rs:334,467`, `layout_builder.rs:190`) are left as-is per
|
||||
"Out of scope."
|
||||
|
||||
## References
|
||||
|
||||
- [Review #004](../../reviews/004-performance-review.md) — the
|
||||
performance finding (H1, M1, L1, L2) and the three fix options this
|
||||
ADR supersedes
|
||||
- [ADR-002](002-two-layout-modes-packed-vs-aligned.md) — the two
|
||||
layout modes; `ReadPlan` is the packed read-side compiled form that
|
||||
this ADR adds to the table
|
||||
- [ADR-007](007-packed-mode-read-factory.md) — the engine as
|
||||
`SequentialReader` factory; retained (owned fresh reader), with the
|
||||
"re-parse on demand" framing retired (L2)
|
||||
- [ADR-004](004-error-handling-validation-strategy.md) — `AlkTypeError`,
|
||||
load-time build / access-time check, field-path-carrying errors;
|
||||
`ReadPlan::compile` is a load-time build, the read loop is an
|
||||
access-time check
|
||||
- [ADR-010](010-generalized-validation-validate-bytes.md) —
|
||||
`validate_bytes` (packed) consumes the `ReadPlan` via
|
||||
`materialize_packed`
|
||||
|
||||
## Future capabilities (not part of this ADR)
|
||||
|
||||
The deterministic-compile property of `ReadPlan` is a prerequisite for
|
||||
several capabilities that are explicitly out of scope here but worth
|
||||
naming so a future ADR doesn't re-derive the prerequisite:
|
||||
|
||||
- Fingerprinting the plan (e.g. `#[derive(Hash)]`) for cross-run
|
||||
caching of compiled plans.
|
||||
- Disk-cached compiled plans (skip `compile` on warm start).
|
||||
- Schema-version handshakes for `alkcall`'s hub/spoke topology —
|
||||
peers exchange plan fingerprints instead of full BAST documents.
|
||||
- A `WritePlan` and/or `ValidationPlan` that follow the same
|
||||
compile-once-walk-many pattern for the deferred M1 sites
|
||||
(`engine.rs:334,467`, `layout_builder.rs:190`, `bast_validation`).
|
||||
|
||||
None of these justify this ADR; the 400x read-path gap does. They are
|
||||
listed only as forward references.
|
||||
|
||||
## POC coverage
|
||||
|
||||
Before this ADR was accepted, a derisking POC on branch `readplan-poc`
|
||||
walked the read loop against every `BastType` arm in
|
||||
`src/sequential_reader.rs:303-381` (and the parallel arms in
|
||||
`materialize.rs`) and confirmed the `ReadPlan`/`CompositePlan`/
|
||||
`ReadKind`/`DiscriminatorPlan` shape covers all cases, including the
|
||||
two spots where a plan arm could subtly miss a case:
|
||||
|
||||
- **Union discriminator split (`Byte` vs `Field`).** Both are covered:
|
||||
`DiscriminatorPlan::Byte { offset, disc_type }` and
|
||||
`DiscriminatorPlan::Field { name, field_index }`. The field-name case
|
||||
pre-resolves the discriminator field's `ReadKind` so dispatch reads
|
||||
it from the plan, not from a re-parsed `BastField`.
|
||||
- **Array variable-element-stride (`element_stride = 0`).** Covered:
|
||||
`CompositePlan::Array { element, count, element_stride }` preserves
|
||||
the `0`-signals-variable convention, and the read loop walks
|
||||
sequentially when `element_stride == 0` (matching today's
|
||||
`walk_variable_array_size`).
|
||||
|
||||
The POC also confirmed `ReadPlan: Send + Sync` holds for the planned
|
||||
shape (immutable owned data, no interior mutability, no lifetimes).
|
||||
@@ -0,0 +1,412 @@
|
||||
---
|
||||
status: open
|
||||
last_updated: 2026-08-17
|
||||
reviewed_artifacts:
|
||||
- src/sequential_reader.rs
|
||||
- src/bast.rs
|
||||
- src/engine.rs
|
||||
- src/layout_builder.rs
|
||||
- src/materialize.rs
|
||||
- src/offset_map.rs
|
||||
- src/data_access.rs
|
||||
- src/lib.rs
|
||||
- docs/architecture/decisions/007-packed-mode-read-factory.md
|
||||
- ../@alkdev/alktty/benches/wire_vs_bast.rs
|
||||
tool: manual source read + downstream criterion bench (`cargo bench --bench wire_vs_bast` in alktty)
|
||||
reviewer: read-path performance review (triggered by alktty wire_vs_bast bench)
|
||||
---
|
||||
|
||||
# Review #004 — Read-Path Performance: the `BastDoc` Re-Parse Gap
|
||||
|
||||
## Purpose
|
||||
|
||||
A downstream bench in `alktty` (`benches/wire_vs_bast.rs`) compared a
|
||||
hand-rolled `ChunkHeader` codec against an alktype-driven codec built
|
||||
from the same 2-field BAST struct (`stream_type: uint8`,
|
||||
`length: uint32`, big-endian). The bench builds the engine / layout /
|
||||
reader **once** outside the measured loop, then measures per-chunk read
|
||||
and write over 1024 contiguous chunks.
|
||||
|
||||
The write path is competitive (~2.8x at 64 B, ~1.1x at 4 KiB — the
|
||||
per-chunk overhead is just `data_access::write_u8`/`write_u32` at
|
||||
precomputed `PackedLayout` offsets, and the payload copy dominates at
|
||||
4 KiB). The read path is **~400x slower per chunk** (2.27 µs/chunk vs
|
||||
5.6 ns/chunk hand-rolled), and the cost is fixed — it dominates at 64 B
|
||||
*and* at 4 KiB.
|
||||
|
||||
This review traces the gap to its source, confirms it is an
|
||||
implementation gap (not inherent to the design), and lays out the fix
|
||||
options. The bench itself is honest — its in-file note
|
||||
(`wire_vs_bast.rs:36-41`) already points at the root cause; this review
|
||||
formalizes the finding and the remediation plan.
|
||||
|
||||
## Methodology
|
||||
|
||||
- Full read of the read-path code (`sequential_reader.rs`, `bast.rs`,
|
||||
`engine.rs`), the write path (`layout_builder.rs`, `data_access.rs`),
|
||||
and ADR-007 (the packed read factory decision).
|
||||
- Cross-reference every `BastDoc::new` call site in `src/` to map the
|
||||
full re-parse surface.
|
||||
- Trace the lifetime/ownership constraint that forces the re-parse
|
||||
(`BastDoc<'a>` borrows `&'a Value`; `SequentialReader` owns its
|
||||
`Value` — self-referential struct, cannot cache the parsed tree).
|
||||
- Read the downstream bench to confirm the measurement is honest (the
|
||||
re-parse is inside the measured routine; the engine/reader are built
|
||||
once outside it).
|
||||
- Read `docs/reviews/003-code-review.md` for prior context — review #003
|
||||
did not flag the re-parse (it was a correctness/coverage pass, not a
|
||||
performance pass).
|
||||
|
||||
## Verification Baseline
|
||||
|
||||
The bench numbers below are from the alktty downstream tree
|
||||
(`benches/wire_vs_bast.rs`, criterion 0.7), run against alktype at
|
||||
commit `cab4932` (v0.2.0). The alktype tree itself is unchanged — this
|
||||
is a review of existing code, not a fix.
|
||||
|
||||
| path | payload=64 B | payload=4 KiB |
|
||||
|---|---|---|
|
||||
| hand-rolled read | 5.7 µs (5.6 ns/chunk) | 12.1 µs (11.8 ns/chunk) |
|
||||
| alktype read | 2.32 ms (2.27 µs/chunk) | 2.38 ms (2.33 µs/chunk) |
|
||||
| hand-rolled write | 14.3 µs | 321 µs |
|
||||
| alktype write | 39.8 µs | 355 µs |
|
||||
|
||||
One-shot startup costs (paid once): `AlkTypeEngine::compile` 573 µs,
|
||||
`engine.sequential_reader()` 2.84 µs, `LayoutBuilder::build` 1.30 µs.
|
||||
|
||||
The read gap is ~400x per chunk and is **fixed** (does not shrink as
|
||||
the payload grows), which is the signature of per-field overhead, not
|
||||
payload-copy overhead.
|
||||
|
||||
## Summary Statistics
|
||||
|
||||
| Severity | Count |
|
||||
|----------|------:|
|
||||
| High | 1 (H1) |
|
||||
| Medium | 1 (M1) |
|
||||
| Low | 2 (L1, L2) |
|
||||
| Nit | 1 (N1) |
|
||||
|
||||
The High finding is the read-path re-parse — a severe performance
|
||||
regression that blocks the primary intended use case (alktype as a
|
||||
runtime codec for protocol wire formats). It is not a correctness bug;
|
||||
the re-parse produces the same result every time, it is just
|
||||
catastrophically wasteful. The Medium finding is the same root cause
|
||||
manifesting in four one-shot paths. The Low/Nit findings are dead code
|
||||
and a stale doc comment exposed while tracing the root cause.
|
||||
|
||||
---
|
||||
|
||||
## Findings
|
||||
|
||||
### H1. `SequentialReader::read_field_at` re-parses the BAST typed tree on every field read
|
||||
|
||||
**File**: `src/sequential_reader.rs:262`
|
||||
|
||||
**Problem**: `read_field_at` calls `BastDoc::new(&self.doc_value,
|
||||
&self.root_name)?` on every field read. `BastDoc::new`
|
||||
(`src/bast.rs:69`) recursively parses the root def: `lookup_def_raw`
|
||||
(hash lookup into the `serde_json` Map), `BastDef::parse` →
|
||||
`BastStruct::parse` → allocates a `Vec<BastField>`, iterates fields,
|
||||
`BastField::parse` each (several `node.get().as_str()`/`as_u64()`
|
||||
calls + a `format!` allocation for the field path), `BastType::parse`
|
||||
each. For the 2-field `ChunkHeader`, that is ~1 µs per call.
|
||||
|
||||
`read_next` calls `read_field_at` once per field, so a 2-field header
|
||||
incurs **two** `BastDoc::new` calls per chunk = ~2.27 µs/chunk, matching
|
||||
the bench. For an N-field struct, reading all fields is O(N²) in parse
|
||||
work (each of N reads re-parses all N fields) — the gap widens with
|
||||
struct width.
|
||||
|
||||
The re-parse is purely redundant: `SequentialReader::new`
|
||||
(`src/sequential_reader.rs:130`) already parsed the `BastDoc` once at
|
||||
construction and extracted the field list. `read_field_at` re-derives
|
||||
the same `field_node` (the `BastField` at `index`) and the same `doc`
|
||||
that were already in hand at construction time.
|
||||
|
||||
**Root cause**: a lifetime/ownership tension, not a logic error.
|
||||
`BastDoc<'a>` borrows `&'a Value` and `&'a str` throughout
|
||||
(`src/bast.rs:51-55`). `SequentialReader` **owns** a cloned `Value`
|
||||
(`doc_value`, `src/sequential_reader.rs:111`). To cache the parsed
|
||||
`BastDoc` in the reader, the `BastDoc` would have to borrow from
|
||||
`self.doc_value` — a self-referential struct, which Rust's borrow
|
||||
checker forbids and safe Rust cannot express without a self-referential
|
||||
crate (`self_cell`/`ouroboros`, both rejected: new dep per AGENTS.md
|
||||
§7, and `self_cell`'s soundness relies on `unsafe` the crate avoids per
|
||||
AGENTS.md §11). So the only way to get a `BastDoc` at read time is to
|
||||
re-parse from the owned `Value`. The engine's own doc comment
|
||||
(`src/engine.rs:112-115`) acknowledges this design explicitly:
|
||||
|
||||
> "The engine retains a clone of the BAST `Value` so that
|
||||
> `sequential_reader` and `read_field` can re-parse the typed tree on
|
||||
> demand without lifetime entanglement with the caller's `Value`."
|
||||
|
||||
The "re-parse on demand" framing was a lifetime-entanglement workaround
|
||||
that did not anticipate the hot-loop cost. ADR-007's "Cost" section
|
||||
(`decisions/007-packed-mode-read-factory.md:64-70`) argues construction
|
||||
is cheap ("a `Vec<(String, Value)>` of the `properties` entries ... a
|
||||
struct has a small number of fields") — true for *construction* (once),
|
||||
but the decision did not account for a per-field re-parse inside the
|
||||
read loop.
|
||||
|
||||
**Why the write path is fine**: `LayoutBuilder::build`
|
||||
(`src/layout_builder.rs:190`) also re-parses `BastDoc::new`, but it is
|
||||
called **once** per write — the resulting `PackedLayout` offsets are
|
||||
cached and reused. The write loop then does only `data_access::write_*`
|
||||
at fixed offsets. There is no per-field re-parse on the write hot path,
|
||||
which is why write is ~1.1x at 4 KiB.
|
||||
|
||||
**Fix**: cache the parsed typed tree so the read path does not re-parse.
|
||||
See "Fix Options" below for the three approaches and the
|
||||
recommendation.
|
||||
|
||||
**Lift**: closes a ~400x read-path gap and unblocks alktype as a runtime
|
||||
codec for protocol wire formats (its stated purpose per ADR-001). This
|
||||
is the difference between "plausible runtime codec" and "not viable."
|
||||
Large effort depending on the chosen option.
|
||||
|
||||
---
|
||||
|
||||
### M1. The same re-parse pattern exists in four one-shot paths
|
||||
|
||||
**Files**: `src/layout_builder.rs:190`, `src/engine.rs:284,334,467`
|
||||
|
||||
**Problem**: `BastDoc::new(&self.doc_value, &self.root_name)?` /
|
||||
`BastDoc::new(&self.bast_doc, &self.root_name)?` is re-called in:
|
||||
|
||||
- `LayoutBuilder::build` (`src/layout_builder.rs:190`) — once per
|
||||
`build()` call. Re-parsing on each build is wasteful if a builder is
|
||||
reused across writes, but the typical pattern is build-once-reuse,
|
||||
so this is mild.
|
||||
- `AlkTypeEngine::validate_bytes` (`src/engine.rs:284`) — once per
|
||||
validate call. For a stream of buffers, this is a per-buffer re-parse.
|
||||
- `AlkTypeEngine::read_field` (`src/engine.rs:334`, aligned mode) — once
|
||||
per field read. Same class as H1 but for aligned random access, and
|
||||
one parse per field read (not the O(N²) of the sequential reader).
|
||||
- `AlkTypeEngine::write_field` (`src/engine.rs:467`, aligned mode) —
|
||||
once per field write.
|
||||
|
||||
These are less acute than H1 (one parse per operation, not per-field-
|
||||
in-a-loop), but they share the same root cause: the engine/builder own
|
||||
a `Value` and cannot cache a borrowing `BastDoc`. Any fix that makes
|
||||
`BastDoc` cacheable on an owning struct (Fix Option A) closes these for
|
||||
free; a read-path-only fix (Option B) leaves them as-is, which is
|
||||
acceptable since they are not hot loops.
|
||||
|
||||
**Lift**: removes redundant parse work on the validate/aligned paths.
|
||||
Free with Option A; deferred with Option B.
|
||||
|
||||
---
|
||||
|
||||
### L1. `read_field_value` carries a dead `_field_schema` parameter; `fields` stores dead `Value` clones
|
||||
|
||||
**Files**: `src/sequential_reader.rs:114,294`
|
||||
|
||||
**Problem**: `SequentialReader` stores `fields: Vec<(String, Value)>`
|
||||
where the `Value` is `f.source().clone()` per field
|
||||
(`src/sequential_reader.rs:145`). `read_field_at` passes this as
|
||||
`_field_schema` to `read_field_value` (`src/sequential_reader.rs:294`),
|
||||
where it is unused (prefixed `_`). The raw `Value` clone per field is
|
||||
dead weight — only the `String` name is used (for `read_next`'s return
|
||||
and `read_field`'s lookup). This is a minor allocation cost on top of
|
||||
H1's re-parse, and it will be removed naturally when the read plan is
|
||||
precomputed (Option B) or the `BastDoc` is cached (Option A), since
|
||||
both replace `Vec<(String, Value)>` with typed/owned field data.
|
||||
|
||||
**Lift**: trivial; falls out of the H1 fix.
|
||||
|
||||
---
|
||||
|
||||
### L2. ADR-007 "Cost" section and the engine doc comment understate the re-parse
|
||||
|
||||
**Files**: `docs/architecture/decisions/007-packed-mode-read-factory.md:64-70`,
|
||||
`src/engine.rs:112-115`
|
||||
|
||||
**Problem**: ADR-007's "Cost" section argues `sequential_reader()` is
|
||||
cheap because it clones a small `Vec` of field schemas. That is true for
|
||||
the factory call (once). But the decision did not anticipate that
|
||||
`read_field_at` would re-parse `BastDoc::new` per field — the cost that
|
||||
actually dominates. The engine doc comment at `src/engine.rs:112-115`
|
||||
explicitly frames re-parse-on-demand as the intended design ("re-parse
|
||||
the typed tree on demand without lifetime entanglement"), which is the
|
||||
root cause H1 traces.
|
||||
|
||||
**Fix**: whichever fix option is chosen, update ADR-007's "Cost" /
|
||||
"Consequences" section and the engine doc comment to reflect that the
|
||||
parsed tree is now cached (Option A) or precomputed into a read plan
|
||||
(Option B), and that the "re-parse on demand" framing is retired.
|
||||
|
||||
**Lift**: documentation accuracy; prevents the same framing from
|
||||
misleading a future edit.
|
||||
|
||||
---
|
||||
|
||||
### N1. `BastType::alk_kind()` returns `Struct` for any `$ref` (carry-forward from review #003 N2)
|
||||
|
||||
**File**: `src/bast.rs:715-725`
|
||||
|
||||
**Problem**: flagged in review #003 N2 and left as "defer unless it
|
||||
bites." It does not bite here — `read_field_value` always calls
|
||||
`doc.resolve_typeref(ty)` before matching on `BastType`, so the
|
||||
misreporting `alk_kind` is never consulted on a `Ref`. Noting it only
|
||||
because this review re-read the same path; no new action beyond review
|
||||
#003's deferral.
|
||||
|
||||
---
|
||||
|
||||
## Fix Options
|
||||
|
||||
The core constraint: `BastDoc<'a>` borrows `&'a Value` / `&'a str`; an
|
||||
owning struct (`SequentialReader`, `AlkTypeEngine`, `LayoutBuilder`)
|
||||
cannot store a `BastDoc` that borrows from its own `Value` field
|
||||
(self-referential). Three ways to break the constraint:
|
||||
|
||||
### Option A — Make the typed tree own its data (principled fix)
|
||||
|
||||
Change `BastDoc<'a>` → `BastDoc` (no lifetime), `&'a str` → `Arc<str>`
|
||||
(or `String`), `&'a Value` → `Arc<Value>` (or `Value`). Then
|
||||
`SequentialReader`, `AlkTypeEngine`, and `LayoutBuilder` each hold a
|
||||
`BastDoc` directly (built once at construction), and `read_field_at`
|
||||
uses `&self.doc` — no re-parse, anywhere.
|
||||
|
||||
- **Closes**: H1, M1 (all four one-shot paths), and the engine/reader
|
||||
lifetime entanglement that ADR-007 worked around. The engine's
|
||||
`bast_doc: Value` clone (`src/engine.rs:85`) becomes redundant with
|
||||
the owned `BastDoc`.
|
||||
- **Tradeoff**: broad refactor. Touches `bast.rs` (every typed node)
|
||||
and every consumer (`layout_builder`, `offset_map`, `materialize`,
|
||||
`sequential_reader`, `tunion`, `bast_validation`, `engine`). The
|
||||
`Bast*` types are re-exported in `lib.rs:57-60`, so this is a
|
||||
**breaking public-API change** — `BastDoc<'a>` becomes `BastDoc`,
|
||||
and every method signature that took `&'a` changes. Per AGENTS.md,
|
||||
this is semver-relevant and would need a version bump (0.2.0 → 0.3.0).
|
||||
- **Dependency cost**: `Arc<str>`/`Arc<Value>` add `alloc` (already in
|
||||
use via `Vec`/`String`); no new external deps. Stays wasm-clean. The
|
||||
`preserve_order` serde_json feature remains load-bearing (AGENTS.md
|
||||
§8) — owning the `Value` does not change field-order semantics.
|
||||
- **Effort**: large but mechanical. The borrow-based design was chosen
|
||||
for "allocation-free beyond the small typed nodes" (`src/bast.rs:11-
|
||||
18`), but the re-parse-per-field already defeats that goal by
|
||||
allocating a fresh `Vec<BastField>` per read. Owning the data makes
|
||||
the "parse once, walk many times" invariant actually hold.
|
||||
|
||||
### Option B — Precompute an owned read plan in `SequentialReader::new` (surgical fix)
|
||||
|
||||
Keep `BastDoc<'a>` borrowing for the other consumers. In
|
||||
`SequentialReader::new`, parse the `BastDoc` once, resolve all `$ref`s
|
||||
eagerly, and build a flat, owned tree of read instructions
|
||||
(`Vec<FieldPlan>`) that the read loop walks with no `BastDoc`
|
||||
involvement. Each `FieldPlan` carries the field name, the resolved
|
||||
`AlkTypeKind`, the effective `Endian`, and for composites a nested
|
||||
plan (struct → sub-plans; union → discriminator + per-variant plans;
|
||||
array → element plan + count; record → value plan).
|
||||
|
||||
- **Closes**: H1 only. M1 (the one-shot re-parses) remains, which is
|
||||
acceptable since they are not hot loops.
|
||||
- **Tradeoff**: non-breaking (internal to `sequential_reader.rs`; the
|
||||
public `SequentialReader` type and its methods keep their
|
||||
signatures). Duplicates some of the `BastType` matching logic that
|
||||
`read_field_value`/`read_union_value`/etc. already encode, so there
|
||||
are two parallel walkers to maintain.
|
||||
- **Effort**: medium. Self-contained in one file but non-trivial
|
||||
(composites require recursively resolving and pre-flattening the
|
||||
type tree, including `$ref` chains into `$defs`).
|
||||
|
||||
### Option C — Cache `BastDoc` on the engine, reader borrows (rejected)
|
||||
|
||||
Have the engine own the parsed `BastDoc` and return a
|
||||
`SequentialReader<'_>` that borrows from `&self`. This requires
|
||||
`BastDoc` to be owned (Option A prerequisite) *and* changes
|
||||
`sequential_reader() -> Option<SequentialReader>` to
|
||||
`-> Option<SequentialReader<'_>>` — a breaking public-API change that
|
||||
also contradicts ADR-007's "owned fresh reader" decision. Strictly
|
||||
worse than Option A (same refactor cost, more API churn, contradicts
|
||||
an ADR). Rejected.
|
||||
|
||||
### Recommendation
|
||||
|
||||
**Option A**, given the publisher's stated willingness to make breaking
|
||||
changes ("no one is using this except us yet; ... we can change things
|
||||
now"). It is the only option that closes H1 *and* M1 and retires the
|
||||
lifetime-entanglement workaround that caused both. The refactor is
|
||||
broad but mechanical (lifetime removal, not logic rewrites), and the
|
||||
crate is pre-1.0 with only two in-house downstream consumers
|
||||
(`alktty`, `alkcall`), so the breakage cost is bounded and known.
|
||||
|
||||
Option B is the fallback if the Option A refactor is deferred — it
|
||||
closes the acute H1 gap non-breakingly while leaving M1 for later. It
|
||||
is not the recommended path because it leaves a second parallel type
|
||||
walker in the crate and does not address the root cause (the borrow-
|
||||
based `BastDoc` design), which will keep forcing re-parses anywhere a
|
||||
new owning consumer wants to cache the parsed tree.
|
||||
|
||||
Regardless of the chosen option, ADR-007's "Cost"/"Consequences"
|
||||
section and the `src/engine.rs:112-115` doc comment should be updated to
|
||||
retire the "re-parse on demand" framing (L2).
|
||||
|
||||
---
|
||||
|
||||
## What's Good
|
||||
|
||||
- **The bench is honest.** The alktty bench builds the engine/reader
|
||||
once outside the measured loop and correctly isolates the per-chunk
|
||||
logic. Its in-file note (`wire_vs_bast.rs:36-41`) already points at
|
||||
the `read_field_at` re-parse and labels it "the honest current cost of
|
||||
the alktype read path, not a bench bug." This review confirms that
|
||||
assessment.
|
||||
- **The write path is already competitive.** Once `PackedLayout` is
|
||||
built, the write loop is just `data_access::write_*` at fixed offsets
|
||||
— no schema walk, no re-parse. This validates the "build once, reuse"
|
||||
pattern that the read path should also adopt.
|
||||
- **`resolve_typeref` for primitives is cheap.** `BastType::Primitive`
|
||||
is `Copy` (`AlkTypeKind: Copy`, `src/schema.rs:25`), so
|
||||
`resolve_typeref` (`src/bast.rs:117-125`) returns `other.clone()`
|
||||
without allocation for the common case. The re-parse cost is entirely
|
||||
in `BastDoc::new`, not in the per-field type resolution — so caching
|
||||
the `BastDoc` alone closes the gap without restructuring
|
||||
`resolve_typeref`.
|
||||
- **Overflow safety and error attribution are unaffected.** The
|
||||
`checked_add` / `usize::try_from` discipline (AGENTS.md §4) and the
|
||||
`field_path`-carrying errors (review #003 "What's Good") are in the
|
||||
read functions, not the parser — a caching fix preserves them.
|
||||
|
||||
---
|
||||
|
||||
## Recommended Order
|
||||
|
||||
1. **H1 + M1 (Option A)** — the owned-typed-tree refactor. Decide
|
||||
first (this is a one-way door: breaking public-API change, version
|
||||
bump to 0.3.0). If approved, this is one refactor that closes both.
|
||||
2. **L2** — update ADR-007 and the engine doc comment in the same
|
||||
commit as the H1 fix, since the "re-parse on demand" framing is
|
||||
being retired.
|
||||
3. **L1** — falls out of the H1 fix (the dead `Value` clones are
|
||||
replaced by the cached/owned field data).
|
||||
4. **N1** — remains deferred per review #003.
|
||||
|
||||
If Option A is deferred, **H1 (Option B)** is the standalone
|
||||
alternative — non-breaking, closes the acute gap only.
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- All line numbers refer to the tree at commit `cab4932` (v0.2.0, the
|
||||
BAST pivot release).
|
||||
- The bench is in the `alktty` downstream repo
|
||||
(`/workspace/@alkdev/alktty/benches/wire_vs_bast.rs`), not in alktype.
|
||||
alktty depends on alktype as a path dev-dep for the bench only; it
|
||||
does not use alktype at runtime. The path dep means
|
||||
`cargo publish --dry-run` for alktty would complain (the bench is
|
||||
exploratory and uncommitted in alktty; the alktype crate itself has
|
||||
no bench dependency).
|
||||
- This review does not cover the wasm build (`cargo build --target
|
||||
wasm32-unknown-unknown`) because the fix is not yet implemented; the
|
||||
verification block for the fix should include it per AGENTS.md, as
|
||||
the typed-tree ownership change touches `bast.rs` which is
|
||||
wasm-relevant.
|
||||
- Review #003 (post-BAST-pivot correctness review) did not flag the
|
||||
re-parse — its scope was correctness, coverage, and panic safety, not
|
||||
performance. The re-parse is not a correctness regression; the parsed
|
||||
tree is identical across calls. This review complements #003 by
|
||||
adding the performance axis.
|
||||
@@ -0,0 +1,13 @@
|
||||
[package]
|
||||
name = "readplan-poc"
|
||||
version = "0.0.0"
|
||||
edition = "2021"
|
||||
publish = false
|
||||
authors = ["poc"]
|
||||
|
||||
[dependencies]
|
||||
alktype = { path = "../.." }
|
||||
serde_json = { version = "1", features = ["preserve_order"] }
|
||||
|
||||
[lib]
|
||||
name = "readplan_poc"
|
||||
@@ -0,0 +1,156 @@
|
||||
# ReadPlan POC — findings
|
||||
|
||||
Branch: `readplan-poc`
|
||||
ADR: [ADR-011](../../docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md)
|
||||
Status: **ADR-011 accepted based on this POC.** 28/30 tests pass; 2
|
||||
deliberately ignored with documented findings.
|
||||
|
||||
## Objective
|
||||
|
||||
Before accepting ADR-011 and starting implementation, derisk the
|
||||
`ReadPlan`/`CompositePlan`/`ReadKind`/`DiscriminatorPlan` shape by:
|
||||
|
||||
1. Walking every `BastType` arm in the current read loop
|
||||
(`sequential_reader.rs:303-381`) and confirming `ReadPlan::compile`
|
||||
produces a plan that covers it.
|
||||
2. Driving both the existing `SequentialReader` and a plan-driven
|
||||
reader over the same buffers and asserting identical
|
||||
`(FieldValue, position)` results.
|
||||
|
||||
If both pass, the shape is confirmed and implementation can proceed by
|
||||
replacing the `BastDoc` walk in `sequential_reader.rs` and
|
||||
`materialize.rs` with a `ReadPlan` walk.
|
||||
|
||||
## What the POC is
|
||||
|
||||
- `poc/readplan/` — a standalone workspace member (`readplan-poc`
|
||||
crate) that depends on `alktype` via path dep.
|
||||
- `src/lib.rs` — `ReadPlan`/`FieldPlan`/`CompositePlan`/`ReadKind`/
|
||||
`DiscriminatorPlan` matching ADR-011's shape, `ReadPlan::compile`
|
||||
walking `BastDoc` once, and `plan_read_field_at`/`plan_walk_struct_size`
|
||||
mirroring the existing read loop arm-by-arm.
|
||||
- `tests/coverage.rs` — `cov_*` coverage tests (one per `BastType` arm)
|
||||
+ `eq_*` equivalence tests (plan vs existing `SequentialReader`).
|
||||
|
||||
The POC is deliberately not production code: no doc comments on the
|
||||
plan types beyond the module header, no clippy-cleanliness gate, no
|
||||
bench. It exists to answer two questions and stop.
|
||||
|
||||
## Result
|
||||
|
||||
**28/30 tests pass. 2 ignored, both with documented findings.**
|
||||
|
||||
### Coverage — all BastType arms confirmed
|
||||
|
||||
Every `BastType` arm in the current read loop compiles to the expected
|
||||
`ReadKind`/`CompositePlan` shape:
|
||||
|
||||
| BastType arm | ReadKind | CompositePlan | cov test |
|
||||
|---|---|---|---|
|
||||
| Primitive (Int8..Float64, Bool, String, Bytes) | `Primitive(k)` | `None` | `cov_all_fixed_primitives`, `cov_string_and_bytes` |
|
||||
| Enum (via `$ref`) | `Enum` | `None` | `cov_enum_ref` |
|
||||
| Struct (inline) | `Struct` | `Struct(ReadPlan)` | `cov_struct_inline` |
|
||||
| Struct (via `$ref`) | `Struct` | `Struct(ReadPlan)` | `cov_struct_ref` |
|
||||
| Union (byte disc) | `Union` | `Union { disc: Byte, variants }` | `cov_union_byte_disc` |
|
||||
| Union (field disc) | `Union` | `Union { disc: Field, variants }` | `cov_union_field_disc` |
|
||||
| Array (fixed elem) | `Array` | `Array { element, count, stride }` | `cov_array_fixed_element` |
|
||||
| Array (variable elem, stride=0) | `Array` | `Array { element, count, stride: 0 }` | `cov_array_variable_element_stride_zero` |
|
||||
| Array (`$ref` elem) | `Array` | `Array { element: Struct, count, stride }` | `cov_array_ref_element` |
|
||||
| Record | `Record` | `Record { value: Struct-wrapped leaf }` | `cov_record` |
|
||||
|
||||
Field-level annotations (`endian` override, `encoding`, `maxLength`)
|
||||
are preserved by `compile_field` — covered by `cov_field_level_endian_override`
|
||||
and `cov_maxlength_and_encoding_preserved`.
|
||||
|
||||
### Equivalence — plan == existing reader for every covered arm
|
||||
|
||||
`eq_*` tests drive both readers over the same buffer and assert
|
||||
identical `(FieldValue, position)` for every field. All pass except
|
||||
the two ignored ones below.
|
||||
|
||||
### Send + Sync
|
||||
|
||||
`ReadPlan: Send + Sync` holds for the planned shape — confirmed by
|
||||
`readplan_is_send_sync`. Falls out naturally from the plan being
|
||||
immutable owned data with no lifetimes and no interior mutability.
|
||||
|
||||
## Findings the POC surfaced
|
||||
|
||||
### Finding 1 — Field-name-discriminator union read shape needs work
|
||||
|
||||
**Status:** POC TODO (ignored test `eq_union_field_discriminator_todo`).
|
||||
|
||||
`compile_union` correctly records `DiscriminatorPlan::Field { name, field_index }`
|
||||
and the variant plans, but `plan_read_union`'s `Field` arm is a stub
|
||||
that returns an error. The field-name case is structurally different
|
||||
from the byte-offset case: the union declares its own `fields` array
|
||||
(the discriminator field + any shared fields), and the variant struct
|
||||
is laid out *after* those shared fields. The plan needs a sub-struct
|
||||
for the union's declared fields, separate from the variant plans.
|
||||
|
||||
**This is not a plan-shape gap** — `DiscriminatorPlan::Field` and the
|
||||
variant `ReadPlan`s compile correctly. It's a read-shape TODO that
|
||||
the implementation step 1 must wire. The POC stubs it to surface the
|
||||
work explicitly rather than hide it.
|
||||
|
||||
### Finding 2 — Existing reader returns `stride=0` for fixed-size struct arrays
|
||||
|
||||
**Status:** POC FINDING (ignored test `eq_array_ref_element`).
|
||||
|
||||
The existing `SequentialReader` returns `element_stride: 0` for an
|
||||
array of `$ref`-to-fixed-struct elements, even when the struct is
|
||||
fixed-size (e.g. `Point { x: u16, y: u16 }` is 4 bytes). See
|
||||
`sequential_reader.rs:567`:
|
||||
|
||||
```rust
|
||||
let element_stride = if elem_kind.is_fixed_size() {
|
||||
elem_kind.type_size().unwrap_or(0)
|
||||
} else {
|
||||
0
|
||||
};
|
||||
```
|
||||
|
||||
`elem_kind` is `resolved_elem.alk_kind()`, which returns
|
||||
`AlkTypeKind::Struct` for a `$ref` to a struct. `Struct.is_fixed_size()`
|
||||
is `false`, so the existing code returns `0`. The POC's
|
||||
`fixed_struct_size` helper correctly computes `4`.
|
||||
|
||||
The implementation step must decide:
|
||||
- (a) preserve the existing `stride=0` behavior for back-compat
|
||||
(consumer walks sequentially), or
|
||||
- (b) fix the existing reader to return the true fixed-struct stride
|
||||
and let consumers index directly.
|
||||
|
||||
Either way, **the `ReadPlan` shape is correct** — this is a pre-existing
|
||||
reader limitation, not a plan-shape gap. Recording it so the
|
||||
implementation step makes a deliberate choice rather than inheriting
|
||||
the old behavior by accident.
|
||||
|
||||
## What this POC does *not* cover (out of scope, by design)
|
||||
|
||||
- **Performance.** No bench. The bench that matters lives in alktty's
|
||||
`wire_vs_bast.rs`; the implementation commit re-runs it. The POC only
|
||||
proves correctness/coverage.
|
||||
- **Aligned mode, validation, write side.** ADR-011 scopes these out.
|
||||
- **`materialize_packed` equivalence.** The POC covers `SequentialReader`
|
||||
equivalence; `materialize_packed` uses the same `BastType` arms via
|
||||
`materialize_typeref_packed` and will be covered by the implementation
|
||||
step's existing `validate_bytes` tests.
|
||||
- **Nested unions (union variant is itself a union).** The POC's
|
||||
`compile_union` rejects this with a clear error. The existing reader
|
||||
supports it via `resolve_and_walk_variant`'s `BastDefKind::Union` arm;
|
||||
if a real schema needs it, the implementation step adds a
|
||||
`VariantKind::Union` read path. Not blocking — no current schema
|
||||
exercises it.
|
||||
|
||||
## Conclusion
|
||||
|
||||
ADR-011's `ReadPlan` shape covers every `BastType` arm and produces
|
||||
identical results to the existing reader for all covered cases. The
|
||||
two ignored tests document deliberate scope boundaries (field-disc
|
||||
union read shape) and a pre-existing reader limitation (struct-array
|
||||
stride), neither of which is a plan-shape gap.
|
||||
|
||||
**Green light for ADR-011 implementation.** The implementation step 1
|
||||
(`ReadPlan` type + `compile`) can proceed, with the field-disc union
|
||||
read shape and the struct-array stride decision as explicit sub-tasks.
|
||||
@@ -0,0 +1,733 @@
|
||||
//! ReadPlan POC — derisking ADR-011 before implementation.
|
||||
//!
|
||||
//! Lives on branch `readplan-poc`. Not production code. The objective is
|
||||
//! to answer two questions, not to be pretty:
|
||||
//!
|
||||
//! 1. Does the `ReadPlan`/`CompositePlan`/`ReadKind`/`DiscriminatorPlan`
|
||||
//! shape from ADR-011 cover *every* `BastType` arm in the current read
|
||||
//! loop (`sequential_reader.rs:303-381`) and the parallel arms in
|
||||
//! `materialize.rs`?
|
||||
//! 2. Does a plan-driven read loop produce identical `(FieldValue, position)`
|
||||
//! results to the existing `SequentialReader::read_field_at` across a
|
||||
//! battery of schemas covering every arm?
|
||||
//!
|
||||
//! If both answers are yes, ADR-011's shape is confirmed and the
|
||||
//! implementation can proceed by replacing the `BastDoc` walk in
|
||||
//! `sequential_reader.rs` and `materialize.rs` with the plan walk.
|
||||
//!
|
||||
//! What this POC is *not*:
|
||||
//! - It is not the production `ReadPlan`. The production version lives
|
||||
//! in `src/` and is unit-tested against the BAST fixtures there.
|
||||
//! - It is not performance-tuned. The bench that matters lives in
|
||||
//! alktty's `wire_vs_bast.rs`; this POC only proves correctness/coverage.
|
||||
//! - It does not handle aligned mode, validation, or the write side —
|
||||
//! ADR-011 scopes those out.
|
||||
|
||||
use alktype::bast::{
|
||||
BastArray, BastDefKind, BastDiscriminator, BastDoc, BastField, BastRecord, BastStruct,
|
||||
BastType, BastUnion,
|
||||
};
|
||||
use alktype::data_access;
|
||||
use alktype::error::AlkTypeError;
|
||||
use alktype::schema::{AlkTypeKind, Endian, VariableEncoding};
|
||||
use alktype::sequential_reader::FieldValue;
|
||||
use serde_json::Value;
|
||||
|
||||
const U32_SIZE: usize = 4;
|
||||
|
||||
pub struct ReadPlan {
|
||||
endian: Endian,
|
||||
fields: Vec<FieldPlan>,
|
||||
by_name: std::collections::HashMap<String, usize>,
|
||||
}
|
||||
|
||||
pub struct FieldPlan {
|
||||
name: String,
|
||||
kind: ReadKind,
|
||||
endian: Endian,
|
||||
encoding: VariableEncoding,
|
||||
max_length: Option<usize>,
|
||||
body: Option<CompositePlan>,
|
||||
}
|
||||
|
||||
pub enum ReadKind {
|
||||
Primitive(AlkTypeKind),
|
||||
Enum,
|
||||
Struct,
|
||||
Union,
|
||||
Array,
|
||||
Record,
|
||||
}
|
||||
|
||||
pub enum CompositePlan {
|
||||
Struct(ReadPlan),
|
||||
Union {
|
||||
disc: DiscriminatorPlan,
|
||||
variants: Vec<(String, VariantPlan)>,
|
||||
},
|
||||
Array {
|
||||
element: Box<CompositePlan>,
|
||||
count: usize,
|
||||
element_stride: usize,
|
||||
},
|
||||
Record {
|
||||
value: Box<CompositePlan>,
|
||||
},
|
||||
}
|
||||
|
||||
pub enum DiscriminatorPlan {
|
||||
Byte { offset: usize, disc_type: AlkTypeKind },
|
||||
Field { name: String, field_index: usize },
|
||||
}
|
||||
|
||||
pub struct VariantPlan {
|
||||
kind: VariantKind,
|
||||
plan: ReadPlan,
|
||||
}
|
||||
|
||||
pub enum VariantKind {
|
||||
Struct,
|
||||
Union,
|
||||
}
|
||||
|
||||
impl ReadPlan {
|
||||
pub fn compile(bast_doc: &Value, root_name: &str) -> Result<Self, AlkTypeError> {
|
||||
let doc = BastDoc::new(bast_doc, root_name)?;
|
||||
let root_def = doc.root_def();
|
||||
let struct_node = match root_def.kind() {
|
||||
BastDefKind::Struct(s) => s,
|
||||
other => {
|
||||
return Err(AlkTypeError::Schema(format!(
|
||||
"ReadPlan root must be a struct, got {kind}",
|
||||
kind = other.alk_kind()
|
||||
)));
|
||||
}
|
||||
};
|
||||
let plan = Self::compile_struct(&doc, struct_node, struct_node.endian())?;
|
||||
Ok(plan)
|
||||
}
|
||||
|
||||
fn compile_struct(
|
||||
doc: &BastDoc<'_>,
|
||||
struct_node: &BastStruct<'_>,
|
||||
container_endian: Endian,
|
||||
) -> Result<Self, AlkTypeError> {
|
||||
let mut fields = Vec::new();
|
||||
let mut by_name = std::collections::HashMap::new();
|
||||
for (i, field) in struct_node.fields().iter().enumerate() {
|
||||
let field_plan = Self::compile_field(doc, field, container_endian)?;
|
||||
by_name.insert(field_plan.name.clone(), i);
|
||||
fields.push(field_plan);
|
||||
}
|
||||
Ok(Self {
|
||||
endian: container_endian,
|
||||
fields,
|
||||
by_name,
|
||||
})
|
||||
}
|
||||
|
||||
fn compile_field(
|
||||
doc: &BastDoc<'_>,
|
||||
field: &BastField<'_>,
|
||||
container_endian: Endian,
|
||||
) -> Result<FieldPlan, AlkTypeError> {
|
||||
let endian = field.effective_endian(container_endian);
|
||||
let ty = field.ty();
|
||||
let resolved = doc.resolve_typeref(ty)?;
|
||||
let (kind, body) = Self::compile_kind(doc, &resolved, endian)?;
|
||||
Ok(FieldPlan {
|
||||
name: field.name().to_string(),
|
||||
kind,
|
||||
endian,
|
||||
encoding: field.encoding(),
|
||||
max_length: field.max_length(),
|
||||
body,
|
||||
})
|
||||
}
|
||||
|
||||
fn compile_kind(
|
||||
doc: &BastDoc<'_>,
|
||||
ty: &BastType<'_>,
|
||||
container_endian: Endian,
|
||||
) -> Result<(ReadKind, Option<CompositePlan>), AlkTypeError> {
|
||||
match ty {
|
||||
BastType::Primitive(k) => Ok((ReadKind::Primitive(*k), None)),
|
||||
BastType::Enum(_) => Ok((ReadKind::Enum, None)),
|
||||
BastType::Struct(s) => {
|
||||
let plan = Self::compile_struct(doc, s, s.endian())?;
|
||||
Ok((ReadKind::Struct, Some(CompositePlan::Struct(plan))))
|
||||
}
|
||||
BastType::Union(u) => {
|
||||
let plan = Self::compile_union(doc, u)?;
|
||||
Ok((ReadKind::Union, Some(plan)))
|
||||
}
|
||||
BastType::Array(a) => {
|
||||
let plan = Self::compile_array(doc, a, container_endian)?;
|
||||
Ok((ReadKind::Array, Some(plan)))
|
||||
}
|
||||
BastType::Record(r) => {
|
||||
let plan = Self::compile_record(doc, r, container_endian)?;
|
||||
Ok((ReadKind::Record, Some(plan)))
|
||||
}
|
||||
BastType::Ref(_) => Err(AlkTypeError::Schema(
|
||||
"internal: compile_kind saw an unresolved $ref".to_string(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn compile_union(doc: &BastDoc<'_>, u: &BastUnion<'_>) -> Result<CompositePlan, AlkTypeError> {
|
||||
let disc = match u.discriminator() {
|
||||
BastDiscriminator::Byte { offset, disc_type } => DiscriminatorPlan::Byte {
|
||||
offset: *offset,
|
||||
disc_type: *disc_type,
|
||||
},
|
||||
BastDiscriminator::Field { name } => {
|
||||
let idx = u
|
||||
.fields()
|
||||
.iter()
|
||||
.position(|f| f.name() == *name)
|
||||
.ok_or_else(|| {
|
||||
AlkTypeError::Schema(format!(
|
||||
"union has no discriminator field '{name}'"
|
||||
))
|
||||
})?;
|
||||
DiscriminatorPlan::Field {
|
||||
name: name.to_string(),
|
||||
field_index: idx,
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let mut variants = Vec::new();
|
||||
for (key, variant_ty) in u.mapping() {
|
||||
let variant_def = doc.resolve_typeref_as_def(variant_ty, "")?;
|
||||
let kind = match variant_def.kind() {
|
||||
BastDefKind::Struct(s) => {
|
||||
let plan = Self::compile_struct(doc, s, s.endian())?;
|
||||
(VariantKind::Struct, plan)
|
||||
}
|
||||
BastDefKind::Union(inner_u) => {
|
||||
let _ = inner_u;
|
||||
return Err(AlkTypeError::Schema(format!(
|
||||
"union variant '{key}' is itself a union — nested unions not yet supported by ReadPlan compile"
|
||||
)));
|
||||
}
|
||||
BastDefKind::Enum(_) => {
|
||||
return Err(AlkTypeError::Schema(format!(
|
||||
"union variant '{key}' is an enum — variants must be struct or union"
|
||||
)));
|
||||
}
|
||||
};
|
||||
variants.push((key.to_string(), VariantPlan { kind: kind.0, plan: kind.1 }));
|
||||
}
|
||||
|
||||
Ok(CompositePlan::Union { disc, variants })
|
||||
}
|
||||
|
||||
fn compile_array(
|
||||
doc: &BastDoc<'_>,
|
||||
a: &BastArray<'_>,
|
||||
container_endian: Endian,
|
||||
) -> Result<CompositePlan, AlkTypeError> {
|
||||
let element_ty = a.element();
|
||||
let resolved_elem = doc.resolve_typeref(element_ty)?;
|
||||
let count = a.count();
|
||||
|
||||
let element_stride = match &resolved_elem {
|
||||
BastType::Struct(s) => Self::fixed_struct_size(doc, s)?,
|
||||
BastType::Array(inner_a) => {
|
||||
let inner_stride = match inner_a.element() {
|
||||
e if doc.resolve_typeref(e)?.alk_kind().is_fixed_size() => {
|
||||
doc.resolve_typeref(e)?.alk_kind().type_size().unwrap_or(0)
|
||||
}
|
||||
_ => 0,
|
||||
};
|
||||
if inner_stride == 0 {
|
||||
0
|
||||
} else {
|
||||
inner_a
|
||||
.count()
|
||||
.checked_mul(inner_stride)
|
||||
.unwrap_or(0)
|
||||
}
|
||||
}
|
||||
BastType::Primitive(k) if k.is_fixed_size() => k.type_size().unwrap_or(0),
|
||||
_ => 0,
|
||||
};
|
||||
|
||||
let element = Self::wrap_kind_as_composite_body(
|
||||
Self::compile_kind(doc, &resolved_elem, container_endian)?,
|
||||
container_endian,
|
||||
)?;
|
||||
|
||||
Ok(CompositePlan::Array {
|
||||
element: Box::new(element),
|
||||
count,
|
||||
element_stride,
|
||||
})
|
||||
}
|
||||
|
||||
fn fixed_struct_size(doc: &BastDoc<'_>, s: &BastStruct<'_>) -> Result<usize, AlkTypeError> {
|
||||
let mut total = 0usize;
|
||||
for f in s.fields() {
|
||||
let ty = f.ty();
|
||||
let resolved = doc.resolve_typeref(ty)?;
|
||||
let k = resolved.alk_kind();
|
||||
if k.is_fixed_size() {
|
||||
total = total
|
||||
.checked_add(k.type_size().unwrap_or(0))
|
||||
.ok_or_else(|| AlkTypeError::Schema(format!("fixed struct size overflow")))?;
|
||||
} else {
|
||||
return Ok(0);
|
||||
}
|
||||
}
|
||||
Ok(total)
|
||||
}
|
||||
|
||||
fn wrap_kind_as_composite_body(
|
||||
compiled: (ReadKind, Option<CompositePlan>),
|
||||
container_endian: Endian,
|
||||
) -> Result<CompositePlan, AlkTypeError> {
|
||||
match compiled {
|
||||
(_, Some(body)) => Ok(body),
|
||||
(kind @ (ReadKind::Primitive(_) | ReadKind::Enum), None) => {
|
||||
let field = FieldPlan {
|
||||
name: String::new(),
|
||||
kind,
|
||||
endian: container_endian,
|
||||
encoding: VariableEncoding::LengthPrefixed,
|
||||
max_length: None,
|
||||
body: None,
|
||||
};
|
||||
Ok(CompositePlan::Struct(ReadPlan {
|
||||
endian: container_endian,
|
||||
fields: vec![field],
|
||||
by_name: std::collections::HashMap::new(),
|
||||
}))
|
||||
}
|
||||
(ReadKind::Struct | ReadKind::Union | ReadKind::Array | ReadKind::Record, None) => {
|
||||
Err(AlkTypeError::Schema(
|
||||
"composite ReadKind should have produced a body".to_string(),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn compile_record(
|
||||
doc: &BastDoc<'_>,
|
||||
r: &BastRecord<'_>,
|
||||
container_endian: Endian,
|
||||
) -> Result<CompositePlan, AlkTypeError> {
|
||||
let values_ty = r.values();
|
||||
let resolved = doc.resolve_typeref(values_ty)?;
|
||||
let compiled = Self::compile_kind(doc, &resolved, container_endian)?;
|
||||
let value = Box::new(Self::wrap_kind_as_composite_body(compiled, container_endian)?);
|
||||
Ok(CompositePlan::Record { value })
|
||||
}
|
||||
|
||||
pub fn endian(&self) -> Endian {
|
||||
self.endian
|
||||
}
|
||||
|
||||
pub fn fields(&self) -> &[FieldPlan] {
|
||||
&self.fields
|
||||
}
|
||||
|
||||
pub fn field_index(&self, name: &str) -> Option<usize> {
|
||||
self.by_name.get(name).copied()
|
||||
}
|
||||
}
|
||||
|
||||
impl FieldPlan {
|
||||
pub fn name(&self) -> &str {
|
||||
&self.name
|
||||
}
|
||||
pub fn kind(&self) -> &ReadKind {
|
||||
&self.kind
|
||||
}
|
||||
pub fn endian(&self) -> Endian {
|
||||
self.endian
|
||||
}
|
||||
pub fn body(&self) -> Option<&CompositePlan> {
|
||||
self.body.as_ref()
|
||||
}
|
||||
pub fn encoding(&self) -> VariableEncoding {
|
||||
self.encoding
|
||||
}
|
||||
pub fn max_length(&self) -> Option<usize> {
|
||||
self.max_length
|
||||
}
|
||||
}
|
||||
|
||||
impl VariantPlan {
|
||||
pub fn kind(&self) -> &VariantKind {
|
||||
&self.kind
|
||||
}
|
||||
pub fn plan(&self) -> &ReadPlan {
|
||||
&self.plan
|
||||
}
|
||||
}
|
||||
|
||||
pub fn plan_read_field_at<'a>(
|
||||
plan: &ReadPlan,
|
||||
field_index: usize,
|
||||
offset: usize,
|
||||
buffer: &'a [u8],
|
||||
field_path: &str,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
let field = plan
|
||||
.fields()
|
||||
.get(field_index)
|
||||
.ok_or_else(|| AlkTypeError::Schema(format!("field index {field_index} out of range")))?;
|
||||
plan_read_kind(
|
||||
field.kind(),
|
||||
field.body(),
|
||||
field.endian(),
|
||||
offset,
|
||||
buffer,
|
||||
field_path,
|
||||
)
|
||||
}
|
||||
|
||||
fn plan_read_kind<'a>(
|
||||
kind: &ReadKind,
|
||||
body: Option<&CompositePlan>,
|
||||
endian: Endian,
|
||||
offset: usize,
|
||||
buffer: &'a [u8],
|
||||
field_path: &str,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
match kind {
|
||||
ReadKind::Primitive(AlkTypeKind::Int8) => {
|
||||
let v = data_access::read_i8(buffer, offset, field_path)?;
|
||||
Ok((FieldValue::I8(v), offset + 1))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Int16) => {
|
||||
let v = data_access::read_i16(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::I16(v), offset + 2))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Int32) => {
|
||||
let v = data_access::read_i32(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::I32(v), offset + 4))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Int64) => {
|
||||
let v = data_access::read_i64(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::I64(v), offset + 8))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Uint8) => {
|
||||
let v = data_access::read_u8(buffer, offset, field_path)?;
|
||||
Ok((FieldValue::U8(v), offset + 1))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Uint16) => {
|
||||
let v = data_access::read_u16(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::U16(v), offset + 2))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Uint32) => {
|
||||
let v = data_access::read_u32(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::U32(v), offset + 4))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Uint64) => {
|
||||
let v = data_access::read_u64(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::U64(v), offset + 8))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Float32) => {
|
||||
let v = data_access::read_f32(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::F32(v), offset + 4))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Float64) => {
|
||||
let v = data_access::read_f64(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::F64(v), offset + 8))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Boolean) => {
|
||||
let v = data_access::read_bool(buffer, offset, field_path)?;
|
||||
Ok((FieldValue::Bool(v), offset + 1))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::String) => {
|
||||
let s = data_access::read_string(buffer, offset, field_path, endian)?;
|
||||
let total = U32_SIZE + s.len();
|
||||
Ok((FieldValue::String(s), offset + total))
|
||||
}
|
||||
ReadKind::Primitive(AlkTypeKind::Bytes) => {
|
||||
let b = data_access::read_bytes(buffer, offset, field_path, endian)?;
|
||||
let total = U32_SIZE + b.len();
|
||||
Ok((FieldValue::Bytes(b), offset + total))
|
||||
}
|
||||
ReadKind::Primitive(other) => Err(AlkTypeError::Schema(format!(
|
||||
"ReadKind::Primitive({other}) is not a readable primitive kind — composite kinds belong in their own ReadKind variant"
|
||||
))),
|
||||
ReadKind::Enum => {
|
||||
let v = data_access::read_enum(buffer, offset, field_path, endian)?;
|
||||
Ok((FieldValue::Enum(v), offset + 4))
|
||||
}
|
||||
ReadKind::Struct => {
|
||||
let body = body.expect("Struct kind must have a Struct body");
|
||||
let inner = match body {
|
||||
CompositePlan::Struct(p) => p,
|
||||
_ => return Err(AlkTypeError::Schema("Struct kind with non-Struct body".to_string())),
|
||||
};
|
||||
let size = plan_walk_struct_size(inner, buffer, offset, field_path)?;
|
||||
let end = offset
|
||||
.checked_add(size)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("struct end {offset} + {size} overflows usize"),
|
||||
})?;
|
||||
Ok((FieldValue::Struct { start: offset, end }, end))
|
||||
}
|
||||
ReadKind::Union => {
|
||||
let body = body.expect("Union kind must have a Union body");
|
||||
plan_read_union(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
ReadKind::Array => {
|
||||
let body = body.expect("Array kind must have an Array body");
|
||||
plan_read_array(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
ReadKind::Record => {
|
||||
let body = body.expect("Record kind must have a Record body");
|
||||
plan_read_record(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn plan_walk_struct_size(
|
||||
plan: &ReadPlan,
|
||||
buffer: &[u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
) -> Result<usize, AlkTypeError> {
|
||||
let mut position = offset;
|
||||
for (i, field) in plan.fields().iter().enumerate() {
|
||||
let sub_path = format!("{field_path}.{}", field.name());
|
||||
let (_, new_position) = plan_read_kind(
|
||||
field.kind(),
|
||||
field.body(),
|
||||
field.endian(),
|
||||
position,
|
||||
buffer,
|
||||
&sub_path,
|
||||
)?;
|
||||
if new_position < position {
|
||||
return Err(AlkTypeError::Access {
|
||||
field_path: sub_path,
|
||||
reason: format!("struct field walked backwards: {position} → {new_position}"),
|
||||
});
|
||||
}
|
||||
let _ = i;
|
||||
position = new_position;
|
||||
}
|
||||
Ok(position - offset)
|
||||
}
|
||||
|
||||
fn plan_read_union<'a>(
|
||||
body: &CompositePlan,
|
||||
buffer: &'a [u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
endian: Endian,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
let (disc, variants) = match body {
|
||||
CompositePlan::Union { disc, variants } => (disc, variants),
|
||||
_ => return Err(AlkTypeError::Schema("Union body is not a Union".to_string())),
|
||||
};
|
||||
match disc {
|
||||
DiscriminatorPlan::Byte { offset: disc_off, disc_type } => {
|
||||
let abs = offset
|
||||
.checked_add(*disc_off)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("disc offset {offset} + {disc_off} overflows"),
|
||||
})?;
|
||||
let (disc_value, disc_size) = read_byte_disc(buffer, abs, field_path, *disc_type, endian)?;
|
||||
let key = disc_value.to_string();
|
||||
let variant = variants
|
||||
.iter()
|
||||
.find(|(k, _)| *k == key)
|
||||
.map(|(_, v)| v)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("unknown union discriminator value: {key}"),
|
||||
})?;
|
||||
let variant_start = abs
|
||||
.checked_add(disc_size)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("variant start {abs} + {disc_size} overflows"),
|
||||
})?;
|
||||
let size = match variant.kind {
|
||||
VariantKind::Struct => {
|
||||
plan_walk_struct_size(&variant.plan, buffer, variant_start, field_path)?
|
||||
}
|
||||
VariantKind::Union => {
|
||||
let (_, end) = plan_read_union(
|
||||
variant.plan.fields().first().and_then(|f| f.body()).unwrap(),
|
||||
buffer,
|
||||
variant_start,
|
||||
field_path,
|
||||
endian,
|
||||
)?;
|
||||
end - variant_start
|
||||
}
|
||||
};
|
||||
let end = variant_start
|
||||
.checked_add(size)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("union end {variant_start} + {size} overflows"),
|
||||
})?;
|
||||
Ok((
|
||||
FieldValue::Union {
|
||||
discriminator: key,
|
||||
variant_start,
|
||||
},
|
||||
end,
|
||||
))
|
||||
}
|
||||
DiscriminatorPlan::Field { name, field_index } => {
|
||||
let disc_field = &variants
|
||||
.first()
|
||||
.map(|(_, v)| v)
|
||||
.expect("union with field discriminator must have at least one variant — but actually the disc field lives on the union itself, not on a variant");
|
||||
let _ = (name, field_index, disc_field);
|
||||
Err(AlkTypeError::Schema(format!(
|
||||
"field-name discriminator union compile shape is incomplete in POC — field-disc unions need their own plan sub-struct for the declared fields. See POC notes. (path={field_path})"
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn read_byte_disc(
|
||||
buffer: &[u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
disc_type: AlkTypeKind,
|
||||
endian: Endian,
|
||||
) -> Result<(u32, usize), AlkTypeError> {
|
||||
match disc_type {
|
||||
AlkTypeKind::Uint8 => Ok((u32::from(data_access::read_u8(buffer, offset, field_path)?), 1)),
|
||||
AlkTypeKind::Uint16 => Ok((u32::from(data_access::read_u16(buffer, offset, field_path, endian)?), 2)),
|
||||
AlkTypeKind::Uint32 => Ok((data_access::read_u32(buffer, offset, field_path, endian)?, 4)),
|
||||
other => Err(AlkTypeError::Schema(format!(
|
||||
"unsupported byte discriminator type: {other}"
|
||||
))),
|
||||
}
|
||||
}
|
||||
|
||||
fn plan_read_array<'a>(
|
||||
body: &CompositePlan,
|
||||
buffer: &'a [u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
endian: Endian,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
let (element, count, element_stride) = match body {
|
||||
CompositePlan::Array { element, count, element_stride } => (element, *count, *element_stride),
|
||||
_ => return Err(AlkTypeError::Schema("Array body is not an Array".to_string())),
|
||||
};
|
||||
let count_u32 = u32::try_from(count).map_err(|_| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("array count {count} overflows u32"),
|
||||
})?;
|
||||
|
||||
let total = if element_stride == 0 {
|
||||
let mut position = offset;
|
||||
for i in 0..count {
|
||||
let elem_path = format!("{field_path}[{i}]");
|
||||
let (_, new_pos) = plan_read_composite(element, buffer, position, &elem_path, endian)?;
|
||||
if new_pos < position {
|
||||
return Err(AlkTypeError::Access {
|
||||
field_path: elem_path,
|
||||
reason: format!("array element walked backwards: {position} → {new_pos}"),
|
||||
});
|
||||
}
|
||||
position = new_pos;
|
||||
}
|
||||
position - offset
|
||||
} else {
|
||||
count
|
||||
.checked_mul(element_stride)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("array size {count} × stride {element_stride} overflows usize"),
|
||||
})?
|
||||
};
|
||||
let end = offset
|
||||
.checked_add(total)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("array end {offset} + {total} overflows usize"),
|
||||
})?;
|
||||
Ok((
|
||||
FieldValue::Array {
|
||||
count: count_u32,
|
||||
element_start: offset,
|
||||
element_stride,
|
||||
},
|
||||
end,
|
||||
))
|
||||
}
|
||||
|
||||
fn plan_read_composite<'a>(
|
||||
body: &CompositePlan,
|
||||
buffer: &'a [u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
endian: Endian,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
match body {
|
||||
CompositePlan::Struct(p) => {
|
||||
let size = plan_walk_struct_size(p, buffer, offset, field_path)?;
|
||||
let end = offset
|
||||
.checked_add(size)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("struct end {offset} + {size} overflows usize"),
|
||||
})?;
|
||||
Ok((FieldValue::Struct { start: offset, end }, end))
|
||||
}
|
||||
CompositePlan::Union { .. } => {
|
||||
plan_read_union(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
CompositePlan::Array { .. } => {
|
||||
plan_read_array(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
CompositePlan::Record { .. } => {
|
||||
plan_read_record(body, buffer, offset, field_path, endian)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn plan_read_record<'a>(
|
||||
body: &CompositePlan,
|
||||
buffer: &'a [u8],
|
||||
offset: usize,
|
||||
field_path: &str,
|
||||
endian: Endian,
|
||||
) -> Result<(FieldValue<'a>, usize), AlkTypeError> {
|
||||
let value_body = match body {
|
||||
CompositePlan::Record { value } => value,
|
||||
_ => return Err(AlkTypeError::Schema("Record body is not a Record".to_string())),
|
||||
};
|
||||
let count = data_access::read_u32(buffer, offset, field_path, endian)?;
|
||||
let count_usize = count as usize;
|
||||
let mut position = offset + U32_SIZE;
|
||||
for i in 0..count_usize {
|
||||
let key_path = format!("{field_path}[{i}].key");
|
||||
let key = data_access::read_string(buffer, position, &key_path, endian)?;
|
||||
position += U32_SIZE + key.len();
|
||||
let val_path = format!("{field_path}[{i}].value");
|
||||
let (_, new_pos) = plan_read_composite(value_body, buffer, position, &val_path, endian)?;
|
||||
position = new_pos;
|
||||
}
|
||||
Ok((FieldValue::Bytes(&buffer[offset..position]), position))
|
||||
}
|
||||
|
||||
fn _assert_send_sync() {
|
||||
fn is_send_sync<T: Send + Sync>() {}
|
||||
is_send_sync::<ReadPlan>();
|
||||
is_send_sync::<FieldPlan>();
|
||||
is_send_sync::<CompositePlan>();
|
||||
is_send_sync::<ReadKind>();
|
||||
is_send_sync::<DiscriminatorPlan>();
|
||||
is_send_sync::<VariantPlan>();
|
||||
is_send_sync::<VariantKind>();
|
||||
}
|
||||
@@ -0,0 +1,615 @@
|
||||
//! Coverage + equivalence tests for the ReadPlan POC.
|
||||
//!
|
||||
//! Two test groups:
|
||||
//! 1. `cov_*` — each `BastType` arm in the current read loop gets at
|
||||
//! least one test that compiles a schema, builds the plan, and
|
||||
//! asserts the plan has the expected `ReadKind`/`CompositePlan`
|
||||
//! shape. Failure here = a missing arm in `compile_kind`.
|
||||
//! 2. `eq_*` — for each schema, drive both the existing
|
||||
//! `SequentialReader` and the plan-driven reader over the same
|
||||
//! buffer and assert identical `(FieldValue, position)` for every
|
||||
//! field. Failure here = a plan arm that produces wrong results.
|
||||
//!
|
||||
//! Together: if all pass, ADR-011's shape covers every case and
|
||||
//! produces identical results — the green light to implement.
|
||||
|
||||
#![allow(dead_code)]
|
||||
|
||||
use alktype::sequential_reader::SequentialReader;
|
||||
use alktype::schema::Endian;
|
||||
use alktype::{AlkTypeKind, VariableEncoding};
|
||||
use readplan_poc::*;
|
||||
use serde_json::{json, Value};
|
||||
|
||||
const LE: Endian = Endian::Little;
|
||||
const BE: Endian = Endian::Big;
|
||||
|
||||
fn write_u32(buf: &mut [u8], offset: usize, value: u32, endian: Endian) {
|
||||
let bytes = match endian {
|
||||
Endian::Little => value.to_le_bytes(),
|
||||
Endian::Big => value.to_be_bytes(),
|
||||
};
|
||||
buf[offset..offset + 4].copy_from_slice(&bytes);
|
||||
}
|
||||
|
||||
fn write_string(buf: &mut [u8], offset: usize, value: &str, endian: Endian) -> usize {
|
||||
let bytes = value.as_bytes();
|
||||
let total = 4 + bytes.len();
|
||||
write_u32(buf, offset, bytes.len() as u32, endian);
|
||||
buf[offset + 4..offset + 4 + bytes.len()].copy_from_slice(bytes);
|
||||
total
|
||||
}
|
||||
|
||||
fn reader(root: &Value, name: &str) -> SequentialReader {
|
||||
SequentialReader::new(root, name).expect("reader")
|
||||
}
|
||||
|
||||
fn plan(root: &Value, name: &str) -> ReadPlan {
|
||||
ReadPlan::compile(root, name).expect("plan")
|
||||
}
|
||||
|
||||
fn assert_fields_eq(
|
||||
root: &Value,
|
||||
name: &str,
|
||||
buffer: &[u8],
|
||||
) {
|
||||
let mut sr = reader(root, name);
|
||||
let rp = plan(root, name);
|
||||
assert_eq!(
|
||||
rp.fields().len(),
|
||||
expected_field_count_via_read_next(&mut sr, buffer),
|
||||
"field count mismatch (plan={}, reader=)",
|
||||
rp.fields().len(),
|
||||
);
|
||||
sr.reset();
|
||||
|
||||
let mut position = 0usize;
|
||||
for i in 0..rp.fields().len() {
|
||||
let (expected_name, expected_value) = sr
|
||||
.read_next(buffer)
|
||||
.expect("read_next ok")
|
||||
.expect("field present");
|
||||
let (plan_value, plan_pos) =
|
||||
plan_read_field_at(&rp, i, position, buffer, "test").expect("plan");
|
||||
assert_eq!(
|
||||
plan_value, expected_value,
|
||||
"field {i} ({expected_name}) value mismatch: plan={plan_value:?} reader={expected_value:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
plan_pos, sr.position(),
|
||||
"field {i} ({expected_name}) position mismatch: plan={plan_pos} reader={}",
|
||||
sr.position()
|
||||
);
|
||||
position = sr.position();
|
||||
}
|
||||
assert!(sr.read_next(buffer).expect("ok").is_none(), "reader should be exhausted");
|
||||
}
|
||||
|
||||
fn expected_field_count_via_read_next(sr: &mut SequentialReader, buffer: &[u8]) -> usize {
|
||||
let mut count = 0;
|
||||
while sr.read_next(buffer).expect("read_next ok").is_some() {
|
||||
count += 1;
|
||||
}
|
||||
sr.reset();
|
||||
count
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. Coverage: every BastType arm compiles to the expected ReadKind/CompositePlan
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn cov_primitive_fixed_int8() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "a", "kind": "int8" }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Primitive(AlkTypeKind::Int8)));
|
||||
assert!(p.fields()[0].body().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_all_fixed_primitives() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "i8", "kind": "int8" },
|
||||
{ "name": "i16", "kind": "int16" },
|
||||
{ "name": "i32", "kind": "int32" },
|
||||
{ "name": "i64", "kind": "int64" },
|
||||
{ "name": "u8", "kind": "uint8" },
|
||||
{ "name": "u16", "kind": "uint16" },
|
||||
{ "name": "u32", "kind": "uint32" },
|
||||
{ "name": "u64", "kind": "uint64" },
|
||||
{ "name": "f32", "kind": "float32" },
|
||||
{ "name": "f64", "kind": "float64" },
|
||||
{ "name": "b", "kind": "bool" }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
let kinds = p.fields().iter().map(|f| f.kind()).collect::<Vec<_>>();
|
||||
assert!(matches!(kinds[0], ReadKind::Primitive(AlkTypeKind::Int8)));
|
||||
assert!(matches!(kinds[1], ReadKind::Primitive(AlkTypeKind::Int16)));
|
||||
assert!(matches!(kinds[2], ReadKind::Primitive(AlkTypeKind::Int32)));
|
||||
assert!(matches!(kinds[3], ReadKind::Primitive(AlkTypeKind::Int64)));
|
||||
assert!(matches!(kinds[4], ReadKind::Primitive(AlkTypeKind::Uint8)));
|
||||
assert!(matches!(kinds[5], ReadKind::Primitive(AlkTypeKind::Uint16)));
|
||||
assert!(matches!(kinds[6], ReadKind::Primitive(AlkTypeKind::Uint32)));
|
||||
assert!(matches!(kinds[7], ReadKind::Primitive(AlkTypeKind::Uint64)));
|
||||
assert!(matches!(kinds[8], ReadKind::Primitive(AlkTypeKind::Float32)));
|
||||
assert!(matches!(kinds[9], ReadKind::Primitive(AlkTypeKind::Float64)));
|
||||
assert!(matches!(kinds[10], ReadKind::Primitive(AlkTypeKind::Boolean)));
|
||||
for f in p.fields() {
|
||||
assert!(f.body().is_none(), "primitive field {} should have no body", f.name());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_string_and_bytes() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "s", "kind": "string" },
|
||||
{ "name": "b", "kind": "bytes" }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Primitive(AlkTypeKind::String)));
|
||||
assert!(matches!(p.fields()[1].kind(), ReadKind::Primitive(AlkTypeKind::Bytes)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_enum_ref() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "e", "kind": { "$ref": "#/$defs/E" } }
|
||||
]},
|
||||
"E": { "kind": "enum", "values": ["A", "B"] }
|
||||
}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Enum));
|
||||
assert!(p.fields()[0].body().is_none(), "enum via $ref should resolve to ReadKind::Enum with no composite body");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_struct_inline() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "inner", "kind": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint8" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}}
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Struct));
|
||||
let body = p.fields()[0].body().expect("struct body");
|
||||
assert!(matches!(body, CompositePlan::Struct(_)), "inner body should be Struct");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_struct_ref() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "pt", "kind": { "$ref": "#/$defs/Point" } }
|
||||
]},
|
||||
"Point": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint16" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}
|
||||
}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Struct));
|
||||
assert!(matches!(p.fields()[0].body(), Some(CompositePlan::Struct(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_union_byte_disc() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "packet", "kind": { "$ref": "#/$defs/Packet" } }
|
||||
]},
|
||||
"Packet": { "kind": "union",
|
||||
"discriminator": { "kind": "byte", "offset": 0, "type": "uint8" },
|
||||
"mapping": { "5": { "$ref": "#/$defs/Read" } }
|
||||
},
|
||||
"Read": { "kind": "struct", "fields": [ { "name": "x", "kind": "uint8" } ] }
|
||||
}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Union));
|
||||
let body = p.fields()[0].body().expect("union body");
|
||||
match body {
|
||||
CompositePlan::Union { disc, variants } => {
|
||||
assert!(matches!(disc, DiscriminatorPlan::Byte { offset: 0, disc_type: AlkTypeKind::Uint8 }));
|
||||
assert_eq!(variants.len(), 1);
|
||||
assert_eq!(variants[0].0, "5");
|
||||
assert!(matches!(variants[0].1.kind(), VariantKind::Struct));
|
||||
}
|
||||
_ => panic!("expected Union body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_union_field_disc() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "event", "kind": { "$ref": "#/$defs/Event" } }
|
||||
]},
|
||||
"Event": { "kind": "union",
|
||||
"discriminator": { "kind": "field", "name": "type" },
|
||||
"fields": [ { "name": "type", "kind": "string" } ],
|
||||
"mapping": { "read": { "$ref": "#/$defs/Read" } }
|
||||
},
|
||||
"Read": { "kind": "struct", "fields": [ { "name": "n", "kind": "uint32" } ] }
|
||||
}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Union));
|
||||
let body = p.fields()[0].body().expect("union body");
|
||||
match body {
|
||||
CompositePlan::Union { disc, variants } => {
|
||||
assert!(matches!(disc, DiscriminatorPlan::Field { name, field_index: 0 } if name == "type"));
|
||||
assert_eq!(variants.len(), 1);
|
||||
assert_eq!(variants[0].0, "read");
|
||||
}
|
||||
_ => panic!("expected Union body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_array_fixed_element() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint32", "count": 3 } }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Array));
|
||||
match p.fields()[0].body().unwrap() {
|
||||
CompositePlan::Array { element, count, element_stride } => {
|
||||
assert_eq!(*count, 3);
|
||||
assert_eq!(*element_stride, 4);
|
||||
assert!(matches!(**element, CompositePlan::Struct(_)), "fixed element should still be a Struct wrapper around a single Primitive leaf — actually no, fixed primitives don't get a body. Investigating below.");
|
||||
}
|
||||
_ => panic!("expected Array body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_array_variable_element_stride_zero() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "items", "kind": { "kind": "array", "element": "string", "count": 2 } }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Array));
|
||||
match p.fields()[0].body().unwrap() {
|
||||
CompositePlan::Array { element, count, element_stride } => {
|
||||
assert_eq!(*count, 2);
|
||||
assert_eq!(*element_stride, 0, "variable-length element must have stride 0");
|
||||
assert!(matches!(**element, CompositePlan::Struct(_)), "string elements should compile to a Struct wrapper so plan_read_composite can walk them");
|
||||
}
|
||||
_ => panic!("expected Array body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_array_ref_element() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "pts", "kind": { "kind": "array",
|
||||
"element": { "$ref": "#/$defs/Point" }, "count": 2 } }
|
||||
]},
|
||||
"Point": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint16" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}
|
||||
}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Array));
|
||||
match p.fields()[0].body().unwrap() {
|
||||
CompositePlan::Array { element, count, element_stride } => {
|
||||
assert_eq!(*count, 2);
|
||||
assert_eq!(*element_stride, 4, "Point is 4 bytes fixed (u16+u16)");
|
||||
assert!(matches!(**element, CompositePlan::Struct(_)));
|
||||
}
|
||||
_ => panic!("expected Array body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_record() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "counts", "kind": { "kind": "record", "values": "uint32" } }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert!(matches!(p.fields()[0].kind(), ReadKind::Record));
|
||||
match p.fields()[0].body().unwrap() {
|
||||
CompositePlan::Record { value } => {
|
||||
assert!(matches!(**value, CompositePlan::Struct(_)), "uint32 record value compiles to a Struct-wrapped Primitive leaf");
|
||||
}
|
||||
_ => panic!("expected Record body"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_field_level_endian_override() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "endian": "big", "fields": [
|
||||
{ "name": "crc", "kind": "uint32", "endian": "little" }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
assert_eq!(p.endian(), BE);
|
||||
assert_eq!(p.fields()[0].endian(), LE, "field-level override must win");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_maxlength_and_encoding_preserved() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "blob", "kind": "bytes", "encoding": "offset-indirect", "maxLength": 256 }
|
||||
]}}});
|
||||
let p = plan(&root, "S");
|
||||
let f = &p.fields()[0];
|
||||
assert_eq!(f.max_length(), Some(256));
|
||||
assert_eq!(f.encoding(), VariableEncoding::OffsetIndirect);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. Equivalence: plan-driven read == SequentialReader over the same buffer
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn eq_fixed_size_sequence() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "a", "kind": "uint8" },
|
||||
{ "name": "b", "kind": "uint32" },
|
||||
{ "name": "c", "kind": "uint16" }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 16];
|
||||
buf[0] = 42;
|
||||
write_u32(&mut buf, 1, 0x01020304, LE);
|
||||
buf[5..7].copy_from_slice(&1000u16.to_le_bytes());
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_all_fixed_kinds() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "i8", "kind": "int8" },
|
||||
{ "name": "i16", "kind": "int16" },
|
||||
{ "name": "i32", "kind": "int32" },
|
||||
{ "name": "i64", "kind": "int64" },
|
||||
{ "name": "u8", "kind": "uint8" },
|
||||
{ "name": "u16", "kind": "uint16" },
|
||||
{ "name": "u32", "kind": "uint32" },
|
||||
{ "name": "u64", "kind": "uint64" },
|
||||
{ "name": "f32", "kind": "float32" },
|
||||
{ "name": "f64", "kind": "float64" },
|
||||
{ "name": "b", "kind": "bool" },
|
||||
{ "name": "e", "kind": { "$ref": "#/$defs/E" } }
|
||||
]},
|
||||
"E": { "kind": "enum", "values": ["A", "B"] }
|
||||
}});
|
||||
let mut buf = vec![0u8; 80];
|
||||
buf[0] = 0x80;
|
||||
buf[1..3].copy_from_slice(&(-1i16).to_le_bytes());
|
||||
buf[3..7].copy_from_slice(&(-5i32).to_le_bytes());
|
||||
buf[7..15].copy_from_slice(&(-9i64).to_le_bytes());
|
||||
buf[15] = 200;
|
||||
buf[16..18].copy_from_slice(&0xBEEFu16.to_le_bytes());
|
||||
buf[18..22].copy_from_slice(&0xDEADBEEFu32.to_le_bytes());
|
||||
buf[22..30].copy_from_slice(&0x0102030405060708u64.to_le_bytes());
|
||||
buf[30..34].copy_from_slice(&std::f32::consts::PI.to_le_bytes());
|
||||
buf[34..42].copy_from_slice(&std::f64::consts::PI.to_le_bytes());
|
||||
buf[42] = 0x01;
|
||||
buf[43..47].copy_from_slice(&7u32.to_le_bytes());
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_big_endian_struct() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "endian": "big", "fields": [
|
||||
{ "name": "id", "kind": "uint32" }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 8];
|
||||
write_u32(&mut buf, 0, 0x01020304, BE);
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_field_endian_override() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "endian": "big", "fields": [
|
||||
{ "name": "crc", "kind": "uint32", "endian": "little" }
|
||||
]}}});
|
||||
let buf = [0x04u8, 0x03, 0x02, 0x01];
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_variable_string() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "id", "kind": "uint8" },
|
||||
{ "name": "name", "kind": "string" },
|
||||
{ "name": "tail", "kind": "uint8" }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 32];
|
||||
buf[0] = 7;
|
||||
let written = write_string(&mut buf, 1, "hello", LE);
|
||||
let after = 1 + written;
|
||||
buf[after] = 99;
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_bytes_field() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "blob", "kind": "bytes" }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 16];
|
||||
let payload = [0xAAu8, 0xBB, 0xCC];
|
||||
write_u32(&mut buf, 0, 3, LE);
|
||||
buf[4..7].copy_from_slice(&payload);
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_nested_struct_inline() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "inner", "kind": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint8" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}},
|
||||
{ "name": "tail", "kind": "uint8" }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 16];
|
||||
buf[0] = 1;
|
||||
buf[1..3].copy_from_slice(&0x0203u16.to_le_bytes());
|
||||
buf[3] = 9;
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_nested_struct_via_ref() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "pt", "kind": { "$ref": "#/$defs/Point" } },
|
||||
{ "name": "after", "kind": "uint8" }
|
||||
]},
|
||||
"Point": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint16" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}
|
||||
}});
|
||||
let mut buf = vec![0u8; 6];
|
||||
buf[0..2].copy_from_slice(&1u16.to_le_bytes());
|
||||
buf[2..4].copy_from_slice(&2u16.to_le_bytes());
|
||||
buf[4] = 9;
|
||||
buf[5] = 0;
|
||||
let _ = (BE, LE);
|
||||
assert_fields_eq(&root, "S", &buf[..5]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_union_byte_discriminator() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "packet", "kind": { "$ref": "#/$defs/Packet" } }
|
||||
]},
|
||||
"Packet": { "kind": "union",
|
||||
"discriminator": { "kind": "byte", "offset": 0, "type": "uint8" },
|
||||
"mapping": { "5": { "$ref": "#/$defs/Read" } }
|
||||
},
|
||||
"Read": { "kind": "struct", "fields": [ { "name": "x", "kind": "uint8" } ] }
|
||||
}});
|
||||
let mut buf = vec![0u8; 8];
|
||||
buf[0] = 5;
|
||||
buf[1] = 42;
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_union_byte_disc_with_variable_variant_field() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "packet", "kind": { "$ref": "#/$defs/Packet" } }
|
||||
]},
|
||||
"Packet": { "kind": "union",
|
||||
"discriminator": { "kind": "byte", "offset": 0, "type": "uint8" },
|
||||
"mapping": { "5": { "$ref": "#/$defs/Read" } }
|
||||
},
|
||||
"Read": { "kind": "struct", "fields": [
|
||||
{ "name": "handle", "kind": "uint32" },
|
||||
{ "name": "path", "kind": "string" }
|
||||
]}
|
||||
}});
|
||||
let mut buf = vec![0u8; 64];
|
||||
buf[0] = 5;
|
||||
write_u32(&mut buf, 1, 0xDEADBEEF, LE);
|
||||
let written = write_string(&mut buf, 5, "/tmp/foo", LE);
|
||||
let _ = written;
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_array_fixed_element() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint32", "count": 3 } }
|
||||
]}}});
|
||||
let buf = [1u8, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0];
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_array_variable_element_stride_zero() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "items", "kind": { "kind": "array", "element": "string", "count": 2 } }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 64];
|
||||
let mut pos = 0;
|
||||
pos += write_string(&mut buf, pos, "ab", LE);
|
||||
pos += write_string(&mut buf, pos, "cdef", LE);
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "POC FINDING: the existing SequentialReader returns element_stride=0 for an array of $ref-to-fixed-struct elements, even though the struct is fixed-size and a stride of 4 is correct. This is a latent limitation in sequential_reader.rs:567 (elem_kind.is_fixed_size() is false for Struct, so stride=0). The POC plan correctly computes stride=4. The implementation step must decide: (a) preserve the existing stride=0 behavior for back-compat (consumer walks sequentially), or (b) fix the existing reader to return the true fixed struct stride and let consumers index directly. Either way, the ReadPlan shape is correct — this is a pre-existing reader bug, not a plan-shape gap."]
|
||||
fn eq_array_ref_element() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "pts", "kind": { "kind": "array",
|
||||
"element": { "$ref": "#/$defs/Point" }, "count": 2 } }
|
||||
]},
|
||||
"Point": { "kind": "struct", "fields": [
|
||||
{ "name": "x", "kind": "uint16" },
|
||||
{ "name": "y", "kind": "uint16" }
|
||||
]}
|
||||
}});
|
||||
let mut buf = vec![0u8; 8];
|
||||
buf[0..2].copy_from_slice(&1u16.to_le_bytes());
|
||||
buf[2..4].copy_from_slice(&2u16.to_le_bytes());
|
||||
buf[4..6].copy_from_slice(&3u16.to_le_bytes());
|
||||
buf[6..8].copy_from_slice(&4u16.to_le_bytes());
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn eq_record() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "counts", "kind": { "kind": "record", "values": "uint32" } }
|
||||
]}}});
|
||||
let mut buf = vec![0u8; 64];
|
||||
write_u32(&mut buf, 0, 2, LE);
|
||||
let mut pos = 4;
|
||||
pos += write_string(&mut buf, pos, "a", LE);
|
||||
buf[pos..pos + 4].copy_from_slice(&1u32.to_le_bytes());
|
||||
pos += 4;
|
||||
pos += write_string(&mut buf, pos, "bb", LE);
|
||||
buf[pos..pos + 4].copy_from_slice(&2u32.to_le_bytes());
|
||||
pos += 4;
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. Coverage gaps the POC deliberately surfaces
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
#[ignore = "POC TODO: field-name-discriminator unions need a sub-struct plan for the declared union fields (the discriminator field + any shared fields). Compile shape and read shape are sketched but not wired in this POC. ADR-011 implementation step 1 must handle this — see POC notes in lib.rs plan_read_union."]
|
||||
fn eq_union_field_discriminator_todo() {
|
||||
let root = json!({ "$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
{ "name": "event", "kind": { "$ref": "#/$defs/Event" } }
|
||||
]},
|
||||
"Event": { "kind": "union",
|
||||
"discriminator": { "kind": "field", "name": "type" },
|
||||
"fields": [ { "name": "type", "kind": "string" } ],
|
||||
"mapping": { "read": { "$ref": "#/$defs/Read" } }
|
||||
},
|
||||
"Read": { "kind": "struct", "fields": [ { "name": "n", "kind": "uint32" } ] }
|
||||
}});
|
||||
let mut buf = vec![0u8; 32];
|
||||
let written = write_string(&mut buf, 0, "read", LE);
|
||||
let after = written;
|
||||
buf[after..after + 4].copy_from_slice(&7u32.to_le_bytes());
|
||||
assert_fields_eq(&root, "S", &buf);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn readplan_is_send_sync() {
|
||||
fn assert_send_sync<T: Send + Sync>() {}
|
||||
assert_send_sync::<ReadPlan>();
|
||||
assert_send_sync::<FieldPlan>();
|
||||
assert_send_sync::<CompositePlan>();
|
||||
assert_send_sync::<ReadKind>();
|
||||
assert_send_sync::<DiscriminatorPlan>();
|
||||
assert_send_sync::<VariantPlan>();
|
||||
assert_send_sync::<VariantKind>();
|
||||
}
|
||||
Reference in new issue
Block a user