Resolve L4/N1, fix M5, close M4 item 4 (review #006)

- L4: BTreeMap path->index for OffsetMap::get and PackedLayout::get;
  the linear scans behind the "random access" doc claim are gone.
  First-occurrence-wins preserved (BastStruct::parse doesn't reject
  duplicate names); locking tests in both modules.
- N1: tunion::read_field_discriminator now accepts uint16/uint32 disc
  fields (matching the reader's plan_discriminator_string_value set);
  one answer to "which field kinds can discriminate a union".
- M5 (new finding, fixed): aligned Record fields accepted maxLength /
  offset-indirect annotations, but the materializer always walks the
  inline count-prefixed form from the entry start — probe-verified
  silent corruption (record data crossed the reservation into the next
  field's bytes; validate_bytes accepted the corrupt buffer).
  Both annotations now rejected at compute with clean Offset errors.
  Parity-preserved from 0.2.0.
- M4 item 4: field_endian_for_element's Struct/Union arms and
  element_alignment were dead — the OQ-001 gate rejects every
  non-fixed-size element kind before either runs, so the phase-5
  "element's own endian is consulted" divergence never existed on any
  reachable path. Dead arms deleted; OQ-001 rejection for struct
  elements and endian propagation for fixed elements locked with tests.

Verified: 546 tests green (446 crate + 17 + 34 + 15 + 12 + 2 ignored),
clippy -D warnings clean, wasm build green.
This commit is contained in:
glm-5.3-flash committed 2026-09-02 20:07:43 +00:00
1 parent 0bc5a541ac
commit 5f9793f9c0
3 files changed
+420 -32

No files matched your search

+73 -5
View File
@@ -48,7 +48,7 @@ use crate::schema::{
AlkTypeKind, Endian, DISCRIMINATOR_PATH, U32_SIZE, MAX_ARRAY_BYTES,
};
use serde_json::Value;
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
const VARIANT_KEY: &str = "__variant";
@@ -73,6 +73,7 @@ pub struct FieldPosition {
#[derive(Debug)]
pub struct PackedLayout {
fields: Vec<(String, FieldPosition)>,
index: BTreeMap<String, usize>,
total_size: usize,
}
@@ -83,10 +84,8 @@ impl PackedLayout {
/// TUnion byte-offset discriminators, the discriminator is recorded
/// under the synthetic path `"<union_path>.__discriminator"`.
pub fn get(&self, field_path: &str) -> Option<&FieldPosition> {
self.fields
.iter()
.find(|(path, _)| path == field_path)
.map(|(_, pos)| pos)
let idx = *self.index.get(field_path)?;
self.fields.get(idx).map(|(_, pos)| pos)
}
/// The total buffer size needed to hold all fields.
@@ -204,7 +203,9 @@ impl LayoutBuilder {
};
let mut offset: usize = 0;
ctx.walk_struct(struct_node, "", &mut offset)?;
let index = Self::build_index(&ctx.fields);
Ok(PackedLayout {
index,
fields: ctx.fields,
total_size: offset,
})
@@ -214,6 +215,18 @@ impl LayoutBuilder {
pub fn endian(&self) -> Endian {
self.endian
}
/// Build the path→index lookup table over the insertion-ordered
/// `fields` vec. First occurrence wins on duplicate paths (matching
/// the linear-scan `find` this index replaced — `BastStruct::parse`
/// does not reject duplicate field names, review #006 L4).
fn build_index(fields: &[(String, FieldPosition)]) -> BTreeMap<String, usize> {
let mut index = BTreeMap::new();
for (i, (path, _)) in fields.iter().enumerate() {
index.entry(path.clone()).or_insert(i);
}
index
}
}
/// Mutable context threaded through the recursive layout computation.
@@ -651,6 +664,61 @@ mod tests {
);
}
// ----- L4: path-indexed random access --------------------------------
#[test]
fn l4_get_is_indexed_random_access_over_large_nested_schema() {
let mut fields = Vec::new();
for i in 0..40 {
fields.push(json!({
"name": format!("blk{i}"),
"kind": {
"kind": "struct",
"fields": [
{ "name": "off", "kind": "uint16" },
{ "name": "data", "kind": "uint64" }
]
}
}));
}
let root = json!({
"$defs": { "S": { "kind": "struct", "fields": fields } }
});
let layout = build(&root, "S", &var_sizes(&[]));
assert_eq!(layout.iter().count(), 80);
let last = layout.get("blk39.data").expect("late dotted-path lookup");
assert_eq!(last.offset, 39 * 10 + 2);
assert_eq!(last.size, 8);
assert_eq!(last.kind, AlkTypeKind::Uint64);
assert_eq!(
layout.get("blk12.off").map(|p| p.offset),
Some(12 * 10)
);
assert_eq!(layout.get("nope"), None);
assert_eq!(layout.get("blk39"), None);
}
#[test]
fn l4_duplicate_field_paths_first_occurrence_wins() {
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"fields": [
{ "name": "a", "kind": "uint8" },
{ "name": "a", "kind": "uint16" }
]
}
}
});
let layout = build(&root, "S", &var_sizes(&[]));
let first = layout.get("a").expect("duplicate path resolves");
assert_eq!(first.offset, 0);
assert_eq!(first.size, 1);
assert_eq!(first.kind, AlkTypeKind::Uint8);
assert_eq!(layout.iter().count(), 2);
}
#[test]
fn fixed_fields_packed_no_alignment_padding() {
let root = json!({
+263 -26
View File
@@ -17,6 +17,7 @@ use crate::bast::{
};
use crate::error::AlkTypeError;
use crate::schema::{AlkTypeKind, Endian, VariableEncoding, MAX_ARRAY_BYTES};
use std::collections::BTreeMap;
/// A byte range within a buffer.
///
@@ -104,6 +105,7 @@ impl OffsetEntry {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct OffsetMap {
fields: Vec<(String, OffsetEntry)>,
index: BTreeMap<String, usize>,
total_size: usize,
}
@@ -144,8 +146,10 @@ impl OffsetMap {
let struct_default_align = struct_node.align().unwrap_or(1).max(1);
let endian = struct_node.endian();
let (total, _align) = ctx.compute_struct(struct_node, "", struct_default_align, endian)?;
let index = Self::build_index(&ctx.fields);
Ok(Self {
fields: ctx.fields,
index,
total_size: total,
})
}
@@ -157,10 +161,8 @@ impl OffsetMap {
/// under the synthetic path `"__discriminator"` (qualified by the
/// union field's path, e.g. `"payload.__discriminator"`).
pub fn get(&self, field_path: &str) -> Option<&OffsetEntry> {
self.fields
.iter()
.find(|(path, _)| path == field_path)
.map(|(_, entry)| entry)
let idx = *self.index.get(field_path)?;
self.fields.get(idx).map(|(_, entry)| entry)
}
/// The total size of the struct in bytes (including trailing alignment padding).
@@ -188,6 +190,19 @@ impl OffsetMap {
self.hash(&mut h);
h.finish()
}
/// Build the path→index lookup table over the insertion-ordered
/// `fields` vec. First occurrence wins on duplicate paths (matching
/// the linear-scan `find` this index replaced — `BastStruct::parse`
/// does not reject duplicate field names, so the semantic must be
/// preserved, review #006 L4).
fn build_index(fields: &[(String, OffsetEntry)]) -> BTreeMap<String, usize> {
let mut index = BTreeMap::new();
for (i, (path, _)) in fields.iter().enumerate() {
index.entry(path.clone()).or_insert(i);
}
index
}
}
/// Mutable context threaded through the recursive offset computation.
@@ -294,6 +309,29 @@ impl<'d> ComputeCtx<'d> {
}),
BastType::Array(a) => self.compute_array_field(a, field, field_path, struct_default_align, field_endian),
BastType::Record(_) => {
if field.encoding() == VariableEncoding::OffsetIndirect {
return Err(AlkTypeError::Offset {
field_path: field_path.to_string(),
reason: "record field with `encoding: offset-indirect` is not supported \
in aligned mode: the materializer walks the record's inline \
count-prefixed form from the entry start, so an offset-indirect \
record would read from the wrong wire shape. Use the default \
inline length-prefixing, or packed mode."
.to_string(),
});
}
if field.max_length().is_some() {
return Err(AlkTypeError::Offset {
field_path: field_path.to_string(),
reason: "record field with `maxLength` is not supported in aligned mode: \
the materializer walks the record's inline count-prefixed form \
from the entry start and does not bound it by the reservation, \
so the record data would silently cross the maxLength boundary \
into subsequent fields (review #006 M5). Use the default inline \
length-prefixing (as the last field), or packed mode."
.to_string(),
});
}
self.compute_variable_field(field, field_path, struct_default_align, resolved.alk_kind(), field_endian)
}
BastType::Primitive(k) if k.is_variable_length() => {
@@ -417,7 +455,12 @@ impl<'d> ComputeCtx<'d> {
reason: format!("element kind {elem_kind} has no fixed size"),
})?;
let elem_natural = elem_kind.natural_alignment();
let elem_align = element_alignment(&resolved_elem, struct_default_align, elem_natural);
// Composites (struct/union/array/record) never reach here — the
// OQ-001 rejection above refuses every non-fixed-size element
// kind — so the element alignment is the struct default vs the
// fixed kind's natural alignment (review #006 M4 item 4: the
// former composite arms of this lookup were dead).
let elem_align = struct_default_align.max(elem_natural).max(1);
let stride = round_up(elem_size, elem_align);
let count = array.count();
let array_bytes = count
@@ -549,13 +592,14 @@ fn field_variable_kind(field: &BastField) -> Option<AlkTypeKind> {
/// element TypeRef doesn't carry a field-level override (BAST field
/// annotations live on the field, not the element), so the referring
/// field's effective endian applies — the same propagation the aligned
/// materializer uses.
/// materializer uses. Only fixed-size element kinds reach here (the
/// OQ-001 rejection upstream refuses composites), so the former
/// `Struct`/`Union` composite arms were dead (review #006 M4 item 4).
fn field_endian_for_element(elem_ty: &BastType, field_endian: Endian) -> Endian {
match elem_ty {
BastType::Struct(s) => s.endian(),
BastType::Union(u) => u.endian(),
BastType::Enum(_) | BastType::Array(_) | BastType::Record(_) => field_endian,
BastType::Primitive(_) | BastType::Ref(_) => field_endian,
BastType::Struct(_) | BastType::Union(_) => field_endian,
}
}
@@ -568,24 +612,6 @@ fn field_alignment(field: &BastField, struct_default_align: usize, natural: usiz
struct_default_align.max(natural).max(1)
}
/// Resolve an element type's alignment. Inline struct/array elements use
/// natural alignment 1; primitives use their natural alignment; field-level
/// `align` from the enclosing field node doesn't apply to inline element
/// schemas (BAST field-level annotations live on the field, not the
/// element TypeRef), so we fall back to the struct default.
fn element_alignment(
elem_ty: &BastType,
struct_default_align: usize,
natural: usize,
) -> usize {
match elem_ty {
BastType::Struct(_) | BastType::Union(_) | BastType::Array(_) => {
struct_default_align.max(1)
}
_ => struct_default_align.max(natural).max(1),
}
}
/// Round `offset` up to the next multiple of `align`. No-op if `align <= 1`.
fn align_up(offset: &mut usize, align: usize) {
if align <= 1 {
@@ -997,6 +1023,73 @@ mod tests {
assert_eq!(m.get("b.x").map(|e| e.range), Some(ByteRange { start: 2, end: 4 }));
}
// ----- M4 item 4: array-element endian propagation + OQ-001 gate ----
#[test]
fn m4_array_of_struct_element_rejected_oq001() {
// The gate that makes field_endian_for_element's composite arms
// dead: a struct element kind is variable-length, so it hits the
// OQ-001 rejection before the endian lookup.
let root = json!({
"$defs": {
"S": { "kind": "struct", "fields": [
{ "name": "points", "kind": { "kind": "array", "element": { "$ref": "#/$defs/Point" }, "count": 2 } }
] },
"Point": { "kind": "struct", "fields": [ { "name": "x", "kind": "uint16" } ] }
}
});
let doc = BastDoc::new(&root, "S").expect("doc");
let err = OffsetMap::compute(&doc).unwrap_err();
match err {
AlkTypeError::Offset { field_path, reason } => {
assert_eq!(field_path, "points");
assert!(reason.contains("OQ-001"), "reason: {reason}");
}
other => panic!("expected Offset, got {other:?}"),
}
}
#[test]
fn m4_array_element_endian_inherits_referring_field_not_element_own() {
// Phase-5 parity rule on the aligned path: the element's effective
// endian is the referring field's, not the element struct's own
// annotation (the pre-M4-cleanup code consulted the element
// struct's `endian` in a dead arm — dead because composites are
// rejected by OQ-001 upstream; this test pins the reachable
// propagation for fixed elements).
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"endian": "big",
"fields": [
{
"name": "vals",
"kind": { "kind": "array", "element": "uint32", "count": 2 },
"endian": "little"
}
]
}
}
});
let m = map(&root, "S");
let e0 = m.get("vals[0]").expect("vals[0]");
assert_eq!(e0.meta.endian, Endian::Little, "field-level endian override propagates to elements");
let root_be = json!({
"$defs": {
"S": {
"kind": "struct",
"endian": "big",
"fields": [
{ "name": "vals", "kind": { "kind": "array", "element": "uint32", "count": 2 } }
]
}
}
});
let m_be = map(&root_be, "S");
assert_eq!(m_be.get("vals[0]").expect("vals[0]").meta.endian, Endian::Big, "struct default propagates to elements");
}
#[test]
fn non_final_record_rejected_in_aligned_mode() {
// M1: a Record field's inline length-prefixed form has the same
@@ -1045,6 +1138,87 @@ mod tests {
assert_eq!(m.total_size(), 8);
}
// ----- M5: record maxLength / offset-indirect rejected in aligned mode
#[test]
fn m5_record_max_length_rejected_in_aligned_mode() {
// Probe-verified silent corruption: the materializer walks the
// record's inline count-prefixed form from the entry start and
// never bounds it by the maxLength reservation, so the record
// data crosses the reservation into the next field's bytes and
// validate_bytes accepts the corrupt buffer. Reject at compute.
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"fields": [
{ "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 8 },
{ "name": "id", "kind": "uint32" }
]
}
}
});
let doc = BastDoc::new(&root, "S").expect("doc parses");
let err = OffsetMap::compute(&doc).unwrap_err();
match err {
AlkTypeError::Offset { field_path, reason } => {
assert_eq!(field_path, "counts");
assert!(reason.contains("maxLength"), "reason: {reason}");
assert!(reason.contains("record"), "reason: {reason}");
}
other => panic!("expected Offset, got {other:?}"),
}
}
#[test]
fn m5_record_offset_indirect_rejected_in_aligned_mode() {
// Same walk-shape mismatch: the materializer reads the inline
// count-prefixed form, not the {offset, length} pair the map
// would have recorded.
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"fields": [
{ "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "encoding": "offset-indirect" },
{ "name": "id", "kind": "uint32" }
]
}
}
});
let doc = BastDoc::new(&root, "S").expect("doc parses");
let err = OffsetMap::compute(&doc).unwrap_err();
match err {
AlkTypeError::Offset { field_path, reason } => {
assert_eq!(field_path, "counts");
assert!(reason.contains("offset-indirect"), "reason: {reason}");
assert!(reason.contains("record"), "reason: {reason}");
}
other => panic!("expected Offset, got {other:?}"),
}
}
#[test]
fn m5_record_max_length_rejected_even_as_last_field() {
// Position doesn't rescue it: the reservation would fix
// total_size, but the walk still reads the inline form — a
// different wire shape than any writer produces.
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"fields": [
{ "name": "id", "kind": "uint32" },
{ "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 64 }
]
}
}
});
let doc = BastDoc::new(&root, "S").expect("doc parses");
let err = OffsetMap::compute(&doc).unwrap_err();
assert!(matches!(err, AlkTypeError::Offset { .. }), "got {err:?}");
}
#[test]
fn non_final_inline_string_rejected_in_aligned_mode() {
let root = json!({
@@ -1201,6 +1375,69 @@ mod tests {
assert_eq!(paths, vec!["a", "b"]);
}
// ----- L4: path-indexed random access --------------------------------
#[test]
fn l4_get_is_indexed_random_access_over_large_nested_schema() {
// The map's doc contract promises random access by field path
// ("read field N without reading fields 0..N-1 first"); L4 made
// that true with a BTreeMap index instead of a linear scan. A
// wide nested schema exercises dotted-path lookups that arrive
// late in insertion order.
let mut fields = Vec::new();
for i in 0..40 {
fields.push(json!({
"name": format!("blk{i}"),
"kind": {
"kind": "struct",
"fields": [
{ "name": "id", "kind": "uint32" },
{ "name": "tag", "kind": "uint8" },
{ "name": "val", "kind": "uint64" }
]
}
}));
}
let root = json!({
"$defs": { "S": { "kind": "struct", "fields": fields } }
});
let m = map(&root, "S");
assert_eq!(m.iter().count(), 120);
let last = m.get("blk39.val").expect("late dotted-path lookup");
assert_eq!(last.range, ByteRange { start: 39 * 16 + 8, end: 39 * 16 + 16 });
assert_eq!(last.meta.kind, AlkTypeKind::Uint64);
let mid = m.get("blk20.tag").expect("mid lookup");
assert_eq!(mid.range, ByteRange { start: 20 * 16 + 4, end: 20 * 16 + 5 });
assert_eq!(m.get("blk7.id").map(|e| e.range), Some(ByteRange { start: 7 * 16, end: 7 * 16 + 4 }));
assert_eq!(m.get("nope"), None);
assert_eq!(m.get("blk"), None);
assert_eq!(m.get("blk20"), None);
}
#[test]
fn l4_duplicate_field_paths_first_occurrence_wins() {
// BastStruct::parse does not reject duplicate field names, so two
// same-named siblings produce two entries with the same path. The
// pre-L4 linear scan returned the first; the index preserves that
// semantic.
let root = json!({
"$defs": {
"S": {
"kind": "struct",
"fields": [
{ "name": "a", "kind": "uint8" },
{ "name": "a", "kind": "uint16" }
]
}
}
});
let m = map(&root, "S");
let first = m.get("a").expect("duplicate path resolves");
assert_eq!(first.range, ByteRange { start: 0, end: 1 });
assert_eq!(first.meta.kind, AlkTypeKind::Uint8);
assert_eq!(m.iter().count(), 2);
}
// ----- Fingerprint contract (ADR-012 §1/§4) ---------------------------
#[test]
+84 -1
View File
@@ -106,7 +106,10 @@ pub fn read_byte_discriminator(
/// - [`AlkTypeError::Schema`] if the union does not have a field-name
/// discriminator, the discriminator field is not declared in
/// `fields`, or the field's kind is not one of `string` / `uint8` /
/// `enum`.
/// `uint16` / `uint32` / `enum` (the same kind set the compiled
/// reader's `plan_discriminator_string_value` accepts — review #006
/// N1 closed the divergence so both public dispatch paths answer
/// identically).
/// - [`AlkTypeError::Access`] if the buffer is too short to contain the
/// discriminator field, or if the read value is not present in the
/// union's `mapping`.
@@ -152,6 +155,14 @@ pub fn read_field_discriminator(
let v = read_u8(buffer, disc_field_offset, name)?;
(v.to_string(), 1)
}
AlkTypeKind::Uint16 => {
let v = read_u16(buffer, disc_field_offset, name, endian)?;
(v.to_string(), 2)
}
AlkTypeKind::Uint32 => {
let v = read_u32(buffer, disc_field_offset, name, endian)?;
(v.to_string(), U32_SIZE)
}
AlkTypeKind::Enum => {
let v = read_enum(buffer, disc_field_offset, name, endian)?;
(v.to_string(), U32_SIZE)
@@ -421,6 +432,78 @@ mod tests {
assert_eq!(d.discriminator_size, 1);
}
#[test]
fn n1_read_field_discriminator_uint16_little_endian() {
// N1: tunion now accepts the same field-disc kinds as the plan
// reader (string/uint8/uint16/uint32/enum) — uint16/uint32 were
// previously rejected with a Schema error, diverging from
// `plan_discriminator_string_value`.
let root = field_union_root("type", "uint16");
let u = doc_union(&root, "U");
let mut buf = vec![0u8; 8];
buf[0..2].copy_from_slice(&1u16.to_le_bytes());
let d = read_field_discriminator(&buf, &u, 0, LE).expect("read");
assert_eq!(d.key, "1");
assert_eq!(d.variant_offset, 2);
assert_eq!(d.discriminator_size, 2);
}
#[test]
fn n1_read_field_discriminator_uint16_big_endian() {
let root = field_union_root("type", "uint16");
let u = doc_union(&root, "U");
let mut buf = vec![0u8; 8];
buf[0..2].copy_from_slice(&1u16.to_be_bytes());
let d = read_field_discriminator(&buf, &u, 0, BE).expect("read");
assert_eq!(d.key, "1");
assert_eq!(d.discriminator_size, 2);
}
#[test]
fn n1_read_field_discriminator_uint32_little_endian() {
let root = json!({
"$defs": {
"U": {
"kind": "union",
"discriminator": { "kind": "field", "name": "type" },
"fields": [ { "name": "type", "kind": "uint32" } ],
"mapping": {
"1024": { "kind": "struct", "fields": [ { "name": "n", "kind": "uint32" } ] }
}
}
}
});
let u = doc_union(&root, "U");
let mut buf = vec![0u8; 8];
buf[0..4].copy_from_slice(&1024u32.to_le_bytes());
let d = read_field_discriminator(&buf, &u, 0, LE).expect("read");
assert_eq!(d.key, "1024");
assert_eq!(d.variant_offset, 4);
assert_eq!(d.discriminator_size, 4);
}
#[test]
fn n1_read_field_discriminator_uint32_big_endian() {
let root = json!({
"$defs": {
"U": {
"kind": "union",
"discriminator": { "kind": "field", "name": "type" },
"fields": [ { "name": "type", "kind": "uint32" } ],
"mapping": {
"1024": { "kind": "struct", "fields": [ { "name": "n", "kind": "uint32" } ] }
}
}
}
});
let u = doc_union(&root, "U");
let mut buf = vec![0u8; 8];
buf[0..4].copy_from_slice(&1024u32.to_be_bytes());
let d = read_field_discriminator(&buf, &u, 0, BE).expect("read");
assert_eq!(d.key, "1024");
assert_eq!(d.discriminator_size, 4);
}
#[test]
fn read_field_discriminator_string_big_endian() {
let root = field_union_root("type", "string");