vj-sim 0.3.0

//! Task 7 — Concrete-hierarchy base assignment & the compile-time layout
//! query API (ledger ST-037, ST-067, ST-129, ST-135, ST-136).
//!
//! The second layout pass: given a template's already-built
//! [`super::StateLayout`] (Task 4/6), assign each *concrete instance* of that
//! template a distinct, non-overlapping base index into the global inline/
//! wide arrays by bump-allocating `template_size(template)` words per
//! instance (ST-067). `slot_of`/`template_size` stay template-relative only
//! (ST-129/ST-135) — no method here ever adds an instance base to a
//! [`super::SlotOffset`]; that addition is ENGINE's job at kernel-invocation
//! time, not STATE's.
//!
//! **Deviation from the plan's literal `templates: Box<TemplateTable>`
//! field type:** the plan's prose ("looked up from `templates.layout_of`")
//! names `dedup::TemplateTable` directly, but that type is `TemplateTable<'d>`
//! — it borrows `Symbol<'d>` representatives tied to the `sv_lang::Design`
//! it was built from, and also carries `instance_order`/`definitions`
//! bookkeeping that only matters during the dedup walk itself. `StateEngine`
//! must outlive the `Design` used to build it (its `InstanceHandle`s cross
//! the compiled-kernel FFI boundary, per this task's own header note, and
//! later tasks construct/move/thread a bare, non-generic `StateEngine` with
//! no `Design` in scope at all — see e.g. Task 9's "a test that imports only
//! `state::{StateEngine, StateCellId, ...}`"), so it cannot itself carry the
//! `'d` lifetime. [`TemplateRegistry`] below is the minimal owned substitute:
//! it holds only the `(TemplateKey, StateLayout)` pairs `slot_of`/
//! `template_size`/`allocate_instance` actually need (moved out of a built
//! `TemplateTable`, which — since `StateLayout` itself owns everything it
//! contains, see `SlotDescriptor`'s doc comment — never borrows from the
//! `Design`), with the same `layout_of` query method name/shape the plan's
//! prose describes. `StateEngine`'s own `templates` field keeps the plan's
//! `pub`, `Box`-wrapped shape.
//!
//! **Deviation:** the plan does not specify a `StateEngine` constructor for
//! this task (every other task's prose assumes one already exists). Added
//! [`StateEngine::new`], defaulting to `BuildMode::FourState` (`FourState`
//! matches every other task's default arena in this plan).
//!
//! **Task 13 deviation:** the plan's Files list for Task 13 (2-state
//! build-mode arena and read/write family) names only `state/write.rs` and
//! `state/arena.rs`, but neither of those already had a `StateEngine`
//! constructor that selects a [`BuildMode`] other than the hardcoded
//! `FourState` [`StateEngine::new`] above -- and every one of this plan's 14
//! existing `StateEngine::new(templates)` call sites depends on that
//! single-argument signature staying unchanged. [`StateEngine::new_with_mode`]
//! is added here (additive only, `new` now forwards to it with
//! `BuildMode::FourState`) as the minimal way to construct a
//! `BuildMode::TwoState` engine Task 13's own tests need, without touching
//! any existing call site.

use std::cell::UnsafeCell;
use std::collections::{HashMap, HashSet};
use std::rc::Rc;
use std::sync::atomic::{AtomicU64, Ordering};

use super::arena::{Arena, BuildMode};
use super::driver::TriregCharge;
use super::region_check::ForceObserved;
use super::write::{SlotLocation, VcState, WorkerSlot};
use super::{SlotClass, StateCellId, StateLayout};
use crate::netlist::{ClassTemplateId, InstanceId, InstantiatedSignalId, SymbolKey, TemplateKey};

/// Process-wide, monotonically-increasing source for each `StateEngine`'s own
/// [`VcState`]'s `engine_id`
/// (Task 8 deviation 2, see `state::write`'s module doc comment): every
/// `StateEngine` gets a distinct id, never reused, so the thread-local
/// self-registration cache in `state::write` can tell two different engines
/// apart even when they happen to share an OS thread (as this crate's own
/// test suite routinely does).
static NEXT_ENGINE_ID: AtomicU64 = AtomicU64::new(0);

/// One concrete instance's base index into the two global arrays, relative
/// to which every one of its template's template-relative [`super::SlotOffset`]
/// values must be added (ST-129) to get a real address — that addition is
/// ENGINE's job, never performed here.
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub struct InstanceBase {
    pub inline_base: u32,
    pub wide_base: u32,
}

/// The concrete type this plan's header design decision makes `SimHandle`
/// point at (Task 8 adds the `SimHandle` conversion). `layout`/`arena` are
/// private raw pointers; they stay valid for the life of this *handle*
/// (never merely for the life of the `StateEngine` that minted them — see
/// below) for two *independent* reasons (review fix, Task 7 r1 — the
/// original comment here conflated them and was wrong about `arena`):
///
/// - `arena` points at the [`Arena`] *struct itself*, inside the heap
///   allocation `arena_owner` below (an owned `Rc<Arena>` clone) keeps alive.
///   That alone is not sufficient for soundness, though: [`Arena`]'s own
///   backing storage is `UnsafeCell`-wrapped and every mutating accessor
///   takes `&self`, specifically so that no code in this module ever needs
///   to form a fresh `&mut Arena` after this pointer is minted — under
///   Rust's Stacked/Tree Borrows aliasing model, forming a `&mut Arena` at
///   all (regardless of whether it is used to write) invalidates every
///   previously-derived raw pointer's borrow-stack tag on `Arena`'s own
///   field memory. See `state::arena`'s module doc comment for the full
///   argument and the `cargo +nightly miri` failure this fixes.
/// - `layout` points into the heap allocation a `Box<StateLayout>` owns
///   inside the `TemplateRegistry` heap allocation `templates_owner` below
///   (an owned `Rc<TemplateRegistry>` clone) keeps alive, at the specific
///   `Box<StateLayout>` slot for this handle's `template` — see
///   [`TemplateRegistry`]'s doc comment — so it stays valid even if that
///   registry's backing `Vec` reallocates from a later `register` call.
///
/// **Review fix (Task 7 r2):** `arena_owner`/`templates_owner` are this
/// handle's *own* `Rc` clones, taken at the moment [`StateEngine::
/// allocate_instance`] mints the handle — not merely raw pointers derived
/// from the minting `StateEngine`'s `Rc<Arena>`/`Rc<TemplateRegistry>` with
/// no owned stake in them. `allocate_instance`'s signature
/// (`&mut self -> Option<Box<InstanceHandle>>`) returns a fully owned value
/// with no borrow-checked connection back to `self` — a helper function that
/// builds a `StateEngine`, calls `allocate_instance`, and returns just the
/// handle is completely ordinary code, and previously, once that
/// `StateEngine` (and therefore its `Rc`s) dropped, `arena`/`layout` above
/// would dangle: `cargo +nightly miri test -p vj-sim instance::` reported
/// "Undefined Behavior: constructing invalid value of type
/// &state::arena::Arena: encountered a dangling reference (use-after-free)"
/// at exactly that read site before this fix. Cloning an `Rc` bumps its
/// strong count, so as long as *any* `InstanceHandle` referencing a given
/// `Arena`/`TemplateRegistry` is alive, that allocation's refcount cannot
/// reach zero — independent of whether the `StateEngine` that minted the
/// handle is itself still alive. See
/// `dropping_the_state_engine_after_allocating_an_instance_leaves_the_handle_valid`
/// below for the Miri-checked regression test.
///
/// **Task 10 (ST-027):** `base`/`template` stay `pub` (CODEGEN needs `base` to
/// mint a `SimHandle` at instantiation time, per this plan's header design
/// decision), but this type never hands a caller a raw pointer it could add
/// `base`/a `SlotOffset` to itself — `layout`/`arena` are private, and the only
/// way to actually reach a signal's storage through this handle is the
/// accessor fns (`accessor_impl::sv_get_scalar`/`sv_set_scalar`/`_wide`) or
/// `arena()`/`layout()`'s own `pub(crate)`-scoped, already-safe accessors,
/// never a `pub fn` returning `*mut`/`*const` for external base+offset
/// arithmetic. See [`super::SlotOffset`]'s own doc comment for the
/// superseded-design context this resolves.
pub struct InstanceHandle {
    pub base: InstanceBase,
    pub template: TemplateKey,
    layout: *const StateLayout,
    arena: *const Arena,
    // Review fix (Task 7 r2): owned `Rc` clones whose sole job is to keep
    // the `Arena`/`TemplateRegistry` heap allocations `arena`/`layout` above
    // point into alive for exactly as long as this handle exists — see this
    // struct's doc comment. Never read directly (their `Drop` impl running
    // when this handle drops is the entire point of holding them), hence
    // `#[allow(dead_code)]`.
    #[allow(dead_code)]
    arena_owner: Rc<Arena>,
    #[allow(dead_code)]
    templates_owner: Rc<TemplateRegistry>,
    /// Task 9 retrofit (ST-013, ST-020..ST-023 — see `state::write`'s module
    /// doc comment): this handle's own clone of the `StateEngine` that
    /// minted it own `Rc<VcState>`, letting the accessor ABI
    /// (`accessor_impl.rs`) call `write::commit_write_at` — the exact same
    /// primitive `StateEngine::commit_write` calls — with no `&mut
    /// StateEngine` back-reference, the same way `arena_owner` lets it reach
    /// the arena directly.
    vc_owner: Rc<VcState>,
    /// Task 9 retrofit: one `StateCellId` per slot in this instance's
    /// template, in the same order `SlotId` indexes `StateLayout::slots`
    /// (`vj_abi::SlotId`'s own doc comment: "index into its template's
    /// `StateLayout.slots`") — minted once, here, at `allocate_instance`
    /// time via Task 8's `StateEngine::alloc_cell`, each already carrying
    /// this instance's absolute (base-applied) `SlotLocation`. This
    /// realizes Task 9's own prose ("Task 8 mints `StateCellId`s 1:1 with
    /// concrete instantiated signals during elaboration via `alloc_cell`")
    /// — `allocate_instance` is this wave's stand-in for "elaboration"
    /// (there is no separate elaboration pass yet).
    ///
    /// Past the own slots, one entry per foreign slot (see `foreign`): the
    /// named signal's own cell, so a cell lookup never needs to resolve.
    cells: std::cell::UnsafeCell<Vec<StateCellId>>,
    /// Foreign slots: slot `own + k` (`own` = the layout's slot count) names slot `foreign[k].1` of
    /// the instance `foreign[k].0` (a signal of another instance this one's
    /// code reaches -- an interface member through an interface port, a
    /// hierarchical reference, an interface task's body, a package
    /// variable). Set once every instance exists
    /// ([`crate::state::accessor_impl::set_foreign_slots`]).
    foreign: std::cell::RefCell<Vec<(*const InstanceHandle, usize)>>,
    /// Per foreign slot, the window of the named slot it stands for when it
    /// aliases a select of it (a ref port bound to `s.m` / `a[2]`,
    /// `state::alias`); empty when no foreign slot is windowed. A windowed
    /// slot's `cells` entry is its view cell, not the target's own.
    windows: std::cell::RefCell<Vec<Option<super::AliasWindow>>>,
    /// Backing for the `WideView`s of windowed-alias reads, which (unlike a
    /// whole slot's) have no arena storage to point at: a ring of
    /// `(aval, bval)` planes plus its cursor, so a view stays valid across
    /// the next few reads -- more than any one operation holds at once.
    wide_scratch: std::cell::RefCell<ScratchRing>,
    /// [`Self::inline_ptr_table`]'s cache: the inline plane's `(base
    /// pointer, length)` it was built against, then one entry per slot.
    inline_ptrs: std::cell::UnsafeCell<InlinePtrCache>,
}

