From b7c7dbe2a1bd4de60bdc447fa9302db83cb61ad1 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Wed, 2 Sep 2026 08:59:13 +0000 Subject: [PATCH] =?UTF-8?q?LayoutBuilder=20caches=20the=20owned=20BastDoc?= =?UTF-8?q?=20(ADR-012=20=C2=A72a,=20plan=20phase=204)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builder stores doc: BastDoc + endian (the doc_value: Value + root_name: String cache is gone); new parses the typed tree once and build walks &self.doc — the per-build BastDoc::new re-parse (layout_builder.rs M1) is retired. - build's root-is-struct re-check replaces its unreachable!() with a clean Schema error (AGENTS.md §3 never-panic; invariant unchanged — new already rejects non-struct roots). - Boxing fallout: the builder now holds the full owned tree, so Layout::Packed boxes it (Box) to keep the engine's Layout enum variant sizes balanced (clippy large_enum_variant). layout_builder() still returns Option<&LayoutBuilder> via auto-deref; public API unchanged. Verification: 465 tests pass unchanged (layout_builder.rs suites drive new/build through the public API); clippy -D warnings clean; wasm32 release build green. --- docs/plans/030-compiled-forms.md | 15 ++++++++++++++- src/engine.rs | 4 ++-- src/layout_builder.rs | 26 ++++++++++++-------------- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/docs/plans/030-compiled-forms.md b/docs/plans/030-compiled-forms.md index dafc5f3..63c2e04 100644 --- a/docs/plans/030-compiled-forms.md +++ b/docs/plans/030-compiled-forms.md @@ -482,7 +482,20 @@ wasm-relevant). --- -## Phase 4 — `LayoutBuilder` caches the owned `BastDoc` (ADR-012 §2a) +## Phase 4 — `LayoutBuilder` caches the owned `BastDoc` (ADR-012 §2a) — **DONE (2026-09-02)** + +> **Status: implemented.** `LayoutBuilder` stores `doc: BastDoc` + +> `endian` (the `doc_value: Value` + `root_name: String` cache is +> gone); `new` parses once, `build` walks `&self.doc` — the +> `layout_builder.rs` re-parse (M1) is retired. The `build`-time +> root-is-struct re-check replaced its `unreachable!()` with a clean +> `Schema` error (AGENTS.md §3 never-panic; the invariant is +> unchanged — `new` already rejects non-struct roots). Boxing fallout: +> the builder now holds the full owned tree, so `Layout::Packed` +> boxes it (`builder: Box`) to keep the engine's +> `Layout` enum variant sizes balanced (clippy +> `large_enum_variant`); `layout_builder()` still returns +> `Option<&LayoutBuilder>` via auto-deref, public API unchanged. **Goal:** `LayoutBuilder::new` parses the owned `BastDoc` once and stores it; `build` reuses it. Removes the `layout_builder.rs:190` diff --git a/src/engine.rs b/src/engine.rs index 420c539..132a045 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -54,7 +54,7 @@ enum Layout { /// hands out (ADR-007 — the reader owns its cursor state, so the /// engine is a factory, not a holder). Packed { - builder: LayoutBuilder, + builder: Box, plan: Arc, }, /// Aligned static layout. Field offsets are precomputed in an @@ -152,7 +152,7 @@ impl AlkTypeEngine { let validation_plan = Arc::new(ValidationPlan::compile(&doc)?); let layout = match mode { LayoutMode::Packed => { - let builder = LayoutBuilder::new(bast_doc, root_name)?; + let builder = Box::new(LayoutBuilder::new(bast_doc, root_name)?); let plan = Arc::new(ReadPlan::compile(bast_doc, root_name)?); Layout::Packed { builder, plan } } diff --git a/src/layout_builder.rs b/src/layout_builder.rs index fd35053..e112296 100644 --- a/src/layout_builder.rs +++ b/src/layout_builder.rs @@ -129,8 +129,7 @@ impl PackedLayout { /// this is correct for protocol wire formats, which pack fields tightly. #[derive(Debug)] pub struct LayoutBuilder { - doc_value: Value, - root_name: String, + doc: BastDoc, endian: Endian, } @@ -140,9 +139,9 @@ impl LayoutBuilder { /// The root type must be a struct. Endianness is read from the /// root struct's `endian` annotation (defaults to little-endian). /// - /// The builder retains the raw BAST `Value` and re-parses the typed - /// tree on each [`build`](Self::build) call; this is cheap (the - /// typed tree borrows from the source without cloning field data). + /// The builder parses the owned [`BastDoc`] once here (ADR-012 §2a) + /// and every [`build`](Self::build) call reuses the cached tree — + /// no re-parse, no `Value` clone per build. /// /// # Errors /// @@ -161,11 +160,7 @@ impl LayoutBuilder { } }; let endian = struct_node.endian(); - Ok(Self { - doc_value: bast_doc.clone(), - root_name: root_name.to_string(), - endian, - }) + Ok(Self { doc, endian }) } /// Build the packed layout given actual data sizes for variable-length @@ -187,14 +182,17 @@ impl LayoutBuilder { /// sizes in `var_sizes`, missing discriminator values, or unknown /// discriminator values. pub fn build(&self, var_sizes: &HashMap) -> Result { - let doc = BastDoc::new(&self.doc_value, &self.root_name)?; - let root_def = doc.root_def(); + let root_def = self.doc.root_def(); let struct_node = match root_def.kind() { BastDefKind::Struct(s) => s, - _ => unreachable!("checked in new; doc_value is immutable"), + _ => { + return Err(AlkTypeError::Schema( + "internal: LayoutBuilder root is not a struct (checked in new)".to_string(), + )); + } }; let mut ctx = BuildCtx { - doc: &doc, + doc: &self.doc, var_sizes, fields: Vec::new(), };