FSM extractor: implicit self-loops for canonical kernel pattern

[twitzelbos]

Summary

• When both walkers run cleanly but find no state-overriding op for a match arm, interpret it as a self-loop on the source variant rather than Unanalyzable. This is the correct semantic given CLAUDE.md §3's canonical pattern: kernels write d.<state_field> = q.<state_field> once at the kernel top and only override in arms that transition.
• The convention applies at the d-struct walker's union points (Select branches, Case arms) plus the top-level fallback. Leaf paths keep Ok(vec![]) so the value-form walker still wins on state-typed slots — the let-binding form is unaffected.
• Unanalyzable is now reserved exclusively for genuinely malformed IR (Enum opcode whose discriminant matches no variant, etc.), pinned by an inverted negative test.

Justification (per CLAUDE.md §11.1)

1. What guarantee does this preserve, strengthen, or introduce?
Strengthens Layer 2 acceptance criterion #2 (fsm-architecture.md §5.4): "zero false positives on the existing widget corpus." PR #6 self-tested only against synthetic widgets in doc.rs — none used the canonical kernel-top default + selective override pattern. First validation against the real core::can_master showed it produced 4 spurious Unanalyzable diagnostics on guarded-transition arms whose else-branches only update auxiliary state (a bit counter). Every production protocol-PHY kernel (CAN, I²C, SPI, UART RX, DHT22, etc.) shares this shape; this PR makes the auto-extractor usable on real widgets.

2. What loophole does this not introduce?
A naive "no state-write found = self-loop" rule applied at every leaf would pollute the value-form walker (which is also called on Enum slots), interpreting a Tier-3 widget that mistakenly used Add on a state slot as "self-loop on source" instead of surfacing the type-system violation. The implementation restricts the convention to union points (where the d-struct context is unambiguous) plus the top-level fallback. The leaf paths (find_definer-None, unrecognised opcode, Struct-without-state-field) still return empty so the value walker's interpretation can win at the top. The negative test arm_with_unmatched_enum_discriminant_yields_unanalyzable pins this — a genuinely malformed enum still produces Unanalyzable with a discriminant-mentioning diagnostic.

3. What downstream code does this affect, and why is the effect intentional?
No widget HDL snapshots changed (verified by cargo test --package rhdl-fpga --lib — 442 tests passing, 0 snapshot diffs). The change is purely additive in the FSM extractor: no IR opcode added, no lowering modified, no Verilog emitted differently. Three pre-existing fsm::extraction::tests were reframed (rename + assertion update) because they were testing the old incorrect behaviour — keeping them as-is would have masked the can_master regression. The reframed tests now assert the correct (new) semantics with comments explaining the semantic shift.

4. What is the alternative design considered and rejected?

• Rewrite every widget to make self-loops explicit (d.field = q.field inside each else-branch). Mechanically tractable across 55 widgets but adds boilerplate that the kernel-top default pattern was specifically designed to avoid — and contradicts the canonical idiom CLAUDE.md §3 documents.
• Declare can_master "not an FSM" and remove #[derive(FsmWidget)]. Contradicts the entire FSM track and §5.4 acceptance criteria.
• Verify the kernel-top default exists before applying the implicit-self-loop interpretation. Cleaner guarantee but adds a Layer 2 prerequisite check that's not strictly necessary — the canonical pattern is universal so far. Tracked as an optional follow-up.

5. Is this change reversible?
Yes — purely a semantic interpretation in the extractor. No IR changes, no on-disk artefacts, no public API additions. Reverting the extractor change would re-introduce the can_master Unanalyzable diagnostics but break nothing else.

Test coverage

Pass-level (crates/rhdl-core/src/fsm/extraction.rs):

• 4 new synthetic-RHIF unit tests:
  • kernel_top_default_plus_guarded_transition_yields_both_edges — explicit then-branch + implicit else-branch produce both edges.
  • guarded_transition_with_implicit_else_yields_self_loop — the can_master Id arm verbatim: else-branch writes a different field (other_field), not state.
  • arm_with_no_state_write_at_all_yields_self_loop — bare arm with no state Splice anywhere in the chain.
  • arm_with_unmatched_enum_discriminant_yields_unanalyzable — negative test: genuinely malformed IR still produces Unanalyzable.
• 3 pre-existing tests reframed for the new semantics:
  • arm_with_unanalyzable_target_is_flagged → arm_with_no_recognisable_target_yields_implicit_self_loop.
  • opaque_arm_result_yields_unanalyzable_diagnostic → opaque_arm_result_yields_implicit_self_loop.
  • struct_opcode_without_state_field_is_unanalyzable → struct_opcode_without_state_field_yields_implicit_self_loop.

Kernel-level integration (crates/rhdl-fpga/src/doc.rs):

• 2 new adversarial widgets compiling through the full pipeline (Stage 1 → RHIF → extractor):
  • adv_can_master_guarded_else_writes_other_field — 3-state FSM with kernel-top default, guarded transitions, else-branches updating only d.bit_idx. Faithful synthetic stand-in for the can_master Id/Eof arms.
  • adv_nested_conditional_implicit_self_loops — nested if-else inside one arm where two paths independently omit d.state; verifies dedup at union points.

Validation

• 65 fsm:: tests pass (was 61, +4 new + 3 reframed).
• 442 rhdl-fpga lib tests pass — no widget HDL snapshot regression.
• 120 rhdl-core lib tests pass.
• CHANGELOG entry added per §16.
• fsm-architecture.md §5.6 status note added.

Test plan

• cargo test --package rhdl-core fsm:: — 65 passing
• cargo test --package rhdl-fpga --lib — 442 passing, 0 snapshot diffs
• cargo test --package rhdl-core --lib — 120 passing
• CHANGELOG + fsm-architecture.md §5.6 updated
• Reviewer verifies the Justification section's "what loophole does this not introduce" reasoning by reading extraction.rs variants_in_d_state_field and confirming the Select/Case empty-branch self-loop conversion does NOT propagate through Enum/Add/Struct-without-field leaves
• Reviewer confirms arm_with_unmatched_enum_discriminant_yields_unanalyzable still trips on genuinely-malformed IR (try changing the discriminant value to a valid one and confirm the test fails as expected)

🤖 Generated with Claude Code

[twitzelbos]

Wrong repo, sorry