/// See [`InstanceHandle::inline_ptr_table`].
type InlinePtrCache = ((usize, usize), Vec<*const u128>);

/// The scratch ring's cursor and the `(aval, bval)` planes held so far.
type ScratchRing = (usize, Vec<(Vec<u64>, Vec<u64>)>);

/// How many windowed wide reads stay valid at once (see
/// [`InstanceHandle::scratch_wide`]).
const WIDE_SCRATCH_SLOTS: usize = 32;

impl InstanceHandle {
    /// Parks one windowed wide read's planes in the scratch ring and returns
    /// pointers to them; valid until `WIDE_SCRATCH_SLOTS` further such reads.
    pub(crate) fn scratch_wide(&self, aval: Vec<u64>, bval: Vec<u64>) -> (*mut u64, *mut u64, u32) {
        let (next, ring) = &mut *self.wide_scratch.borrow_mut();
        let at = *next % WIDE_SCRATCH_SLOTS;
        *next += 1;
        if ring.len() < WIDE_SCRATCH_SLOTS {
            ring.push((aval, bval));
        } else {
            ring[at] = (aval, bval);
        }
        let (a, b) = &ring[at.min(ring.len() - 1)];
        (a.as_ptr().cast_mut(), b.as_ptr().cast_mut(), a.len() as u32)
    }

    /// SAFETY: `arena` points into the heap allocation this handle's own
    /// `arena_owner: Rc<Arena>` clone keeps alive for exactly as long as
    /// `self` does (see this struct's doc comment), and no `&mut Arena` is
    /// ever formed anywhere in this crate (Task 7 r1), so this shared
    /// reference cannot be invalidated by aliasing.
    pub(crate) fn arena(&self) -> &Arena {
        unsafe { &*self.arena }
    }

    /// SAFETY: `layout` points into the `Box<StateLayout>` this handle's
    /// `template` owns inside the heap allocation this handle's own
    /// `templates_owner: Rc<TemplateRegistry>` clone keeps alive;
    /// `TemplateRegistry::register` never moves an already-boxed
    /// `StateLayout` (see that struct's doc comment), so this pointer is
    /// stable for the handle's lifetime.
    pub(crate) fn layout(&self) -> &StateLayout {
        unsafe { &*self.layout }
    }

    /// The shared VC-tracking state this handle's own `vc_owner: Rc<VcState>`
    /// clone keeps alive (Task 9 retrofit — see this struct's doc comment).
    pub(crate) fn vc(&self) -> &VcState {
        &self.vc_owner
    }

    /// The `StateCellId` minted for this instance's `idx`-th slot (Task 9
    /// retrofit — see `cells`' own doc comment above).
    pub(crate) fn cell_for_slot(&self, idx: usize) -> StateCellId {
        // SAFETY: `cells` is only written by `set_foreign`, before any
        // kernel runs; no reference into it outlives this call.
        let cells = unsafe { &*self.cells.get() };
        cells[idx]
    }

    /// The instance whose own slot `idx` is, and its index there: `self`
    /// for one of its own slots, the target of a foreign slot otherwise
    /// (see `foreign`). The bare accessors (`sv_get_scalar`, ...) address
    /// own slots only -- they stay small enough for AOT+LTO to inline --
    /// and the production entry points resolve first
    /// ([`crate::state::accessor_impl::resolve_foreign_slot`]).
    #[inline]
    pub(crate) fn resolve_slot(&self, idx: usize) -> (&InstanceHandle, usize) {
        if idx < self.own_slot_count() {
            (self, idx)
        } else {
            self.resolve_foreign_slot(idx)
        }
    }

    /// [`Self::resolve_slot`] for a foreign slot -- kept out of line so the
    /// accessors' own-slot path stays small enough to inline.
    #[cold]
    #[inline(never)]
    fn resolve_foreign_slot(&self, idx: usize) -> (&InstanceHandle, usize) {
        let own = self.own_slot_count();
        let (owner, slot) = self
            .foreign
            .borrow()
            .get(idx - own)
            .copied()
            .unwrap_or_else(|| panic!("slot {idx} is neither own nor a registered foreign slot"));
        // SAFETY: `owner` came from a `SimHandle` minted by `into_sim_handle`
        // (`set_foreign_slots`), which lives for the whole run.
        (unsafe { &*owner }, slot)
    }

    /// Registers this instance's foreign slots (see `foreign`); each target
    /// must be one of its instance's own slots.
    pub(crate) fn set_foreign(
        &self,
        targets: Vec<(*const InstanceHandle, usize)>,
        views: Vec<Option<(super::AliasWindow, StateCellId)>>,
    ) {
        debug_assert_eq!(targets.len(), views.len());
        // SAFETY: called before any kernel runs, with no reference into
        // `cells` alive (`cell_for_slot` holds none past its own call).
        let cells = unsafe { &mut *self.cells.get() };
        cells.truncate(self.own_slot_count());
        for (&(owner, slot), view) in targets.iter().zip(&views) {
            match view {
                Some((_, cell)) => cells.push(*cell),
                // SAFETY: see `resolve_foreign_slot`.
                None => cells.push(unsafe { &*owner }.cell_for_slot(slot)),
            }
        }
        *self.windows.borrow_mut() = if views.iter().any(Option::is_some) {
            views.iter().map(|v| v.map(|(w, _)| w)).collect()
        } else {
            Vec::new()
        };
        *self.foreign.borrow_mut() = targets;
        // SAFETY: see `inline_ptr_table`.
        unsafe { &mut *self.inline_ptrs.get() }.1.clear();
    }

    /// The window foreign slot `idx` (`>= own_slot_count`) stands for within
    /// the slot [`Self::resolve_slot`] names, `None` for a whole-slot alias.
    #[cold]
    #[inline(never)]
    pub(crate) fn foreign_window(&self, idx: usize) -> Option<super::AliasWindow> {
        let own = self.own_slot_count();
        self.windows.borrow().get(idx - own).copied().flatten()
    }

    /// One entry per slot number (own, then foreign): the address of the
    /// arena `u128` cell `sv_get_scalar` reads for that slot, or null when
    /// the slot has no such cell (not `Inline`, or a windowed alias, whose
    /// read goes through [`super::accessor_impl::real_sv_get_scalar`]'s slow
    /// path). JIT code loads an `Inline` signal straight through it instead
    /// of calling the accessor (`sv_rt_inline_ptrs`).
    ///
    /// The table is rebuilt whenever the inline plane has moved or grown
    /// since it was built (instances are allocated before any kernel runs,
    /// but a stale pointer would be a use-after-free, so it is checked on
    /// every request) and whenever [`Self::set_foreign`] re-points a slot.
    /// The caller may use the returned pointer until the next arena
    /// allocation or `set_foreign`, i.e. for the rest of one kernel call.
    pub(crate) fn inline_ptr_table(&self) -> *const *const u128 {
        let inline = self.arena().inline();
        let stamp = (inline.as_ptr() as usize, inline.len());
        // SAFETY: single logical thread; no reference into the cache is
        // held across this call (the returned raw pointer is not a borrow).
        let cache = unsafe { &mut *self.inline_ptrs.get() };
        if cache.0 != stamp || cache.1.is_empty() {
            let own = self.own_slot_count();
            let windows = self.windows.borrow();
            let total = own + self.foreign.borrow().len();
            cache.1.clear();
            cache.1.extend((0..total).map(|idx| {
                if idx >= own && windows.get(idx - own).is_some_and(Option::is_some) {
                    return std::ptr::null();
                }
                let (owner, oidx) = self.resolve_slot(idx);
                let offset = owner.layout().slots[oidx].offset;
                if offset.class != SlotClass::Inline {
                    return std::ptr::null();
                }
                let word = owner.base.inline_base + offset.word_offset;
                owner
                    .arena()
                    .inline()
                    .get(word as usize)
                    .map_or(std::ptr::null(), std::ptr::from_ref)
            }));
            cache.0 = stamp;
        }
        cache.1.as_ptr()
    }

    /// The number of this instance's own slots.
    pub(crate) fn own_slot_count(&self) -> usize {
        self.layout().slots.len()
    }
}

/// Task 7's owned, `'static` substitute for `dedup::TemplateTable<'d>` (see
/// this module's top doc comment) — the minimal `(TemplateKey, StateLayout)`
/// registry [`StateEngine`] actually needs after elaboration, once the
/// `sv_lang::Design`-borrowing dedup walk itself is done.
///
/// **Review fix (Task 7 r1): each `StateLayout` is individually boxed, and
/// `templates` is `UnsafeCell`-wrapped so `register` never needs `&mut
/// self`.** `allocate_instance` hands out a `*const StateLayout` pointing
/// directly into this registry's backing storage (see
/// [`InstanceHandle::layout`]). Two independent hazards, both now fixed:
/// - With `templates: Vec<(TemplateKey, StateLayout)>` (the pre-fix shape),
///   that pointer targeted the `Vec`'s own tuple element storage — a later
///   `register` call growing this `pub` (via `StateEngine.templates`), and
///   therefore externally reachable, `Vec` could reallocate it and silently
///   dangle every previously-minted `layout` pointer, exactly the hazard
///   `register`'s old doc comment warned about without preventing it.
///   `Vec<(TemplateKey, Box<StateLayout>)>` instead gives each `StateLayout`
///   its own, independent heap allocation: growing the outer `Vec` moves the
///   `Box` pointers around, never the `StateLayout` values those pointers
///   point at.
/// - Separately (see [`StateEngine`]'s doc comment for the full argument):
///   under Rust's Stacked/Tree Borrows aliasing model, a `Box<T>` value
///   being moved — including as a field of a larger struct moved by value,
///   e.g. a `StateEngine` returned from a constructor — recursively
///   retags (as freshly, exclusively owned) everything reachable through
///   nested `Box`/`&`/`&mut` fields, which would silently invalidate a
///   `layout` pointer minted before the move even though `Box<StateLayout>`
///   itself never reallocates. `templates` therefore uses `UnsafeCell`
///   (mutated only through `&self`, never `&mut self`, exactly like
///   [`Arena`]) so that `StateEngine.templates` itself can be an `Rc`, not a
///   `Box` — `Rc`'s move doesn't carry that same recursive-retag semantics
///   (it isn't a uniquely-owning reference-like type), so Miri's retag walk
///   never descends into (and therefore never invalidates) the boxed
///   `StateLayout`s nested inside.
#[derive(Default)]
pub struct TemplateRegistry {
    templates: UnsafeCell<Vec<(TemplateKey, Box<StateLayout>)>>,
}

impl std::fmt::Debug for TemplateRegistry {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        // SAFETY: reads only `.len()` through a shared reference for
        // diagnostic purposes; per this type's single-logical-writer
        // invariant (see the struct doc comment and `register`), a length
        // read that doesn't outlive this call and races no concurrent
        // `register` call is sound.
        let len = unsafe { (*self.templates.get()).len() };
        f.debug_struct("TemplateRegistry")
            .field("template_count", &len)
            .finish()
    }
}

