vj-sim 0.3.0

//! Task 5 — Template dedup (hash-then-verify) and deterministic traversal
//! order (ledger ST-059, ST-064, ST-065, ST-066).
//!
//! `TemplateTable::build` walks every concrete instance reachable from
//! `design.top_instances()` (via `Symbol::instance_body()`/`members()`'s
//! `Instance`/`InstanceArray`-kind entries, expanding
//! `instance_array_elements()` for any array-of-instances declaration along
//! the way -- recursing through nested `InstanceArray` elements for a
//! multi-dimensional array -- and recursing into `GenerateBlock`/
//! `GenerateBlockArray`-kind members for `if`/`case`/`for`-generate content,
//! which is the actual `SymbolKind` a `generate for` loop of instances
//! elaborates to, skipping any generate branch
//! `Symbol::generate_block_is_uninstantiated` reports as not taken),
//! building a [`super::StateLayout`] for each one (Task 4/6's
//! `build_template_layout`, called once per concrete instance — ST-065's own
//! text: `Compilation::compile()` forces `DisableInstanceCaching`, so slang
//! never pointer-shares two structurally-identical bodies and every concrete
//! instance really must be individually walked before dedup can collapse
//! them) and bucketing instances into shared templates by a `(Definition
//! identity, structural-shape hash)` key, confirmed pairwise via
//! `Symbol::instance_body_has_same_type` before a hash-bucket hit is ever
//! accepted as a true dedup — hash-then-verify, never pointer identity
//! (ST-065).
//!
//! **Review correction (r2):** an earlier revision treated
//! `design.top_instances()`'s own elements as traversal *roots only* —
//! registering every descendant instance's body as a `TemplateTable` entry
//! but never a root's own body. That directly contradicted
//! `docs/design/state.md` §2's stated walk ("Template tables ... Built by
//! walking `design.top_instances()` → `Symbol::instance_body()` →
//! `Symbol::members()` recursively into child instances ..., collecting
//! every member with `sym.kind()` in `{Variable, Net, Port}`" — the very
//! first `members()` call in that description *is* on a top instance's own
//! body): any `Variable`/`Net`/`Port` declared directly inside a top module
//! (e.g. `module top; logic clk; leaf a(clk); endmodule`) was silently and
//! permanently dropped from the entire state subsystem — never captured in
//! any `StateLayout`, never reachable via `layout_of`/`slot_of`, with no
//! error and (before this revision) no test catching it. `Symbol::top_instances()` returns ordinary `Instance`-kind
//! `Symbol`s (see `sv_lang::Design::top_instances()`'s own doc example, which
//! calls `.instance_body()` on the returned item exactly like any other
//! `Instance` symbol reached via `members()`), so `build` below now feeds
//! each one through [`register_instance`](TemplateTable::register_instance)
//! directly — the same hash-then-verify registration, definition-index
//! bookkeeping, and `instance_order` bookkeeping as any other concrete
//! instance — rather than hand-unwrapping its body and skipping straight to
//! [`walk_children`](TemplateTable::walk_children). A top module with no
//! local `Variable`/`Net`/`Port` members of its own (the common case) still
//! gets a real, if zero-slot, template entry: this is not a special case,
//! it is exactly how an intermediate (non-top, non-leaf) module with no
//! local members already behaved before this fix, so top-level bodies are
//! now treated uniformly with every other instance body instead of being
//! the one silent exception. This does change `templates.len()`/
//! `instance_order` literal counts for a design with N top modules (each
//! top now contributes its own template-table entry and one `instance_order`
//! entry) — every test below that asserted a literal count has been updated
//! to account for the (now-registered) top-level template(s), and a new
//! test (`top_level_modules_own_members_get_a_state_layout`) proves a
//! top-level `Variable` is actually captured, not just that the counts
//! changed.

use super::layout::{LayoutError, build_template_layout};
use super::{SlotClass, StateLayout};
use crate::netlist::TemplateKey;
use std::collections::HashMap;
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
use sv_lang::kinds::SymbolKind;
use sv_lang::{Design, Symbol};

