diff --git a/src/layout_builder.rs b/src/layout_builder.rs index 4218fd8..11d3d2b 100644 --- a/src/layout_builder.rs +++ b/src/layout_builder.rs @@ -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, total_size: usize, } @@ -83,10 +84,8 @@ impl PackedLayout { /// TUnion byte-offset discriminators, the discriminator is recorded /// under the synthetic 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 { + 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!({ diff --git a/src/offset_map.rs b/src/offset_map.rs index aa0d89b..c6eb855 100644 --- a/src/offset_map.rs +++ b/src/offset_map.rs @@ -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, 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 { + 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 { /// 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] diff --git a/src/tunion.rs b/src/tunion.rs index 2bd7a91..0d1a077 100644 --- a/src/tunion.rs +++ b/src/tunion.rs @@ -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");