impl TemplateRegistry {
    pub fn new() -> Self {
        TemplateRegistry {
            templates: UnsafeCell::new(Vec::new()),
        }
    }

    /// Registers one template's layout. Callable at any time, including
    /// after `StateEngine::allocate_instance` has already minted one or more
    /// `InstanceHandle`s against this registry (see this struct's doc
    /// comment for why growing `templates` no longer moves any
    /// already-registered `StateLayout`, and never needs to invalidate a
    /// previously-minted `layout` pointer via a `StateEngine` move either).
    ///
    /// # Safety
    /// The caller must ensure no other live `&`/`&mut` borrow into this
    /// registry's backing `Vec` (in particular, no live `&StateLayout`
    /// returned by [`Self::layout_of`]) is held across this call, and that
    /// at most one logical writer registers templates at a time — the same
    /// single-logical-writer discipline `Arena`'s mutating accessors
    /// document. `&self`, not `&mut self`, so this can be called through a
    /// `Rc<TemplateRegistry>` (see [`StateEngine`]'s doc comment for why
    /// `templates` is an `Rc`, not a `Box`).
    pub unsafe fn register(&self, key: TemplateKey, layout: StateLayout) {
        // SAFETY: forwarded to this function's own `# Safety` section above.
        unsafe {
            (*self.templates.get()).push((key, Box::new(layout)));
        }
    }

    /// Template-relative layout lookup (ST-066-style: never a per-instance
    /// storage offset). Safe: any number of live `&StateLayout` reads
    /// coexist without aliasing; the aliasing obligation is on the `unsafe`
    /// [`Self::register`] instead (see its doc comment).
    pub fn layout_of(&self, key: &TemplateKey) -> Option<&StateLayout> {
        // SAFETY: produces a shared reference from the `UnsafeCell`. Multiple
        // concurrent reads never conflict with each other; the caller of
        // `register` is the one obligated not to hold a read like this one
        // live across their call.
        unsafe { &*self.templates.get() }
            .iter()
            .find(|(k, _)| k == key)
            .map(|(_, layout)| layout.as_ref())
    }

    /// Task 16 (ST-098): a hash of the ordered template-signature list this
    /// registry holds, in registration order — `rollback::ArenaSnapshot`'s
    /// `layout_hash` guard (`restore` refuses to poke a `StateEngine` whose
    /// registered template shapes don't match the snapshot's). Not in Task
    /// 16's own Files list (`state/rollback.rs` (new), `state/mod.rs`) — a
    /// method on the type that actually owns the template list is required,
    /// the same "add it directly to the type/struct that owns the resource"
    /// precedent Task 14/15's own `StateEngine` field additions already
    /// establish for this crate.
    ///
    /// Deliberately hashes only the *shape* every slot's `SlotOffset`
    /// describes (class/word_offset/bit_width/elem_word_count/
    /// row_stride_words/num_rows) plus each `TemplateKey` and each
    /// `StateLayout`'s own word counts — never `SlotDescriptor.initial`
    /// (`sv_lang::OwnedSVInt` has no stable hash of its own to reuse, and an
    /// initializer's concrete value never affects arena addressing, which is
    /// the one thing this guard exists to protect) and never
    /// `SlotDescriptor.net_info` (net classification also never affects
    /// where a slot's storage lives). Two registries with the same
    /// registered shapes in the same order hash identically regardless of
    /// initializer values; two registries whose shapes genuinely differ
    /// hash differently with overwhelming probability — exactly the
    /// distinction `restore`'s layout-mismatch guard needs.
    pub(crate) fn signature_hash(&self) -> u64 {
        use std::hash::Hasher;
        // SAFETY: reads the backing Vec's (key, layout) pairs through a
        // shared reference for a pure, non-outliving hash computation that
        // never itself grows or moves anything; per this type's
        // single-logical-writer invariant (see the struct doc comment and
        // `register`'s own `# Safety` section), a read like this one that
        // doesn't outlive this call and races no concurrent `register` call
        // is sound — identical reasoning to `layout_of` above.
        let templates: &Vec<(TemplateKey, Box<StateLayout>)> = unsafe { &*self.templates.get() };
        let mut hasher = std::collections::hash_map::DefaultHasher::new();
        for (key, layout) in templates.iter() {
            hasher.write_u32(key.definition);
            hasher.write_u64(key.params_hash);
            hasher.write_u32(layout.inline_words);
            hasher.write_u32(layout.wide_words);
            hasher.write_usize(layout.slots.len());
            for slot in &layout.slots {
                hasher.write_u32(slot.key.0);
                hasher.write_u8(match slot.offset.class {
                    SlotClass::Inline => 0,
                    SlotClass::Wide => 1,
                    SlotClass::Memory => 2,
                    SlotClass::ForceWide => 3,
                });
                hasher.write_u32(slot.offset.word_offset);
                hasher.write_u32(slot.offset.bit_width);
                hasher.write_u32(slot.offset.elem_word_count);
                hasher.write_u32(slot.offset.row_stride_words);
                hasher.write_u32(slot.offset.num_rows);
                hasher.write_u8(slot.net_info.is_some() as u8);
            }
        }
        hasher.finish()
    }
}

/// STATE's top-level owned state (arena storage + the compiled template set)
/// — **review fix: `arena`/`templates` are `Rc`-wrapped, not `Box`-wrapped or
/// embedded by value.** A `SimHandle`/`InstanceHandle` minted by
/// [`Self::allocate_instance`] must stay valid for the life of the
/// simulation run and cross the compiled-kernel FFI boundary, so the raw
/// pointers it stores must survive `StateEngine` itself being moved —
/// returned by value from a constructor, pushed into a `Vec`, or embedded in
/// a future root object are all ordinary things a caller does (this task's
/// own regression test exercises exactly this). With `arena`/`templates`
/// embedded directly, any such move would silently dangle every
/// previously-minted handle — no compiler diagnostic, UB on the next
/// accessor call.
///
/// A first review fix boxed them (`Box<Arena>`/`Box<TemplateRegistry>`),
/// reasoning that a heap allocation's address doesn't change when the
/// (movable) `Box` pointer value travels with a moved `StateEngine`. That
/// address argument is correct but insufficient: `cargo +nightly miri test
/// -p vj-sim instance::` still fails with `Box<Arena>`, because moving a
/// `Box<T>` value — including as a field of a larger struct passed by value
/// into a function, e.g. `Box::new(engine)` or returning `engine` from a
/// constructor — retags the `Box`'s pointee as freshly, exclusively owned
/// under Rust's Stacked/Tree Borrows model (the same aliasing treatment
/// `&mut T` gets), independent of the pointee's own address ever changing.
/// That invalidates every previously-derived raw pointer into it, `Arena`'s
/// own `UnsafeCell`-based interior mutability notwithstanding. `Rc<T>` does
/// not carry this exclusive-ownership retag-on-move semantics — it's a
/// plain (refcounted) shared-ownership pointer, not a reference-like type
/// Miri's retagging pass treats specially — so `Rc<Arena>`/
/// `Rc<TemplateRegistry>` keep the same "stable heap address, movable
/// container" property `Box` gave without the move-retag hazard. This is
/// consistent with (and depends on) `Arena`'s and `TemplateRegistry`'s own
/// mutating methods already being `&self`-based via `UnsafeCell`: `Rc`
/// cannot produce `&mut` access at all (short of `Rc::get_mut`, unused
/// here), so nothing about this design ever needed `&mut Arena`/
/// `&mut TemplateRegistry` in the first place. `self.arena`/`self.templates`
/// field access auto-derefs through `Rc` exactly as it would through a bare
/// field or a `Box`.
///
/// **Task 10 (ST-152):** every resource STATE owns — `arena`, `templates`,
/// `cells` (Task 8), `vc` (the Task 9 retrofit's VC-tracking state; the VC
/// dispatch table and undo log join this list once Tasks 11/16 land) — is a
/// *direct* field of this struct, never an `Option<*mut OtherGlobal>`
/// indirection and never a free-standing process-wide global or
/// lazily-initialized singleton cell of any kind anywhere in this crate (this
/// module's own test suite enforces the exact forbidden spellings by scanning
/// this crate's `state` source). The only pointer any accessor/ABI function
/// accepts to reach any of it is `SimHandle`/`InstanceHandle` (itself
/// reachable only via `SimHandle`, per this plan's header decision) or, for
/// scheduler-side callers, a `&`/`&mut StateEngine` reached however the caller
/// already holds one — never a second, independent "give me the arena"
/// pointer parameter threaded alongside it.
///
/// **Task 15 (ST-033):** every [`super::FourStateValue`] in the running
/// simulation lives in exactly one of `Arena.{inline,wide_aval,wide_bval}`
/// (or the `_2s` twins), reached only via a [`StateCellId`]/[`SlotLocation`]
/// STATE itself minted (Task 8's `alloc_cell`) — no other crate/module in
/// this workspace holds a second copy. This is the ST-152 discipline above
/// applied specifically to signal *values* (not merely STATE's own
/// resources): every read/write path this crate exposes
/// (`SignalAccess::read`/`write`, the accessor ABI, `driver::resolve_net_step`)
/// resolves a `StateCellId` down to a `SlotLocation` via `self.cells` and
/// indexes into `self.arena`'s planes directly, never into a second,
/// independently-populated store — see `region_check::
/// no_second_authoritative_copy_exists_for_a_cell_read_through_two_apis` for
/// the concrete regression proving two different read APIs observe the exact
/// same storage with no manual "sync" step between them.
pub struct StateEngine {
    pub arena: Rc<Arena>,
    pub templates: Rc<TemplateRegistry>,
    /// Task 8 (ST-109): dense `StateCellId -> SlotLocation` table;
    /// `alloc_cell` (`state::write`) pushes, `commit_write` indexes. See
    /// `state::write`'s module doc comment, deviation 1, for why this is
    /// `pub(crate)` rather than module-private.
    pub(crate) cells: Vec<SlotLocation>,
    /// Task 9 retrofit (see `state::write`'s module doc comment): bundles
    /// the former `current_time`/`current_region`/`worker_buffers`/
    /// `engine_id` fields behind one `Rc`-shared, interior-mutable handle so
    /// `InstanceHandle` (`allocate_instance` below) can clone a reference to
    /// it and call the exact same `write::commit_write_at` primitive
    /// `Self::commit_write` calls, without needing a `&mut StateEngine`
    /// back-reference. `set_current_time`/`set_current_region`/
    /// `register_worker_thread` below keep their exact pre-Task-9 public
    /// signatures; only their bodies now forward through this field.
    pub(crate) vc: Rc<VcState>,
    /// Task 11 (ST-035, ST-073, ST-074): the fixed 3-role VC listener
    /// dispatch table. `SignalAccess::subscribe` (`accessor_impl.rs`) is the
    /// one entry point that populates it; nothing else in this crate reaches
    /// it except through `&mut self.dispatch` on this direct field (ST-152 —
    /// no second, free-standing global).
    pub(crate) dispatch: super::dispatch::Dispatch,
    /// Task 14 (ST-034, `state::driver`): `trireg` decay-charge side table —
    /// `trireg`'s own `TriregCharge` is never stored in a normal
    /// `DriverEntry`/the arena's own planes (see `TriregCharge`'s own doc
    /// comment); this field only stores the state, the actual decay-timing
    /// behavior is a TIMING-subsystem hook out of this wave's scope. Not in
    /// Task 14's own Files list (which names only `state/driver.rs` (new)
    /// and `state/mod.rs`) — a direct field is required to give this side
    /// table anywhere to live, per ST-152's "every resource STATE owns is a
    /// direct field of `StateEngine`" discipline this same struct's own doc
    /// comment already establishes for every other field here.
    pub(crate) trireg_charge: HashMap<StateCellId, TriregCharge>,
    /// Task 15 (ST-122, `state::region_check`): the runtime guard for
    /// **external** writes — `driver::force_driver`/`release_driver` each
    /// call every entry here (in registration order) so a future
    /// partitioning/independence assumption can invalidate itself the
    /// instant an external `force`/`vpi_put_value` touches a
    /// region-crossing net, rather than trusting a now-stale static
    /// classification (ARCHITECTURE §4/§9). Not in Task 15's own Files list
    /// (which names only `state/region_check.rs` (new) and `state/mod.rs`)
    /// — a direct field is required to give this registry anywhere to live,
    /// per ST-152's discipline this same struct's own doc comment already
    /// establishes for every other field here (the same precedent Task 14's
    /// `trireg_charge` field sets immediately above).
    pub(crate) force_observed: Vec<Box<dyn ForceObserved>>,
    /// Task 16 (ST-098, `state::rollback`): per-cell reset behavior for
    /// `StateEngine::reset_to_initial`, populated in exact lockstep with
    /// `cells` — `write::StateEngine::alloc_cell` pushes a conservative
    /// `rollback::CellReset::AllX` default for *every* cell it allocates
    /// (regardless of caller), and `allocate_instance`'s per-slot loop below
    /// (the one call site that actually has a `SlotDescriptor` — with its
    /// `initial`/`net_info` — in scope) overwrites that default for each of
    /// a template's real slots. Not in Task 16's own Files list (`state/
    /// rollback.rs` (new), `state/mod.rs`) — a direct field is required to
    /// give this bookkeeping anywhere to live, per ST-152's discipline this
    /// same struct's own doc comment already establishes for every other
    /// field here (the same precedent Task 14's `trireg_charge` and Task
    /// 15's `force_observed` fields set immediately above).
    pub(crate) cell_reset: Vec<super::rollback::CellReset>,
    /// Task 17 (ST-103, ST-104, ST-110, `state::frame`): the STATE-owned
    /// backing storage for every `process::ActivationRecord` this engine
    /// hands out — `new_activation_record` below is the only place that
    /// grows it. Not in Task 17's own Files list (`state/frame.rs` (new),
    /// `state/mod.rs`) — a direct field is required to give this arena
    /// anywhere to live, per ST-152's discipline this same struct's own doc
    /// comment already establishes for every other field here (the same
    /// precedent Task 14's `trireg_charge`, Task 15's `force_observed`, and
    /// Task 16's `cell_reset` fields set immediately above).
    pub(crate) frame: super::frame::FrameArena,
    /// Task 18 (ST-118, ST-119, `state::introspect`): each `allocate_instance`
    /// call mints a fresh [`InstanceId`] from this counter (0, 1, 2, ...,
    /// never reused/freed — the same bump-allocator discipline `alloc_cell`
    /// already uses for `StateCellId`) and records it as the instance half
    /// of every one of that instance's slots' `InstantiatedSignalId` key in
    /// `hier_cells` below. Not in Task 18's own Files list (`state/
    /// introspect.rs` (new), `state/mod.rs`) — a direct field/counter is
    /// required to give per-instance identity anywhere to come from, per
    /// ST-152's discipline this same struct's own doc comment already
    /// establishes for every other field here (the same precedent Task
    /// 14/15/16/17's own `StateEngine` field additions set).
    pub(crate) next_instance_id: u32,
    /// Task 18 (ST-118, ST-119): the one `InstantiatedSignalId ->
    /// StateCellId` map [`introspect::read_by_hier_path`] resolves a
    /// hierarchical path's already-`NetlistView`-resolved signal identity
    /// through. Populated exclusively by `allocate_instance`'s per-slot loop
    /// below, in the same pass that mints each slot's `StateCellId` — never
    /// a second, independently-populated store (ST-152). Keyed by
    /// `InstantiatedSignalId` (instance + member), never by `member:
    /// SymbolKey` alone, which is the concrete data-structure-level fact
    /// that makes FRONTEND's compiled-body dedup invisible above this
    /// boundary (ST-119): two instances sharing one template's `SymbolKey`s
    /// still get two disjoint keys here, because each instance's own
    /// `InstanceId` (from `next_instance_id` above) differs.
    pub(crate) hier_cells: HashMap<InstantiatedSignalId, StateCellId>,
    /// R-run Task 16 (wave-B-state Task 5, ST-111): the set of
    /// [`StateCellId`]s [`super::event::StateEngine::alloc_event_cell`] has
    /// minted — an ordinary 1-bit `SlotClass::Inline` cell whose only
    /// distinguishing property is membership here (see `state::event`'s
    /// module doc comment for why this is a side-table, not a new
    /// `SlotClass::Event` variant). Not in this task's own Files list
    /// (`state/event.rs` (new), `state/mod.rs`) — a direct field is required
    /// to give this bookkeeping anywhere to live, per ST-152's discipline
    /// this same struct's own doc comment already establishes for every
    /// other field here (the same precedent `trireg_charge`/
    /// `force_observed`/`cell_reset`/`hier_cells` set).
    pub(crate) event_cells: HashSet<StateCellId>,
}