/// One entry per unique `(module,params)` template; `templates`' insertion
/// order is first-seen order during the deterministic DFS walk below —
/// ST-064's "identical offsets across runs" precondition falls out of that
/// determinism, not a separate sort.
pub struct TemplateTable<'d> {
    templates: Vec<(TemplateKey, StateLayout)>,
    /// The representative `InstanceBody` `Symbol` first registered under
    /// each `templates` entry's key, kept parallel to `templates` by index —
    /// the ground truth `instance_body_has_same_type` verifies every later
    /// hash-bucket hit against (ST-065: hash-then-verify, never pointer
    /// identity).
    representatives: Vec<Symbol<'d>>,
    /// One `TemplateKey` per concrete instance discovered, in the exact DFS
    /// order this walk produced them (ST-064's "walk order" query; consumed
    /// later by Task 7's per-instance base-assignment driver, which needs one
    /// entry per concrete instance, not one per unique template).
    instance_order: Vec<TemplateKey>,
}

impl<'d> TemplateTable<'d> {
    /// Walks every concrete instance reachable from `design.top_instances()`
    /// (ST-059), assigning each a [`TemplateKey`] via hash-then-verify dedup
    /// (ST-065) and recording the DFS visitation order (ST-064).
    pub fn build(design: &'d Design) -> Result<TemplateTable<'d>, LayoutError> {
        let mut table = TemplateTable {
            templates: Vec::new(),
            representatives: Vec::new(),
            instance_order: Vec::new(),
        };
        let mut definitions: Vec<Symbol<'d>> = Vec::new();
        let mut buckets: HashMap<TemplateKey, usize> = HashMap::new();

        for top in design.top_instances() {
            // Review correction (r2): `top` is itself an `Instance`-kind
            // `Symbol` (see `sv_lang::Design::top_instances()`'s own doc
            // example), so it is registered exactly like any other concrete
            // instance discovered via `walk_children` below — not merely
            // unwrapped for its body and skipped straight to walking
            // children. This is what makes a top module's own
            // `Variable`/`Net`/`Port` members (e.g. `module top; logic clk;
            // ...; endmodule`) get a `StateLayout`/`TemplateKey` per
            // `docs/design/state.md` §2's walk, instead of being silently
            // dropped. `register_instance` itself calls `walk_children` on
            // `top`'s body as its last step, so descendants are still
            // walked exactly as before.
            table.register_instance(top, &mut definitions, &mut buckets)?;
        }

        Ok(table)
    }

    /// Visits `body`'s `Instance`/`InstanceArray`/`GenerateBlock`/
    /// `GenerateBlockArray`-kind members (every other kind — `Variable`,
    /// `Net`, `Port`, `Genvar`, ... — is this walk's own business only
    /// through [`build_template_layout`], never re-walked here) and
    /// registers each concrete instance found, recursing through generate
    /// constructs since slang elaborates `if`/`case`/`for`-generate content
    /// as `GenerateBlock`/`GenerateBlockArray` symbols, not as
    /// `Instance`/`InstanceArray` ones.
    fn walk_children(
        &mut self,
        body: Symbol<'d>,
        definitions: &mut Vec<Symbol<'d>>,
        buckets: &mut HashMap<TemplateKey, usize>,
    ) -> Result<(), LayoutError> {
        for member in body.members() {
            match member.kind() {
                SymbolKind::Instance => self.register_instance(member, definitions, buckets)?,
                SymbolKind::InstanceArray => {
                    // ST-059: an array-of-instances element is a distinct
                    // concrete instance that can legitimately dedup
                    // against, or differ from, its array siblings — each
                    // one is registered individually, never as one shared
                    // entry for the whole array. A multi-dimensional array's
                    // elements can themselves be nested `InstanceArray`
                    // symbols (one level per extra dimension), so dispatch
                    // through `register_array_element` rather than assuming
                    // every element is a concrete `Instance`.
                    for elem in member.instance_array_elements() {
                        self.register_array_element(elem, definitions, buckets)?;
                    }
                }
                SymbolKind::GenerateBlock => {
                    // ST-059: the ordinary way an `if`/`case`-generate
                    // construct elaborates. `generate_block_is_uninstantiated`
                    // reports true for a branch that was NOT selected during
                    // elaboration (e.g. the `else` side of a taken `if`) —
                    // skip it so an untaken branch contributes nothing,
                    // rather than walking its (possibly incomplete) member
                    // list.
                    if !member.generate_block_is_uninstantiated() {
                        self.walk_children(member, definitions, buckets)?;
                    }
                }
                SymbolKind::GenerateBlockArray => {
                    // ST-059: what an ordinary (labeled or unlabeled)
                    // `generate for` loop of instances actually elaborates
                    // to — one `GenerateBlock` per loop iteration, each
                    // walked exactly like any other generate block.
                    for entry in member.generate_block_array_entries() {
                        if !entry.generate_block_is_uninstantiated() {
                            self.walk_children(entry, definitions, buckets)?;
                        }
                    }
                }
                _ => {}
            }
        }
        Ok(())
    }

    /// Dispatches one element of an `InstanceArray`'s `elements()` list: a
    /// concrete `Instance` is registered directly, and a nested
    /// `InstanceArray` (one extra dimension of a multi-dimensional array,
    /// e.g. `leaf arr[1:0][1:0]();`) is expanded recursively. A
    /// `PrimitiveInstance`/`CheckerInstance` element (slang's other two
    /// `InstanceArray`-element kinds) carries no module-shaped
    /// `InstanceBody` the way an `Instance` does, so — like
    /// [`build_template_layout`]'s own `Variable`/`Net`/`Port`-only member
    /// filter — it is intentionally out of this task's scope rather than an
    /// error.
    fn register_array_element(
        &mut self,
        elem: Symbol<'d>,
        definitions: &mut Vec<Symbol<'d>>,
        buckets: &mut HashMap<TemplateKey, usize>,
    ) -> Result<(), LayoutError> {
        match elem.kind() {
            SymbolKind::Instance => self.register_instance(elem, definitions, buckets)?,
            SymbolKind::InstanceArray => {
                for nested in elem.instance_array_elements() {
                    self.register_array_element(nested, definitions, buckets)?;
                }
            }
            _ => {}
        }
        Ok(())
    }

    /// Registers one concrete `Instance` symbol: builds its `StateLayout`,
    /// buckets it into a template via hash-then-verify dedup, records its
    /// `TemplateKey` in `instance_order`, and recurses into its children.
    /// Called both from [`walk_children`](Self::walk_children)'s
    /// `Instance`-kind dispatch for an ordinary descendant instance, and
    /// directly from [`build`](Self::build) for each of
    /// `design.top_instances()`'s own elements (review r2 fix) — a top
    /// instance is registered identically to any other, so its own
    /// `Variable`/`Net`/`Port` members are captured too, not just those of
    /// its descendants.
    fn register_instance(
        &mut self,
        inst: Symbol<'d>,
        definitions: &mut Vec<Symbol<'d>>,
        buckets: &mut HashMap<TemplateKey, usize>,
    ) -> Result<(), LayoutError> {
        let body = inst
            .instance_body()
            .expect("an Instance symbol always has a body");
        // ST-065: every concrete instance is individually walked/hashed —
        // `Compilation::compile()` forces `DisableInstanceCaching`, so slang
        // itself never pointer-shares two structurally-identical bodies;
        // this build (Task 4/6's own entry point) is the thing that makes a
        // hash bucketing possible at all.
        let layout = build_template_layout(&body)?;
        let definition = body
            .instance_body_definition()
            .expect("an InstanceBody symbol always has a Definition");
        let definition_index = match definitions.iter().position(|d| *d == definition) {
            Some(i) => i as u32,
            None => {
                definitions.push(definition);
                (definitions.len() - 1) as u32
            }
        };
        let mut key = TemplateKey {
            definition: definition_index,
            params_hash: structural_shape_hash(&layout),
        };

        // Hash-then-verify (ST-065): bucket by `key` first — a plain
        // HashMap lookup, never a call into slang — and only when that
        // lookup hits does this function call `instance_body_has_same_type`
        // to confirm the hit is a true structural match, not a hash
        // collision. A verify miss perturbs the hash and retries rather than
        // silently merging two different templates.
        let resolved_index = loop {
            match buckets.get(&key) {
                Some(&idx) if body.instance_body_has_same_type(&self.representatives[idx]) => {
                    break idx;
                }
                Some(_) => {
                    key.params_hash = key.params_hash.wrapping_add(0x9E37_79B9_7F4A_7C15);
                }
                None => {
                    let idx = self.templates.len();
                    self.templates.push((key, layout));
                    self.representatives.push(body);
                    buckets.insert(key, idx);
                    break idx;
                }
            }
        };

        let resolved_key = self.templates[resolved_index].0;
        self.instance_order.push(resolved_key);

        self.walk_children(body, definitions, buckets)
    }

    /// Template-relative layout lookup — never a per-instance storage offset
    /// (ST-066: sharing a template means sharing layout, never storage;
    /// per-instance base assignment is Task 7's job, not this module's).
    pub fn layout_of(&self, key: &TemplateKey) -> Option<&StateLayout> {
        self.templates
            .iter()
            .find(|(k, _)| k == key)
            .map(|(_, layout)| layout)
    }

    /// The exact DFS order concrete instances were discovered in (ST-064) —
    /// one entry per concrete instance, not one per unique template.
    pub fn walk_order(&self) -> impl Iterator<Item = &TemplateKey> {
        self.instance_order.iter()
    }
}

fn slot_class_discriminant(class: SlotClass) -> u8 {
    match class {
        SlotClass::Inline => 0,
        SlotClass::Wide => 1,
        SlotClass::Memory => 2,
        // Review fix (Task 14 r2, blocking finding 2): `force_driver`'s
        // synthetic contribution-cell class. The layout builder this
        // function's own callers key off of never produces it (see
        // `SlotClass::ForceWide`'s own doc comment), but the discriminant
        // must still be total over the enum.
        SlotClass::ForceWide => 3,
    }
}

/// A deterministic, order-sensitive stand-in for "resolved parameter values"
/// (ST-065's own text: the real resolved-parameter hash is the `netlist`
/// track's job once it exists; this hash only needs to be a deterministic
/// function of what this crate can already observe — the concrete,
/// parameter-substituted [`StateLayout`] this task builds anyway, which
/// already reflects a `logic [W-1:0]`-style width difference between two
/// differently-parameterized instances of the same module).
fn structural_shape_hash(layout: &StateLayout) -> u64 {
    let mut hasher = DefaultHasher::new();
    layout.slots.len().hash(&mut hasher);
    for slot in &layout.slots {
        slot_class_discriminant(slot.offset.class).hash(&mut hasher);
        slot.offset.bit_width.hash(&mut hasher);
        slot.offset.elem_word_count.hash(&mut hasher);
        slot.offset.row_stride_words.hash(&mut hasher);
        slot.offset.num_rows.hash(&mut hasher);
    }
    layout.inline_words.hash(&mut hasher);
    layout.wide_words.hash(&mut hasher);
    hasher.finish()
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::state::value::compile_source;

    /// ST-059/ST-065: two textually-identical instances of the same module
    /// must share exactly one template. `top` itself is now also registered
    /// as its own (zero-slot) template (review r2 fix), so the table holds 2
    /// entries total — `top`'s own and `leaf`'s shared one — but `a` and `b`
    /// must still resolve to the exact same `TemplateKey`. Fails if dedup
    /// incorrectly walks `a`/`b` as separate templates (3 total instead of
    /// 2), or if `top`'s own template collapses into `leaf`'s (2 keys equal
    /// when they must differ).
    #[test]
    fn two_textually_identical_instances_share_one_template() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n module top; leaf a(); leaf b(); endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own (zero-slot) template plus leaf's one shared template"
        );