impl StateEngine {
    pub fn new(templates: TemplateRegistry) -> Self {
        Self::new_with_mode(templates, BuildMode::FourState)
    }

    /// Task 13 (ST-100, ST-101, ST-102): the only constructor that can
    /// produce a `BuildMode::TwoState` `StateEngine` -- `mode` is threaded
    /// straight into `Arena::new` and never mutated after (see
    /// `write::StateEngine::build_mode`'s own doc comment for why no setter
    /// exists anywhere in this crate).
    pub fn new_with_mode(templates: TemplateRegistry, mode: BuildMode) -> Self {
        let engine_id = NEXT_ENGINE_ID.fetch_add(1, Ordering::Relaxed);
        StateEngine {
            arena: Rc::new(Arena::new(mode)),
            templates: Rc::new(templates),
            cells: Vec::new(),
            vc: Rc::new(VcState::new(engine_id)),
            dispatch: super::dispatch::Dispatch::new(),
            trireg_charge: HashMap::new(),
            force_observed: Vec::new(),
            cell_reset: Vec::new(),
            frame: super::frame::FrameArena::new(),
            next_instance_id: 0,
            hier_cells: HashMap::new(),
            event_cells: HashSet::new(),
        }
    }

    /// Task 8's "how STATE learns now" header decision: called by ENGINE's
    /// region-eval loop once per region, before invoking any kernel/process
    /// that might call back into `commit_write`. `commit_write` itself never
    /// gains a time parameter -- it reads this (via `self.vc`) instead.
    pub fn set_current_time(&mut self, at: vj_abi::SimTime) {
        self.vc.set_current_time(at);
    }

    /// The `Region` counterpart to `set_current_time`.
    pub fn set_current_region(&mut self, region: vj_abi::Region) {
        self.vc.set_current_region(region);
    }

    /// Task 8's "cross-thread visibility of the VC speculative buffer"
    /// header decision: called once by each thread before its first
    /// `commit_write` (or self-called by `commit_write` itself on first
    /// use, via `state::write`'s thread-local cache). Pushes a fresh,
    /// individually-boxed `RefCell<Vec<VcRecord>>` onto `self.vc`'s
    /// `worker_buffers` and returns its index as an opaque `WorkerSlot` --
    /// each entry's own heap allocation has a stable address for the life of
    /// this `StateEngine` regardless of how many further times
    /// `worker_buffers` itself grows (the same box-per-entry stability
    /// argument `TemplateRegistry` already relies on for `StateLayout`).
    ///
    /// **SAFETY precondition** (documented here, exercised for real once a
    /// later task's `drain_committed_changes` exists): this function -- and
    /// every `commit_write` call that appends through the slot it returns --
    /// may run only when no other thread concurrently drains any registered
    /// buffer; eval and commit/drain are barrier-separated phases in this
    /// wave's WOCB execution model, never concurrent with each other.
    pub fn register_worker_thread(&mut self) -> WorkerSlot {
        // SAFETY: `&mut self` guarantees no other call into this
        // `StateEngine` (and therefore this `VcState`) races this one --
        // forwards to `VcState`'s own `&self`-based method (see its doc
        // comment, `state::write`, for why it's `unsafe`/`&self`-based at
        // all: so it is also callable through `InstanceHandle`'s own
        // `Rc<VcState>` clone with no `&mut StateEngine` in scope).
        unsafe { self.vc.register_worker_thread() }
    }

    /// Template-relative only (ST-129/ST-135): never adds an instance base.
    /// Returns `None` — never a fabricated default `SlotOffset` — when
    /// `symbol` is not present in `template`'s built `StateLayout`, or when
    /// `template` itself was never registered (ST-037: `SlotOffset` values
    /// are minted exclusively by the Task 4/6 layout builder, never
    /// synthesized on demand by this query API).
    pub fn slot_of(&self, template: &TemplateKey, symbol: SymbolKey) -> Option<super::SlotOffset> {
        self.templates
            .layout_of(template)?
            .slots
            .iter()
            .find(|slot| slot.key == symbol)
            .map(|slot| slot.offset)
    }

    /// `(inline_words, wide_words)` straight from the `StateLayout` (ST-135).
    pub fn template_size(&self, template: &TemplateKey) -> Option<(u32, u32)> {
        self.templates
            .layout_of(template)
            .map(|layout| (layout.inline_words, layout.wide_words))
    }

    /// Looks up `template_size`, bulk-allocates that many words via the two
    /// [`Arena`] bulk allocators, and returns a heap-boxed [`InstanceHandle`]
    /// (boxed so its address is stable — Task 8 turns that address directly
    /// into a `SimHandle`) whose base is disjoint from every previously
    /// issued base for this arena (ST-067/ST-136). Returns `None` iff
    /// `template` was never registered — never a fabricated zero-sized
    /// allocation.
    pub fn allocate_instance(&mut self, template: TemplateKey) -> Option<Box<InstanceHandle>> {
        let (inline_words, wide_words) = self.template_size(&template)?;
        // SAFETY: `self.arena` is this `StateEngine`'s own `Rc<Arena>`;
        // nothing else in this crate holds a live `&[T]`/`&mut [T]` borrow
        // of either plane across this call (allocate_instance never hands
        // one out), and `&mut self` above guarantees no other call into
        // this `StateEngine` (and therefore this `Arena`) races this one —
        // the single-logical-writer obligation `alloc_inline_words`/
        // `alloc_wide_words` document. Using `&self.arena` (never
        // `&mut self.arena`, i.e. never forming `&mut Arena`) here is the
        // whole point: it's what keeps every previously-minted
        // `InstanceHandle::arena` raw pointer's Stacked-Borrows tag alive
        // across this and every future `allocate_instance` call — see
        // `state::arena`'s module doc comment.
        let (inline_base, wide_base) = unsafe {
            (
                self.arena.alloc_inline_words(inline_words),
                self.arena.alloc_wide_words(wide_words),
            )
        };

        // `template_size` above already proved `template` is registered, so
        // this second lookup (needed to mint the raw pointer into the
        // registry's own storage) cannot fail in practice; `?` still keeps
        // this function total rather than assuming it via `.unwrap()`.
        // Points into the `Box<StateLayout>` `TemplateRegistry` owns for
        // this template (see `TemplateRegistry`'s doc comment) — stable
        // across any later `register` call, not into `templates`' own `Vec`
        // storage.
        let layout_ptr: *const StateLayout = self.templates.layout_of(&template)?;
        // A shared reborrow of `self.arena`'s pointee, never `&mut Arena` —
        // see the SAFETY comment above and `InstanceHandle`'s doc comment.
        let arena_ptr: *const Arena = &*self.arena;

        // Task 9 retrofit (ST-013, ST-020..ST-023 — see `InstanceHandle::cells`'
        // own doc comment): mint one absolute, already-base-applied
        // `StateCellId` per slot in this template's layout, via Task 8's
        // `alloc_cell`, in `SlotId`/slot-index order. `layout_ptr` was minted
        // into `self.templates`' own storage above (never `self`'s), so
        // borrowing through it here does not conflict with the `&mut self`
        // `alloc_cell` calls in the loop below.
        // SAFETY: `layout_ptr` was just returned by `self.templates.layout_of`
        // above and points into a `Box<StateLayout>` `TemplateRegistry` owns
        // for `template` — see that struct's doc comment for why this
        // pointer stays valid for as long as the registry itself does
        // (unaffected by the `&mut self.cells` pushes in the loop below,
        // which touch a wholly different field).
        let layout_ref: &StateLayout = unsafe { &*layout_ptr };
        // Task 18 retrofit (ST-118, ST-119, `state::introspect`): mint this
        // instance's own `InstanceId` from `self.next_instance_id`'s
        // bump-allocator counter, once per `allocate_instance` call — never
        // reused across instances, so the same template's `SymbolKey`s
        // combine with a *different* `InstanceId` for every concrete
        // instance, which is the concrete fact that keeps FRONTEND's
        // compiled-body dedup invisible above `hier_cells` (ST-119).
        let instance_id = InstanceId(self.next_instance_id);
        self.next_instance_id = self
            .next_instance_id
            .checked_add(1)
            .expect("more than u32::MAX instances allocated in one StateEngine");
        let mut cells = Vec::with_capacity(layout_ref.slots.len());
        for slot in &layout_ref.slots {
            let word_offset = match slot.offset.class {
                SlotClass::Inline => inline_base + slot.offset.word_offset,
                SlotClass::Wide | SlotClass::Memory => wide_base + slot.offset.word_offset,
                // Review fix (Task 14 r2, blocking finding 2): a real
                // elaborated signal's own `SlotOffset` (this loop's `slot`)
                // comes from the layout builder (Task 4/6), which never
                // emits `ForceWide` — that class exists only for
                // `driver::force_driver`'s own synthetic, caller-invisible
                // contribution cells, allocated directly via `alloc_cell`
                // outside this template-instantiation path entirely. See
                // `SlotClass::ForceWide`'s own doc comment.
                SlotClass::ForceWide => unreachable!(
                    "allocate_instance: no SlotDescriptor/SlotOffset the layout builder \
                     produces is ever SlotClass::ForceWide"
                ),
            };
            let loc = SlotLocation {
                class: slot.offset.class,
                word_offset,
                bit_width: slot.offset.bit_width,
                elem_word_count: slot.offset.elem_word_count,
                // Task 11 (ST-138): threaded from the real layout, never
                // zeroed here -- a `SlotClass::Memory` slot's own
                // `row_stride_words`/`num_rows` must survive onto the cell
                // this `StateCellId` resolves to, the same way
                // `accessor_impl::slot_location_for` threads them through
                // its own, separate `InstanceHandle`-relative resolution
                // path (see that function's doc comment).
                row_stride_words: slot.offset.row_stride_words,
                num_rows: slot.offset.num_rows,
            };
            let id = self.alloc_cell(loc);
            // Task 16 (ST-098): overwrite `alloc_cell`'s conservative
            // `CellReset::AllX` default with this real slot's actual
            // reset behavior, derived from its `SlotDescriptor` — the one
            // place in this crate that still has `slot` (and therefore
            // `slot.net_info`/`slot.initial`) in scope. A net-backed slot
            // is never touched by `reset_to_initial` at all (its reset
            // state is the emergent `Z` `driver::resolve_net_step` folds
            // to over zero active drivers, not a stored arena default —
            // see `SlotDescriptor::net_info`'s own doc comment); a slot
            // with a declared initializer re-blits it (the layout builder
            // already converted it to `FourStateValue`, CONTRACTS D4; an
            // initializer too wide for sv-lang's expansion cap arrives as
            // `None` and gets the same honest all-X default an
            // uninitialized slot gets); every other slot is all-X.
            let reset = if slot.net_info.is_some() {
                super::rollback::CellReset::Net
            } else if let Some(initial) = &slot.initial {
                super::rollback::CellReset::Initial(initial.clone())
            } else {
                super::rollback::CellReset::AllX
            };
            self.cell_reset[id.0 as usize] = reset;
            // Task 18 retrofit (ST-118, ST-119): record this slot's
            // `InstantiatedSignalId -> StateCellId` mapping in the same pass
            // that mints `id` above — never a second, later-populated store
            // (ST-152). `slot.key` is this member's already-deduped
            // `SymbolKey`; combined with `instance_id` above, the resulting
            // key is unique per (instance, member) even when two instances
            // share this template's `StateLayout`.
            self.hier_cells.insert(
                InstantiatedSignalId {
                    instance: instance_id,
                    member: slot.key,
                },
                id,
            );
            cells.push(id);
        }

        Some(Box::new(InstanceHandle {
            base: InstanceBase {
                inline_base,
                wide_base,
            },
            template,
            layout: layout_ptr,
            arena: arena_ptr,
            // Review fix (Task 7 r2): clone (not merely reference) this
            // StateEngine's own Rc<Arena>/Rc<TemplateRegistry> into the
            // handle itself, so the handle keeps these allocations alive on
            // its own — independent of whether `self` (this StateEngine) is
            // later dropped. See InstanceHandle's doc comment.
            arena_owner: Rc::clone(&self.arena),
            templates_owner: Rc::clone(&self.templates),
            // Task 9 retrofit: same reasoning as arena_owner/templates_owner
            // above, applied to the shared VC-tracking state.
            vc_owner: Rc::clone(&self.vc),
            cells: std::cell::UnsafeCell::new(cells),
            foreign: std::cell::RefCell::new(Vec::new()),
            windows: std::cell::RefCell::new(Vec::new()),
            wide_scratch: std::cell::RefCell::new((0, Vec::new())),
            inline_ptrs: std::cell::UnsafeCell::new(((0, 0), Vec::new())),
        }))
    }
}

/// Classes plan Task 4 (`docs/superpowers/plans/2026-10-03-classes.md`, LRM
/// 8.9): the `(InstanceId, TemplateKey)` pair [`StateEngine::
/// allocate_class_static_singletons`] uses for one `ClassTemplateId`'s
/// static-property singleton segment — a deterministic, pure function of
/// `class_template` alone (never re-derived differently by two call sites),
/// so every caller (that one `allocate_instance` call, and every later
/// foreign-slot reference a `Class::static_prop`/`obj.static_prop` access
/// mints) agrees on exactly which synthetic instance a given class
/// template's statics live in.
///
/// **Not defined in `netlist::`** despite otherwise matching
/// [`crate::netlist::ClassTemplateId`]'s own "frontend-independent, needed
/// by STATE" placement rationale: `netlist_dedup_invariants.rs`'s NL-018
/// guard (`codegen_never_needs_a_second_dedup_key_derivation`) source-scans
/// exactly five `netlist::` files for any function whose signature returns a
/// `TemplateKey`, asserting the only name that ever does is `dedup_key` —
/// protecting that module's own monopoly on a REAL elaborated module
/// instance's dedup key. This function mints a key for a wholly different,
/// synthetic thing (a class-static singleton has no `Definition`/dedup
/// identity of its own), the same way `vj::run`'s own package/bank
/// synthetic `TemplateKey`s are minted inline there, outside `netlist::`
/// entirely, rather than through a second `netlist::` function — this one
/// lives next to its one real caller for the identical reason.
///
/// **Reserved id-range convention**, matching the one `vj::run`
/// already established for its own synthetic instances rather than
/// inventing a second one: a real elaborated `InstanceId` is bump-allocated
/// from 0; a design's packages each get one counting DOWN from `u32::MAX`;
/// the net-resolution "bank" reserves the single id `u32::MAX - (1 << 20)`.
/// Class-static segments reserve the next block down again, counting from
/// `u32::MAX - (2 << 20)` — disjoint from both of those by the same
/// generous-headroom convention (not a mathematically tight partition: no
/// real design instantiates anywhere near `2^20` packages, let alone that
/// many packages *and* that many distinct class templates with static
/// properties at once), so a `TemplateKey.definition` minted here can never
/// collide with one a real module `Definition`, a package, or the bank ever
/// uses.
///
/// Panics (rather than silently wrapping into collision) if `class_template`'s
/// own dense id would exhaust that reserved block — `expect`-style, never a
/// wrapping/saturating fallback, because a wrapped id would silently alias a
/// *different* class template's segment: exactly the "gate that can never
/// fail" this plan's own correctness posture forbids. `2^20` class templates
/// with static properties is not a real design this implementation
/// supports; it is a defensive bound stated loudly rather than left to wrap.
pub fn class_static_instance(class_template: ClassTemplateId) -> (InstanceId, TemplateKey) {
    const RESERVED_BASE: u32 = u32::MAX - (2 << 20);
    let n = RESERVED_BASE
        .checked_sub(class_template.0)
        .unwrap_or_else(|| {
            panic!(
                "class_static_instance: ClassTemplateId({}) exceeds the reserved \
                 synthetic-instance-id block (RESERVED_BASE = {RESERVED_BASE}) -- \
                 more class templates with static properties than this \
                 implementation's id-space convention supports",
                class_template.0
            )
        });
    (
        InstanceId(n),
        TemplateKey {
            definition: n,
            // Distinct from the bank's `params_hash: 1` and every package's
            // `params_hash: 0` -- defensive only, `definition` alone is
            // already disjoint from both by construction above.
            params_hash: 2,
        },
    )
}