        let keys: Vec<TemplateKey> = table.walk_order().copied().collect();
        assert_eq!(
            keys.len(),
            3,
            "one TemplateKey per concrete instance: top itself, a, b"
        );
        assert_eq!(
            keys[1], keys[2],
            "leaf a() and leaf b() must dedup into the same template"
        );
        assert_ne!(
            keys[0], keys[1],
            "top's own template must be distinct from leaf's"
        );
    }

    /// ST-065: two instances of the same module with different resolved
    /// parameters must NOT collapse into one template, even though the
    /// hash-then-verify path is the same code as the identical-instance
    /// case above. Fails if the dedup step (or a naive pointer-identity
    /// dedup) collapses these into one template despite
    /// `instance_body_has_same_type` correctly reporting them as different.
    #[test]
    fn differently_parameterized_instances_get_distinct_templates() {
        let design = compile_source(
            "module leaf #(parameter int W=1); logic [W-1:0] x; endmodule\n\
             module top; leaf #(.W(4)) a(); leaf #(.W(8)) b(); endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            3,
            "top's own zero-slot template plus two differently-parameterized leaf templates"
        );

        // `top`'s own template has zero slots (no local Variable/Net/Port),
        // so it is filtered out here rather than indexed into directly —
        // this is exactly what distinguishes it from the two leaf templates
        // being compared.
        let mut widths: Vec<u32> = table
            .templates
            .iter()
            .filter_map(|(_, layout)| layout.slots.first().map(|s| s.offset.bit_width))
            .collect();
        widths.sort_unstable();
        assert_eq!(
            widths,
            vec![4, 8],
            "the two templates' single slot must reflect W=4 and W=8 respectively"
        );
    }

    /// ST-064: building `TemplateTable` twice from the *same* `Design` must
    /// assign byte-identical `(TemplateKey, offset)` pairs both times — the
    /// literal bit-identical-determinism-across-runs precondition. Fails if
    /// traversal order (and therefore definition-index/hash assignment)
    /// ever depends on something not guaranteed stable, such as a
    /// `HashMap`'s iteration order, instead of `members()`'s
    /// guaranteed-stable declaration order.
    #[test]
    fn building_twice_from_the_same_design_assigns_identical_offsets() {
        let design = compile_source(
            "module leaf #(parameter int W=1); logic [W-1:0] x; endmodule\n\
             module top; leaf #(.W(4)) a(); leaf #(.W(8)) b(); leaf #(.W(4)) c(); endmodule\n",
        )
        .expect("valid SV source compiles");

        let first = TemplateTable::build(&design).expect("first dedup walk builds");
        let second = TemplateTable::build(&design).expect("second dedup walk builds");

        assert_eq!(first.templates.len(), second.templates.len());
        assert_eq!(first.instance_order, second.instance_order);

        for ((k1, l1), (k2, l2)) in first.templates.iter().zip(second.templates.iter()) {
            assert_eq!(k1, k2);
            assert_eq!(l1.slots.len(), l2.slots.len());
            assert_eq!(l1.inline_words, l2.inline_words);
            assert_eq!(l1.wide_words, l2.wide_words);
            for (s1, s2) in l1.slots.iter().zip(l2.slots.iter()) {
                assert_eq!(s1.offset.class, s2.offset.class);
                assert_eq!(s1.offset.word_offset, s2.offset.word_offset);
                assert_eq!(s1.offset.bit_width, s2.offset.bit_width);
            }
        }
    }

    /// ST-066: sharing a template means sharing layout, never storage.
    /// `leaf a(); leaf b();` share one `TemplateKey`/`StateLayout` (already
    /// proven above); this test additionally confirms `layout_of` hands back
    /// the exact same `&StateLayout` reference from both `a`'s and `b`'s
    /// perspective (`std::ptr::eq`), never two independently re-derived
    /// copies — proving by construction that per-instance base assignment
    /// (which is what would make storage distinct) is not this module's job
    /// at all (`layout_of`'s own signature returns only a template-relative
    /// `&StateLayout`, never a per-instance offset).
    #[test]
    fn sharing_a_template_means_sharing_layout_never_storage() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n module top; leaf a(); leaf b(); endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own (zero-slot) template plus leaf's one shared template"
        );

        let keys: Vec<TemplateKey> = table.walk_order().copied().collect();
        assert_eq!(
            keys.len(),
            3,
            "one TemplateKey per concrete instance (top itself, a, b)"
        );
        assert_eq!(
            keys[1], keys[2],
            "a and b must resolve to the same TemplateKey"
        );

        let from_a = table
            .layout_of(&keys[1])
            .expect("a's template has a layout");
        let from_b = table
            .layout_of(&keys[2])
            .expect("b's template has a layout");
        assert!(
            std::ptr::eq(from_a, from_b),
            "layout_of must hand back the exact same StateLayout, never a re-derived copy"
        );
    }

    /// ST-059: a labeled `generate for` loop of instances elaborates to
    /// `GenerateBlockArray`/`GenerateBlock` symbols, not `InstanceArray`.
    /// Fails (pre-fix) with `templates.len() == 0` and
    /// `instance_order.len() == 0` — the whole loop body silently vanishes.
    #[test]
    fn labeled_generate_for_loop_instances_are_discovered_and_deduped() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n\
             module top;\n\
               genvar i;\n\
               generate\n\
                 for (i = 0; i < 3; i = i + 1) begin : gf\n\
                   leaf u();\n\
                 end\n\
               endgenerate\n\
             endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own zero-slot template plus the one leaf template all 3 generate-for instances share"
        );
        assert_eq!(
            table.instance_order.len(),
            4,
            "one TemplateKey per concrete instance: top itself plus 3 generate-for instances"
        );
    }

    /// ST-059: the unlabeled `generate for` idiom (`for (...) begin ... end`
    /// with no `: label`) is the single most common instance-array idiom in
    /// real RTL; it elaborates to an anonymous `GenerateBlockArray`
    /// (`genblk1`) exactly like the labeled form. Fails (pre-fix) with 0
    /// instances discovered.
    #[test]
    fn unlabeled_generate_for_loop_instances_are_discovered() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n\
             module top;\n\
               genvar i;\n\
               generate\n\
                 for (i = 0; i < 3; i = i + 1) begin\n\
                   leaf u();\n\
                 end\n\
               endgenerate\n\
             endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own zero-slot template plus the one leaf template all 3 generate-for instances share"
        );
        assert_eq!(
            table.instance_order.len(),
            4,
            "one TemplateKey per concrete instance: top itself plus 3 generate-for instances"
        );
    }

    /// ST-059: an `if`-generate conditional instance must be discovered when
    /// its branch is taken, and the untaken branch (reported by
    /// `Symbol::generate_block_is_uninstantiated`) must contribute nothing —
    /// not a phantom instance, not an error. Fails (pre-fix) with 0
    /// instances discovered for the taken branch (the bug), or would fail if
    /// the untaken branch were walked anyway (walking an uninstantiated
    /// generate block is not guaranteed safe/empty).
    #[test]
    fn if_generate_conditional_instance_is_discovered_and_untaken_branch_contributes_nothing() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n\
             module top;\n\
               generate\n\
                 if (1) begin : g_taken\n\
                   leaf a();\n\
                 end else begin : g_untaken\n\
                   leaf b();\n\
                 end\n\
               endgenerate\n\
             endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own zero-slot template plus the taken if-generate branch's leaf template"
        );
        assert_eq!(
            table.instance_order.len(),
            2,
            "top itself plus the one taken-branch instance; the untaken else-generate branch must contribute zero"
        );
    }

    /// A multi-dimensional `InstanceArray`'s elements are themselves nested
    /// `InstanceArray` symbols for every dimension but the innermost — only
    /// the final level is actual `Instance` symbols. Fails (pre-fix) with a
    /// panic ("an Instance symbol always has a body") because
    /// `register_instance` was called directly on an outer-dimension
    /// `InstanceArray` element instead of recursing into it.
    #[test]
    fn multi_dimensional_instance_array_elements_are_discovered_without_panicking() {
        let design = compile_source(
            "module leaf; logic x; endmodule\n\
             module top;\n\
               leaf arr[1:0][1:0]();\n\
             endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");
        assert_eq!(
            table.templates.len(),
            2,
            "top's own zero-slot template plus the one template all 4 array elements dedup into"
        );
        assert_eq!(
            table.instance_order.len(),
            5,
            "top itself plus one TemplateKey per concrete array element (2x2 = 4)"
        );
    }

    /// Review r2 fix: a top-level module's own `Variable`/`Net`/`Port`
    /// members must be captured in a `StateLayout`, exactly like any other
    /// instance body's members — `docs/design/state.md` §2's walk explicitly
    /// starts with `design.top_instances()` → `instance_body()` →
    /// `members()`, and that first `members()` call is on `top`'s own body.
    /// Before the fix, `top` was only ever a traversal root (never itself
    /// registered via `register_instance`/`build_template_layout`), so
    /// `clk` here would be silently and permanently dropped: not present in
    /// any `StateLayout`, not reachable via `layout_of`, and
    /// `table.walk_order()` would report only 1 concrete instance (`a`)
    /// instead of 2 (`top`, `a`). Fails (pre-fix) on the `keys.len() == 2`
    /// assertion alone.
    #[test]
    fn top_level_modules_own_members_get_a_state_layout() {
        let design = compile_source(
            "module leaf; logic y; endmodule\n\
             module top; logic clk; leaf a(); endmodule\n",
        )
        .expect("valid SV source compiles");

        let table = TemplateTable::build(&design).expect("dedup walk builds");

        let keys: Vec<TemplateKey> = table.walk_order().copied().collect();
        assert_eq!(
            keys.len(),
            2,
            "one TemplateKey per concrete instance: top itself, a"
        );
        assert_ne!(
            keys[0], keys[1],
            "top's own template (with `clk`) must differ from leaf's (with `y`)"
        );

        let top_layout = table
            .layout_of(&keys[0])
            .expect("top's own body must have a registered StateLayout");
        assert_eq!(
            top_layout.slots.len(),
            1,
            "top's own `clk` variable must be captured as a slot, not silently dropped"
        );

        let leaf_layout = table
            .layout_of(&keys[1])
            .expect("leaf's body must have a registered StateLayout");
        assert_eq!(
            leaf_layout.slots.len(),
            1,
            "leaf's own `y` variable must still be captured as a slot"
        );
    }
}