impl StateEngine {
    /// Classes plan Task 4 (`docs/superpowers/plans/2026-10-03-classes.md`,
    /// LRM 8.9): allocates one singleton segment per `(ClassTemplateId,
    /// StateLayout)` pair in `layouts` — exactly once per class template,
    /// regardless of how many objects of it are ever created (object
    /// creation itself, Task 5's `new()`/the object heap, never allocates
    /// one of these; it is minted once here, at elaboration time, sized
    /// exactly the way one real module `Template`'s own single set of
    /// slots already is). `layouts` is expected to already be deduplicated
    /// by the caller (e.g. one entry per
    /// [`crate::netlist::class_templates_with_statics`] result) — this
    /// function does not itself re-check for a repeated `ClassTemplateId`,
    /// the same "trust the caller's own dedup pass" discipline
    /// `allocate_instance`'s own callers already follow for `TemplateKey`.
    ///
    /// For each pair, registers `layout` under the `TemplateKey`
    /// [`class_static_instance`] mints for its `ClassTemplateId`, then
    /// calls [`Self::allocate_instance`] on it immediately — the same
    /// two-step `vj::run` already performs for a package's own
    /// singleton instance (the established precedent this reuses, not a
    /// new mechanism: `allocate_instance`'s own base-index/`SlotOffset`
    /// addressing and the `SlotId`/accessor-ABI read/write path are
    /// completely unchanged by this function).
    ///
    /// Returns the minted synthetic `InstanceId` (from
    /// [`class_static_instance`]) and the singleton's own `InstanceHandle`,
    /// keyed by `ClassTemplateId`, for the caller to keep alive for the
    /// rest of the run — a future task's `Class::static_prop`/
    /// `obj.static_prop` codegen resolves through the returned handle via a
    /// foreign slot, exactly like a package variable access does today
    /// (not wired by this function: nothing in this plan's Task 4 scope yet
    /// emits such a reference — see this plan's own Task 4 deviations).
    #[must_use]
    pub fn allocate_class_static_singletons(
        &mut self,
        layouts: Vec<(ClassTemplateId, StateLayout)>,
    ) -> HashMap<ClassTemplateId, (InstanceId, Box<InstanceHandle>)> {
        let mut out = HashMap::with_capacity(layouts.len());
        for (class_template, layout) in layouts {
            let (instance_id, key) = class_static_instance(class_template);
            // SAFETY: `self.templates` is this `StateEngine`'s own registry;
            // this function's own loop never calls `layout_of` (so no live
            // borrow into it is held across this call), and `&mut self`
            // throughout this whole function rules out a concurrent
            // `register`/`allocate_instance` call racing this one — the
            // same single-logical-writer discipline `allocate_instance`
            // above already relies on for this exact `templates` field.
            unsafe {
                self.templates.register(key, layout);
            }
            let handle = self.allocate_instance(key).unwrap_or_else(|| {
                unreachable!(
                    "allocate_class_static_singletons: class_static_instance's own \
                     TemplateKey was just registered above, so allocate_instance \
                     cannot report it unregistered"
                )
            });
            out.insert(class_template, (instance_id, handle));
        }
        out
    }

    /// Task 18 (ST-118, ST-119): the one lookup `introspect::
    /// read_by_hier_path` resolves a `NetlistView`-resolved
    /// `InstantiatedSignalId` through — `pub(crate)` (not `pub`) because the
    /// hierarchical-path-string entry point is `introspect::
    /// read_by_hier_path`, not this raw map query; kept visible within the
    /// crate so this module's own tests can exercise the map directly (per
    /// this task's own "obtained through the hier-path map, not by hand"
    /// test requirement) without a second, parallel lookup path.
    pub(crate) fn cell_for_instantiated_signal(
        &self,
        sig: InstantiatedSignalId,
    ) -> Option<StateCellId> {
        self.hier_cells.get(&sig).copied()
    }

    /// Task 18 structural test accessor (ST-119): the map's raw entry
    /// count, so a test can assert `hier_cells` grows by exactly one entry
    /// per signal per instance at the data-structure level, not merely by
    /// observing reads. `#[cfg(test)]` — no non-test code needs a bare
    /// count.
    #[cfg(test)]
    pub(crate) fn hier_cells_len(&self) -> usize {
        self.hier_cells.len()
    }
}

/// `const u128 *const *sv_rt_inline_ptrs(SimHandle)`: for each slot number of
/// `handle`'s instance, the address `sv_get_scalar` would read (null where
/// the slot needs the accessor: not `Inline`, or a windowed alias). JIT code
/// calls it once per segment and loads an `Inline` signal directly, so a
/// process activation costs one call rather than one per signal. Valid for
/// the rest of the kernel call ([`InstanceHandle::inline_ptr_table`]). Lives
/// here, not in `accessor_impl.rs`, whose source-scan test forbids raw
/// pointer returns (this is a read-only table, never base+offset arithmetic).
#[cfg_attr(feature = "aot-exports", unsafe(export_name = "sv_rt_inline_ptrs"))]
pub extern "C" fn real_sv_rt_inline_ptrs(handle: vj_abi::SimHandle) -> *const *const u128 {
    // SAFETY: see `sv_get_scalar`.
    unsafe { InstanceHandle::from_sim_handle(handle) }.inline_ptr_table()
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::state::{SlotClass, SlotDescriptor, SlotOffset};

    fn key(definition: u32, params_hash: u64) -> TemplateKey {
        TemplateKey {
            definition,
            params_hash,
        }
    }

    fn inline_slot(sym: u32, word_offset: u32, bit_width: u32) -> SlotDescriptor {
        SlotDescriptor {
            key: SymbolKey(sym),
            offset: SlotOffset {
                class: SlotClass::Inline,
                word_offset,
                bit_width,
                elem_word_count: 1,
                row_stride_words: 0,
                num_rows: 0,
            },
            initial: None,
            net_info: None,
        }
    }

    /// Task 11 (ST-138) regression: `allocate_instance`'s own `SlotLocation`
    /// construction (`self.cells` -- the `StateCellId`-keyed table
    /// `StateEngine::commit_write` indexes through) must thread a
    /// `SlotClass::Memory` slot's `row_stride_words`/`num_rows` through, not
    /// drop/zero them -- a *different* real path from
    /// `accessor_impl::slot_location_for`'s own regression test, which only
    /// covers the `InstanceHandle`-relative accessor-ABI resolution, not
    /// this `StateCellId`-keyed one. `row_stride_words: 5, num_rows: 9`
    /// deliberately distinct from `elem_word_count: 3`, so a bug that copied
    /// `elem_word_count` instead (or left both zeroed) is caught.
    #[test]
    fn allocate_instance_threads_memory_row_stride_and_num_rows_into_cells() {
        let templates = TemplateRegistry::new();
        let tmpl = key(0, 0);
        let memory_slot = SlotDescriptor {
            key: SymbolKey(0),
            offset: SlotOffset {
                class: SlotClass::Memory,
                word_offset: 0,
                bit_width: 32,
                elem_word_count: 3,
                row_stride_words: 5,
                num_rows: 9,
            },
            initial: None,
            net_info: None,
        };
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![memory_slot],
                    inline_words: 0,
                    wide_words: 3 * 9,
                },
            );
        }
        let mut engine = StateEngine::new(templates);
        engine.allocate_instance(tmpl).expect("template registered");

        assert_eq!(engine.cells.len(), 1);
        let loc = engine.cells[0];
        assert_eq!(loc.class, SlotClass::Memory);
        assert_eq!(loc.elem_word_count, 3);
        assert_eq!(
            loc.row_stride_words, 5,
            "allocate_instance must copy row_stride_words from the SlotOffset into self.cells, not drop/zero it"
        );
        assert_eq!(
            loc.num_rows, 9,
            "allocate_instance must copy num_rows from the SlotOffset into self.cells, not drop/zero it"
        );
    }

    /// ST-067: a template with `inline_words == 1`; call `allocate_instance`
    /// 8x (standing in for a real driver iterating 8 concrete generate-for
    /// instances Task 5's dedup walk discovers). The 8 returned
    /// `base.inline_base` values must be exactly `0,1,2,...,7` — strictly
    /// increasing, no gaps, no repeats.
    #[test]
    fn second_pass_assigns_distinct_nonoverlapping_bases_for_8_instances() {
        let templates = TemplateRegistry::new();
        let tmpl = key(0, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);

        let bases: Vec<u32> = (0..8)
            .map(|_| {
                engine
                    .allocate_instance(tmpl)
                    .expect("template is registered")
                    .base
                    .inline_base
            })
            .collect();

        assert_eq!(bases, vec![0, 1, 2, 3, 4, 5, 6, 7]);
    }

    /// ST-129: `slot_of(template, symbol)` called before *and* after calling
    /// `allocate_instance` for that template 3 times must return a
    /// byte-identical `SlotOffset` every time — the function's own signature
    /// takes no `InstanceHandle`/`InstanceId`, so it is structurally
    /// incapable of returning an instance-adjusted value.
    #[test]
    fn slot_of_is_template_relative_and_instance_independent() {
        let templates = TemplateRegistry::new();
        let tmpl = key(1, 0);
        let sym = SymbolKey(7);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![inline_slot(7, 0, 4)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);

        let before = engine
            .slot_of(&tmpl, sym)
            .expect("slot exists before allocation");

        for _ in 0..3 {
            engine
                .allocate_instance(tmpl)
                .expect("template is registered");
        }

        let after = engine
            .slot_of(&tmpl, sym)
            .expect("slot exists after allocation");
        assert_eq!(before, after, "slot_of must be instance-independent");
    }

    /// ST-135: a template with one 8-bit inline member and one 128-bit wide
    /// member; `template_size(key) == Some((1, 2))`.
    #[test]
    fn template_size_matches_the_built_layouts_word_counts() {
        let templates = TemplateRegistry::new();
        let tmpl = key(2, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![
                        inline_slot(0, 0, 8),
                        SlotDescriptor {
                            key: SymbolKey(1),
                            offset: SlotOffset {
                                class: SlotClass::Wide,
                                word_offset: 0,
                                bit_width: 128,
                                elem_word_count: 2,
                                row_stride_words: 0,
                                num_rows: 0,
                            },
                            initial: None,
                            net_info: None,
                        },
                    ],
                    inline_words: 1,
                    wide_words: 2,
                },
            );
        }
        let engine = StateEngine::new(templates);

        assert_eq!(engine.template_size(&tmpl), Some((1, 2)));
    }

    /// ST-136: 3 sequential `allocate_instance` calls, interleaved across two
    /// *different* templates; all 3 `InstanceHandle`s must have pairwise
    /// distinct `(base.inline_base, base.wide_base)` — proving the arena's
    /// bump allocator never reuses a freed/previous range (there is no free
    /// path at all in v1).
    #[test]
    fn allocate_instance_n_times_yields_n_handles_that_never_shrink_or_reuse() {
        let templates = TemplateRegistry::new();
        let tmpl_a = key(3, 0);
        let tmpl_b = key(4, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller; both calls are sequential.
        unsafe {
            templates.register(
                tmpl_a,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
            templates.register(
                tmpl_b,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);

        let h1 = engine.allocate_instance(tmpl_a).expect("tmpl_a registered");
        let h2 = engine.allocate_instance(tmpl_b).expect("tmpl_b registered");
        let h3 = engine.allocate_instance(tmpl_a).expect("tmpl_a registered");

        let bases: Vec<(u32, u32)> = [&h1, &h2, &h3]
            .iter()
            .map(|h| (h.base.inline_base, h.base.wide_base))
            .collect();

        assert_ne!(bases[0], bases[1]);
        assert_ne!(bases[1], bases[2]);
        assert_ne!(bases[0], bases[2]);
    }

    /// ST-037: `slot_of` for a `SymbolKey` never present in any built
    /// `StateLayout` returns `None`, never a fabricated default
    /// `SlotOffset` — the concrete proof that `SlotOffset` values are minted
    /// exclusively by the Task 4/6 builder, never synthesized on demand by
    /// the query API.
    #[test]
    fn slot_offset_has_no_external_constructor_outside_the_layout_builder() {
        let templates = TemplateRegistry::new();
        let tmpl = key(5, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let engine = StateEngine::new(templates);

        assert!(engine.slot_of(&tmpl, SymbolKey(999)).is_none());
        assert!(engine.slot_of(&key(999, 0), SymbolKey(0)).is_none());
    }

    /// Review fix regression test: the `Box<Arena>`/`Box<TemplateRegistry>`
    /// change this module's `StateEngine` doc comment describes exists to
    /// make this provable. Build a `StateEngine`, `allocate_instance` once,
    /// and (through the handle, before any move) write a known pattern into
    /// its one slot directly via the (Task 2) `Arena` accessors — this task
    /// does not yet have `commit_write` (Task 8), so the arena's own public
    /// `inline_mut`/`inline` methods stand in for it, reached through the
    /// still-owned `engine.arena` field. Then force the whole `StateEngine`
    /// to relocate in memory (moved out through a second `Box`, exactly the
    /// pattern the plan names), and read the same slot again through the
    /// *original*, pre-move `InstanceHandle`'s private raw pointers. With
    /// `arena`/`templates` embedded by value (not boxed), this would read
    /// through a dangling pointer once the move happens; it passes only
    /// because `Box`'s heap allocation address is move-invariant.
    ///
    /// Review fix r1: this test previously failed under
    /// `cargo +nightly miri test -p vj-sim instance::` even with the
    /// `Box<Arena>` fix in place — the struct-address move-invariance the
    /// doc comment above argues for is real, but it isn't what Miri flagged.
    /// `engine.arena.inline_mut()` below (standing in for Task 8's
    /// not-yet-written `commit_write`) used to require `&mut self.arena`,
    /// i.e. a fresh `&mut Arena` reborrow of the *whole* `Arena` struct's
    /// memory (not just its inline plane) — under Stacked/Tree Borrows that
    /// invalidates `handle.arena`'s already-minted tag on that same memory,
    /// independent of the struct's address never moving. `Arena` now backs
    /// its planes with `UnsafeCell` and takes `&self` for every mutating
    /// accessor (see `state::arena`'s module doc comment) precisely so this
    /// call never needs `&mut Arena` at all.
    #[test]
    fn moving_the_state_engine_after_allocating_an_instance_leaves_the_handle_valid() {
        let templates = TemplateRegistry::new();
        let tmpl = key(6, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);
        let handle = engine
            .allocate_instance(tmpl)
            .expect("template is registered");

        const PATTERN: u128 = 0xDEAD_BEEF_CAFE_F00D_0123_4567_89AB_CDEF;
        // SAFETY: single-threaded test; nothing else holds a live borrow of
        // the inline plane (no `inline()`/`inline_mut()` call is in flight)
        // across this write, satisfying `inline_mut`'s single-logical-writer
        // obligation.
        unsafe {
            engine.arena.inline_mut()[handle.base.inline_base as usize] = PATTERN;
        }

        // Force `engine` to relocate in memory: move it out through a second
        // `Box`, exactly the pattern the plan names.
        let relocated = Box::new(engine);
        let engine = *relocated;
        // Prevent an "unused" warning from suggesting `engine` is dead code —
        // its only remaining job is to keep the arena's heap allocation
        // alive for the read below.
        let _ = &engine;

        // SAFETY: `handle.arena` was derived from `engine.arena`'s `Rc`
        // heap allocation before the move above; that heap allocation itself
        // never moved (only the pointer inside `StateEngine`'s `Rc<Arena>`
        // did), `engine` (and therefore its `Rc<Arena>`) is still alive at
        // this point (bound in scope on the line above), and `handle.base
        // .inline_base` is a valid inline-plane index this same test
        // allocated via `allocate_instance`. No concurrent writer exists in
        // this single-threaded test, no `&mut Arena` was formed anywhere
        // between minting `handle.arena` and this read (the write above went
        // through `inline_mut`'s `&self`-based path), and — unlike `Box` —
        // moving `engine` (and therefore its `Rc<Arena>` field) through
        // `Box::new(engine)` above does not itself retag `Arena`'s memory as
        // exclusively (re-)owned, since `Rc` is not a uniquely-owning
        // reference-like type (see `StateEngine`'s doc comment); the exact
        // failure this comment used to be wrong about.
        let read_back = unsafe { (*handle.arena).inline()[handle.base.inline_base as usize] };
        assert_eq!(
            read_back, PATTERN,
            "handle must survive StateEngine relocating in memory"
        );
    }

    /// Review fix regression test (finding 1's own "every later
    /// `allocate_instance` call... invalidates every previously-issued
    /// `InstanceHandle`'s raw arena pointer" scenario, distinct from the
    /// move scenario above): mint a handle, then call `allocate_instance`
    /// many more times — enough to force the `Arena`'s backing `Vec`s to
    /// reallocate repeatedly — and prove the *first* handle's `arena`
    /// pointer, and the value written through it, both survive. Before this
    /// fix (`Arena`'s mutating accessors took `&mut self`), every one of
    /// these later `allocate_instance` calls formed a fresh `&mut Arena` via
    /// `self.arena.alloc_inline_words(...)`, which under Stacked/Tree
    /// Borrows invalidates the first handle's already-minted `*const Arena`
    /// tag — this test is what `cargo +nightly miri test -p vj-sim
    /// instance::` must stay clean against for that scenario, not just the
    /// move scenario the previous test covers.
    #[test]
    fn allocate_instance_called_many_more_times_leaves_the_first_handles_arena_pointer_valid() {
        let templates = TemplateRegistry::new();
        let tmpl = key(7, 0);
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);

        let first = engine
            .allocate_instance(tmpl)
            .expect("template is registered");

        const PATTERN: u128 = 0x1357_9BDF_2468_ACE0_FEDC_BA98_7654_3210;
        // SAFETY: `first.arena` points at the `Rc`-owned `Arena`; no other live
        // borrow of the inline plane is held across this write, and this
        // single-threaded test performs no concurrent write.
        unsafe {
            (*first.arena).inline_mut()[first.base.inline_base as usize] = PATTERN;
        }

        // Enough further allocations to force multiple `Vec` growth events
        // in the inline plane (well past any small fixed initial capacity).
        let mut later_handles = Vec::new();
        for _ in 0..4096 {
            later_handles.push(
                engine
                    .allocate_instance(tmpl)
                    .expect("template is registered"),
            );
        }

        // SAFETY: `first.arena` was minted before the loop above and never
        // invalidated by it (every `allocate_instance` call routes its
        // `Arena` mutation through `&self`-based accessors, never forming a
        // `&mut Arena`); `first.base.inline_base` is still a valid inline
        // index into the same, never-shrinking arena.
        let read_back = unsafe { (*first.arena).inline()[first.base.inline_base as usize] };
        assert_eq!(
            read_back, PATTERN,
            "first handle's arena pointer must survive many later allocate_instance calls"
        );
        assert_eq!(later_handles.len(), 4096);
    }

    /// Review fix regression test (findings 2 and 3): mint a handle for one
    /// template, then call the `pub` `TemplateRegistry::register` API
    /// directly (exactly as an external caller reachable via
    /// `StateEngine.templates` could) many more times — enough to force
    /// `TemplateRegistry`'s backing `Vec` to reallocate — and prove the
    /// first handle's `layout` pointer still resolves to the correct,
    /// original `StateLayout`. Before this fix
    /// (`templates: Vec<(TemplateKey, StateLayout)>`), `layout_ptr` pointed
    /// directly into the `Vec`'s own tuple storage, so this exact sequence
    /// would silently dangle it on reallocation with no compiler diagnostic
    /// (the empirical repro the finding describes); with each `StateLayout`
    /// individually boxed, the outer `Vec` reallocating never moves the
    /// `StateLayout` itself.
    #[test]
    fn registering_more_templates_after_allocating_an_instance_leaves_the_layout_pointer_valid() {
        let templates = TemplateRegistry::new();
        let tmpl_a = key(8, 0);
        // `StateLayout`/`SlotDescriptor` deliberately cannot derive
        // `Clone`/`PartialEq` (see `SlotDescriptor`'s doc comment:
        // `sv_lang::OwnedSVInt` has a `Drop` impl), so this test compares
        // `handle.layout`'s fields individually against these plain,
        // independently-constructed expected values rather than cloning the
        // registered `StateLayout` and comparing whole structs.
        let expected_offset = inline_slot(0, 0, 1).offset;
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl_a,
                StateLayout {
                    slots: vec![inline_slot(0, 0, 1)],
                    inline_words: 1,
                    wide_words: 0,
                },
            );
        }
        let mut engine = StateEngine::new(templates);

        let handle = engine
            .allocate_instance(tmpl_a)
            .expect("tmpl_a is registered");

        // Register enough further templates directly through the `pub`
        // `TemplateRegistry` API (`engine.templates`, exactly as an external
        // caller could) to force the backing `Vec` to reallocate at least
        // once past any small fixed initial capacity.
        //
        // SAFETY: nothing in this test holds a live `&StateLayout` (from
        // `layout_of`) across these calls, and this single-threaded test
        // performs no concurrent `register` call — `register`'s
        // single-logical-writer obligation holds.
        unsafe {
            for i in 0..10_000u32 {
                engine.templates.register(
                    key(100 + i, 0),
                    StateLayout {
                        slots: vec![inline_slot(0, 0, 1)],
                        inline_words: 1,
                        wide_words: 0,
                    },
                );
            }
        }

        // SAFETY: `handle.layout` was minted before the loop above, into the
        // `Box<StateLayout>` `TemplateRegistry` owns for `tmpl_a`; per
        // `TemplateRegistry`'s doc comment, growing its outer `Vec` never
        // moves that boxed `StateLayout`, so this pointer is still valid.
        // Review fix (Task 7 r2): `handle` itself now holds its own
        // `Rc<TemplateRegistry>` clone (`templates_owner`), which is what
        // actually keeps this allocation alive — not `engine` merely still
        // being in scope (a property of this test, not of `InstanceHandle`
        // as a type); see `dropping_the_state_engine_after_allocating_an_instance_leaves_the_handle_valid`
        // below, which proves this by dropping `engine` before this read.
        let layout_via_handle = unsafe { &*handle.layout };
        assert_eq!(
            layout_via_handle.inline_words, 1,
            "layout pointer must survive further register() calls after allocate_instance"
        );
        assert_eq!(layout_via_handle.wide_words, 0);
        assert_eq!(layout_via_handle.slots.len(), 1);
        assert_eq!(layout_via_handle.slots[0].key, SymbolKey(0));
        assert_eq!(layout_via_handle.slots[0].offset, expected_offset);
        // Cross-check against the safe query API too, which must agree.
        assert_eq!(engine.template_size(&tmpl_a), Some((1, 0)));
    }

    /// Review fix regression test (Task 7 r2, finding 1): the drop-scenario
    /// counterpart to `moving_the_state_engine_after_allocating_an_instance_
    /// leaves_the_handle_valid` above (that test moves the `StateEngine`; this
    /// one drops it outright — a strictly stronger hazard). Build a
    /// `StateEngine`, `allocate_instance` once, write a known pattern through
    /// the handle, then explicitly `drop` the `StateEngine` itself — the
    /// realistic pattern the finding names: a helper function that builds a
    /// `StateEngine`, calls `allocate_instance`, and returns just the
    /// (fully owned, detached) `Box<InstanceHandle>`, with nothing in
    /// `allocate_instance`'s own signature (`&mut self -> Option<Box
    /// <InstanceHandle>>`) tying the handle's lifetime back to `self`. Read
    /// the same slot again through the handle *after* the engine has been
    /// dropped and assert it still reads the correct pattern.
    ///
    /// Before `arena_owner`/`templates_owner` existed on `InstanceHandle`
    /// (see its doc comment), `handle.arena`/`handle.layout` were raw
    /// pointers derived from — but not keeping alive — `engine`'s
    /// `Rc<Arena>`/`Rc<TemplateRegistry>`; dropping `engine` here would drop
    /// each `Rc`'s last strong reference, free the backing heap allocation,
    /// and dangle both pointers. `cargo +nightly miri test -p vj-sim
    /// instance::` reported exactly this: "Undefined Behavior: constructing
    /// invalid value of type &state::arena::Arena: encountered a dangling
    /// reference (use-after-free)" at the read below, before this fix. This
    /// test is what that `cargo +nightly miri test` invocation must stay
    /// clean against for the drop scenario specifically, the same way the
    /// two `allocate_instance`/`register`-reallocation tests above pin it
    /// for their own scenarios.
    #[test]
    fn dropping_the_state_engine_after_allocating_an_instance_leaves_the_handle_valid() {
        const PATTERN: u128 = 0x0F1E_2D3C_4B5A_6978_8796_A5B4_C3D2_E1F0;

        // Build the engine, allocate an instance, write the pattern, and
        // drop the engine, all inside this block — `handle` is the only
        // thing that survives it, mirroring "a helper function that returns
        // just the handle" from the finding.
        let handle = {
            let templates = TemplateRegistry::new();
            let tmpl = key(9, 0);
            // SAFETY: `templates` is a local, single-owned value with no
            // other live borrow and no concurrent caller.
            unsafe {
                templates.register(
                    tmpl,
                    StateLayout {
                        slots: vec![inline_slot(0, 0, 1)],
                        inline_words: 1,
                        wide_words: 0,
                    },
                );
            }
            let mut engine = StateEngine::new(templates);
            let handle = engine
                .allocate_instance(tmpl)
                .expect("template is registered");

            // SAFETY: single-threaded test; nothing else holds a live borrow
            // of the inline plane across this write, satisfying
            // `inline_mut`'s single-logical-writer obligation.
            unsafe {
                engine.arena.inline_mut()[handle.base.inline_base as usize] = PATTERN;
            }

            // The exact hazard the finding describes: drop the StateEngine
            // while `handle` is still alive and about to escape this block.
            drop(engine);

            handle
        };

        // SAFETY: `handle.arena` was cloned into `handle`'s own
        // `arena_owner: Rc<Arena>` field at `allocate_instance` time (Task 7
        // r2's fix); that `Rc` clone — not `engine`, which is fully dropped
        // by this point — is what keeps the `Arena` heap allocation alive
        // here, so `handle.arena` is still a valid pointer into it.
        // `handle.base.inline_base` is a valid inline-plane index this same
        // test allocated via `allocate_instance`, and no `&mut Arena` was
        // ever formed between the write above and this read (both go
        // through `Arena`'s `&self`-based accessors).
        let read_back = unsafe { (*handle.arena).inline()[handle.base.inline_base as usize] };
        assert_eq!(
            read_back, PATTERN,
            "handle must survive the StateEngine that minted it being dropped outright"
        );
    }

    /// Classes plan Task 4: [`class_static_instance`] is a pure, idempotent
    /// function of its `ClassTemplateId` argument alone — two independent
    /// call sites (e.g. two different `new()` sites' codegen resolving the
    /// *same* class's static property) must mint the byte-identical
    /// `(InstanceId, TemplateKey)` pair, never two different ones that would
    /// silently split one class template's statics across two arena
    /// segments.
    #[test]
    fn class_static_instance_is_deterministic_across_repeat_calls() {
        let id = ClassTemplateId(3);
        assert_eq!(class_static_instance(id), class_static_instance(id));
    }

    /// Two distinct `ClassTemplateId`s must never mint the same
    /// `(InstanceId, TemplateKey)` — the one property that actually matters
    /// for `TemplateRegistry::register`'s correctness (a collision here
    /// would silently alias two unrelated classes' static-property storage).
    #[test]
    fn class_static_instance_is_injective_over_distinct_class_templates() {
        let (a_inst, a_key) = class_static_instance(ClassTemplateId(0));
        let (b_inst, b_key) = class_static_instance(ClassTemplateId(1));
        assert_ne!(a_inst, b_inst);
        assert_ne!(a_key, b_key);
    }

    /// The reserved id block this function mints from must stay strictly
    /// below the bank's own reserved id (`u32::MAX - (1 << 20)`,
    /// `vj::run`) for every `ClassTemplateId` a real run could ever
    /// produce (Task 2's own dedup never emits more class templates than
    /// there are `new()` call sites in the design) — proving the two
    /// synthetic ranges this function's own doc comment claims are disjoint
    /// actually are, rather than merely asserting it in prose.
    #[test]
    fn class_static_instance_stays_clear_of_the_bank_reserved_id() {
        const BANK_RESERVED: u32 = u32::MAX - (1 << 20);
        for raw in [0u32, 1, 1000, 1 << 19] {
            let (_, key) = class_static_instance(ClassTemplateId(raw));
            assert!(
                key.definition < BANK_RESERVED,
                "ClassTemplateId({raw})'s minted definition {} must stay below \
                 the bank's reserved id {BANK_RESERVED}",
                key.definition
            );
        }
    }

    /// A class template whose id would exhaust the reserved block panics
    /// loudly rather than wrapping into a collision with a lower
    /// `ClassTemplateId`'s own segment ("no gate that cannot fail" applied
    /// to id-space exhaustion).
    #[test]
    #[should_panic(expected = "exceeds the reserved")]
    fn class_static_instance_panics_rather_than_wraps_past_the_reserved_block() {
        let _ = class_static_instance(ClassTemplateId(u32::MAX));
    }

    /// Sanity: the function really does return a `TemplateKey` with a
    /// `params_hash` distinguishable from the bank's own/every package's own
    /// (defensive only — `definition` alone is already disjoint) — pinned
    /// here so a future edit to the bank/package convention in
    /// `vj::run` is forced to notice this function's own
    /// disjointness claim.
    #[test]
    fn class_static_instance_params_hash_is_not_the_banks_or_a_packages() {
        let (_, key) = class_static_instance(ClassTemplateId(0));
        assert_ne!(key.params_hash, 1, "1 is the bank's own params_hash");
        assert_ne!(key.params_hash, 0, "0 is every package's own params_hash");
    }

    /// Classes plan Task 4 (LRM 8.9): `allocate_class_static_singletons`
    /// called once with two distinct `ClassTemplateId`s' layouts (each
    /// shaped like a `static int count;`) must hand back two handles whose
    /// storage never aliases — writing through one class template's handle
    /// must leave the other's completely untouched. This is the structural
    /// half of "two objects of a class with `static int count` both see
    /// the same value after one increments it": since every object of ONE
    /// class template shares this single handle (object creation itself is
    /// Task 5's job and never mints a second one), the other half — "the
    /// same value" — follows for free as long as no *other* class
    /// template's segment can ever overlap it, which is what this test
    /// pins.
    #[test]
    fn allocate_class_static_singletons_gives_each_class_template_its_own_storage() {
        let templates = TemplateRegistry::new();
        let mut engine = StateEngine::new(templates);

        let layout_of = || StateLayout {
            slots: vec![inline_slot(0, 0, 32)],
            inline_words: 1,
            wide_words: 0,
        };
        let a = ClassTemplateId(0);
        let b = ClassTemplateId(1);
        let mut handles =
            engine.allocate_class_static_singletons(vec![(a, layout_of()), (b, layout_of())]);

        let (_, handle_a) = handles.remove(&a).expect("class template a allocated");
        let (_, handle_b) = handles.remove(&b).expect("class template b allocated");

        // SAFETY: single-threaded test; nothing else holds a live borrow of
        // the inline plane across these writes.
        unsafe {
            engine.arena.inline_mut()[handle_a.base.inline_base as usize] = 11;
            engine.arena.inline_mut()[handle_b.base.inline_base as usize] = 22;
        }
        assert_eq!(
            engine.arena.inline()[handle_a.base.inline_base as usize],
            11
        );
        assert_eq!(
            engine.arena.inline()[handle_b.base.inline_base as usize],
            22,
            "class template b's static-property storage must be untouched by \
             a write through class template a's own singleton handle"
        );
    }

    /// Two reads through the SAME class template's singleton handle (one
    /// standing in for object #1's `obj.count` read, the other for object
    /// #2's, since Task 5's `new()` does not exist yet to mint two real
    /// objects) must observe the write the first one made — the literal
    /// "two objects ... both see the same value after one increments it"
    /// scenario, exercised at the mechanism level this task owns.
    #[test]
    fn allocate_class_static_singletons_write_is_visible_to_every_future_reader() {
        let templates = TemplateRegistry::new();
        let mut engine = StateEngine::new(templates);
        let class_template = ClassTemplateId(0);
        let layout = StateLayout {
            slots: vec![inline_slot(0, 0, 32)],
            inline_words: 1,
            wide_words: 0,
        };
        let mut handles = engine.allocate_class_static_singletons(vec![(class_template, layout)]);
        let (_, handle) = handles.remove(&class_template).unwrap();
        let slot = handle.base.inline_base as usize;

        // "object #1" increments the shared static counter.
        // SAFETY: single-threaded test, no concurrent borrow of the plane.
        unsafe {
            engine.arena.inline_mut()[slot] = 0;
            engine.arena.inline_mut()[slot] += 1;
        }
        // "object #2" reads it back through the very same singleton handle
        // (there is only ever one -- that is the whole point of this
        // mechanism, not an incidental test detail).
        assert_eq!(
            engine.arena.inline()[slot],
            1,
            "a second reader of the same class template's static property must \
             observe the first reader's write, since both resolve to the same \
             singleton handle"
        );
    }

    /// `allocate_class_static_singletons` must mint the exact synthetic
    /// `InstanceId` [`class_static_instance`] independently computes for the
    /// same `ClassTemplateId` -- never a second, divergent numbering scheme
    /// of its own.
    #[test]
    fn allocate_class_static_singletons_instance_id_matches_class_static_instance() {
        let templates = TemplateRegistry::new();
        let mut engine = StateEngine::new(templates);
        let class_template = ClassTemplateId(7);
        let layout = StateLayout {
            slots: vec![inline_slot(0, 0, 1)],
            inline_words: 1,
            wide_words: 0,
        };
        let mut handles = engine.allocate_class_static_singletons(vec![(class_template, layout)]);
        let (instance_id, _) = handles.remove(&class_template).unwrap();

        let (expected_instance_id, _) = class_static_instance(class_template);
        assert_eq!(instance_id, expected_instance_id);
    }

    /// Calling `allocate_class_static_singletons` with an empty `layouts`
    /// (every class template in the design has no `static` property) must
    /// allocate nothing and return an empty map -- never a phantom,
    /// zero-width singleton no class actually needs.
    #[test]
    fn allocate_class_static_singletons_with_no_layouts_allocates_nothing() {
        let templates = TemplateRegistry::new();
        let mut engine = StateEngine::new(templates);
        let handles = engine.allocate_class_static_singletons(Vec::new());
        assert!(handles.is_empty());
    }
}