1
0
Fork 0
mirror of https://github.com/imjasonh/nescript synced 2026-07-08 08:55:38 +00:00
Commit graph

197 commits

Author SHA1 Message Date
Claude
ee3dddb19e
examples: add feature_canary that turns red on any memory silent-drop regression
Phase 5 of the post-PR-#31 audit, and the structural piece that
closes the failure mode the earlier phases couldn't fix alone.

The audit's recurring diagnosis: pixel/audio goldens capture
*whatever* the program does, not what it *should* do. A silent
drop in codegen is still deterministic — the golden locks in
the broken behaviour and every future run agrees with it. That's
how state-locals, uninitialized struct-field writes, `on exit`
handlers, and `slow` placement each sat broken for months-to-a-
year in a green CI.

The canary inverts the relationship: the committed golden is a
solid-green universal backdrop that only appears when every
round-trip check passes. Each check writes a distinctive constant
through one language construct, reads it back, and clears
`all_ok` on mismatch. A final `if all_ok == 0 { set_palette Fail }`
flips the entire screen red for the rest of the run.

Checks cover the silent-drop shapes caught by this audit:
  - state-local variable write-read (PR #31)
  - uninitialized struct-field write-read (caught by phase 1)
  - u8 / u16 globals (u16 exercises both StoreVar + StoreVarHi)
  - array-element write at nonzero index
  - `slow`-placed global still round-trips
  - function call return value

The canary doesn't use `debug.assert` on purpose — debug-only
ops get stripped in release and the emulator harness runs
release builds. The palette swap works in release and is what
the harness pixel-diff sees.

### Why this matters as a long-lived test

The harness already had 34 pixel goldens covering full-program
behaviour, but none of them exist specifically to fail if a
*specific language feature* silently drops. The canary does.
Every silent-drop bug the audit found would have flipped it
red the moment the check was added, which is the "behaviour
assertion that can't be satisfied by silence" the plan called
for.

### Harness footprint

`tests/emulator/goldens/feature_canary.{png,audio.hash}` +
`examples/feature_canary.{ne,nes}`. 35/35 ROMs match their
goldens with the canary added. Listed in both README tables.

https://claude.ai/code/session_01AoQ678uVeqpyayvWHpfDhC
2026-04-18 00:14:40 +00:00
Claude
00f1305564
codegen+CLAUDE: harden Call arity and document the silent-drop review checklist
Phase 3/4 of the post-PR-#31 audit.

### Call args > 4 is now an assert

`IrOp::Call` silently `.take(4)`-d the arg list with a comment
claiming the analyzer's E0506 check made the extras unreachable.
Replace with an explicit `assert!(args.len() <= 4, ...)` so if
the analyzer ever regresses, the codegen crashes loudly instead
of miscompiling the call. Iterate over all args (not just the
first 4) since the assert guarantees correctness.

### CLAUDE.md: new-feature PR checklist

Document the lesson the audit taught: every new language-feature
PR must include (1) an example exercising it, (2) a runtime
*behaviour* assertion (not just a "ROM validates" shape check),
(3) a negative test for invalid use. Call out the specific
address-map lookup pattern (`if let Some(&addr) = map.get(..)`
with no else) that shipped the state-local bug, and recommend
the `IrCodeGen::var_addr` / explicit `.unwrap_or_else(|| panic!)`
idiom instead.

Chose not to add a regex-based CI tripwire for "silently" /
"for now" comments because the false-positive rate against
legitimate design decisions ("silently truncate to 8 bits per
the cast spec", etc.) would train contributors to ignore it.
The durable checklist in CLAUDE.md is what next agents need.

https://claude.ai/code/session_01AoQ678uVeqpyayvWHpfDhC
2026-04-18 00:09:34 +00:00
Claude
48bae97c51
analyzer+codegen: turn silently-dropped feature paths into hard failures (or fix them)
Phase 2 of the post-PR-#31 audit. The codebase had four documented
"silently skip" paths that parsed user intent but produced no code.
Each one was the same shape as the state-local bug: the analyzer
accepted the program, the IR lowered the construct, but somewhere
downstream the emitted code was dropped on the floor — and a pixel
golden that captured the broken behaviour locked it in as correct.

Fix each per the plan, either by implementing the feature or
rejecting the program at the analyzer.

### on_exit handlers now actually run

`IrOp::Transition` used to comment "on_exit of the current state
isn't called here because we don't know from an IR op alone which
state we're leaving." The codegen emitted the exit handler's body
as an IR function but never JSR'd it. Three example programs
(pong, war, state_machine) relied on `stop_music` or mode-flag
translation inside `on exit` that had been silently never running.

Emit a small CMP-chain against `ZP_CURRENT_STATE` before each
transition: for every state that declares an on_exit, compare the
current index, branch past on miss, JSR the exit handler on match,
then JMP to the shared done-label so only the leaving state's
handler fires. The chain is inlined at each transition site
(bounded by the number of states declaring on_exit) rather than
factored into a single trampoline — simpler to reason about, and
transitions are rare enough that the extra bytes don't matter.

Pong / war / state_machine ROMs change because the dispatch code
is now emitted. Video goldens stay byte-identical (no transitions
happen within the 180-frame harness window under no-input). Pong
and war audio hashes shifted from pure code-layout timing and are
regenerated. `docs/pong.gif` and `docs/war.gif` are byte-identical.

### State-local array initializers now refuse to compile (E0601)

`src/ir/lowering.rs:887` had the comment "Array initializers for
state-locals aren't supported yet... Programs that try this should
get a diagnostic from the analyzer; for now, silently skip." The
analyzer never actually emitted that diagnostic. Verified by
compiling `state Main { var buf: u8[4] = [10,20,30,40] ... }`:
the program built a valid ROM with no trace of 10/20/30/40 in PRG.

Add E0601 to the analyzer's state-local pass. The IR lowerer's
defensive `continue` stays in place as a belt-and-braces guard.

### `on scanline` without MMC3 is now E0603

Previously E0203 ("invalid operation for type") which is a
miscategorisation — the feature is unsupported on the current
mapper, not a type error. Dedicated E0603 makes the future-work
shape explicit.

### `slow` variables now actually live outside zero page

`Placement::Slow` was parsed into the AST but `allocate_ram`
ignored it, so `slow var cold: u8` still landed in ZP like any
other u8. Wire `var.placement` through `allocate_ram_with_placement`
and skip the ZP branch when `Slow` is set. `Fast` remains
advisory (the existing default already prefers ZP for u8 vars),
validated by W0107.

### Other address-map silent drops hardened

Alongside the var_addrs hardening from phase 1, three `state_indices`
lookup sites that did `.copied().unwrap_or(0)` or silent `if let`
are now explicit panics: scanline IRQ dispatch, MMC3 reload, and
`IrOp::Transition`. A miss in any of them is a compiler bug, not
valid input — the analyzer catches unknown state names upstream.

### Regression guards

Four new tests would have failed against the old silently-dropping
code paths:

- `analyze_state_local_array_initializer_rejected` — expects E0601.
- `analyze_on_exit_declaration_accepted` — expects no errors.
- `analyze_slow_var_forced_out_of_zero_page` — expects alloc
  address >= $0100.
- `transition_dispatches_leaving_states_on_exit_handler` — counts
  distinct JSR targets in the PRG before/after adding `on exit` to
  a state; the exit-bearing build must have more.

All 720 tests pass. All 34 emulator goldens pass after the pong/war
audio hash refresh.

https://claude.ai/code/session_01AoQ678uVeqpyayvWHpfDhC
2026-04-18 00:06:39 +00:00
Claude
f7012c6533
codegen: make var_addrs misses panic loudly and fix latent struct-field silent drop
Phase 1 of the post-PR-#31 audit. The PR #31 state-local bug had a
specific shape: analyzer allocated a slot, codegen looked it up by
VarId, silently emitted nothing on miss. Six sites in gen_op plus
the global-initializer loop and the parameter-shuffle prologue all
used the same `if let Some(&addr) = self.var_addrs.get(var) { ... }`
pattern with no else branch. Any future allocation-map desync would
slip through the same crack.

Replace every site with a new `IrCodeGen::var_addr(VarId) -> u16`
helper that panics with an explicit "compiler bug" message on miss.
An IR op referencing an unmapped VarId is not valid input — it means
the analyzer and lowerer disagreed on what to allocate, and we want
that crash to surface in CI rather than be absorbed by whatever
zero-filled RAM happened to sit at the read.

Running cargo test against the hardened lookup surfaced exactly the
bug shape the plan predicted: uninitialized struct globals (e.g.
`var p: Point` with no literal initializer) never had their flattened
field VarIds (`"p.x"`, `"p.y"`) registered in var_addrs. The IR
lowerer's `get_or_create_var("p.x")` minted a VarId, the analyzer's
`flatten_struct_fields` allocated an address for it, but IrCodeGen::new
only populated var_addrs from `ir.globals`, which doesn't contain
synthesized field entries for uninitialized structs. Every `p.x = N`
silently compiled to nothing.

Fix by exposing the IR lowerer's name→VarId map on IrProgram and
joining it with the analyzer's allocations in IrCodeGen::new. Every
allocated name that the lowerer knows about now gets a var_addrs
entry. Example ROMs are byte-identical (no example relied on the
dropped writes), but the bug was reachable — any user program with
a plain `var pos: Point` declaration and field writes would have hit
it silently.

Add `uninitialized_struct_field_store_emits_sta_to_allocated_address`
as a byte-level regression guard: compile `p.x = 123` and scan PRG
for `LDA #\$7B / STA <addr>`. Fails against the old silently-dropping
codegen.

https://claude.ai/code/session_01AoQ678uVeqpyayvWHpfDhC
2026-04-17 23:49:29 +00:00
431f144be7
Merge pull request #31 from imjasonh/claude/resolve-issue-22-0YxNw
analyzer+ir: fix state-local variables and overlay them across mutually-exclusive states
2026-04-17 09:00:07 -04:00
Claude
40dec7907a
examples: move state-scoped globals to state-local in coin_cavern + platformer
Both examples declared their gameplay variables at the top level
even though every read and write happened inside one specific state.
That pattern hid the overlay feature from new users and kept the
state-local code path from being exercised outside the dedicated
`state_machine.ne` demo (which is how the "state-locals silently
drop their writes" bug survived so long).

`coin_cavern.ne`: the five Playing-only physics/position/inventory
vars (`player_x`, `player_y`, `player_vy`, `on_ground`,
`coins_left`) move onto Playing's state block. `score` stays
global because GameOver-era code could reasonably grow to read it.
The `on_enter` body loses its redundant resets — the declared
initializers on the state-locals re-run on every entry, so
retrying after `transition Title` comes back to a fresh state.

`platformer.ne`: player physics, camera, liveness, animation
phase, and the autopilot budget (`player_y`, `on_ground`,
`rise_count`, `fall_vy`, `camera_x`, `anim_tick`, `alive`,
`auto_jumps`) all move onto Playing. `frame_tick` and
`stomp_count` stay global — Title reads the former to
auto-advance, GameOver reads the latter to tally coins on the
death screen. The analyzer now overlays Title's `blink`,
Playing's eight physics vars, and GameOver's `linger` starting
at the same ZP byte (`$1A`), so the three scenes share a
9-byte window instead of each claiming their own slots.

Byte-level ROM bytes for both examples shift because variable
addresses moved. Video goldens stay pixel-identical (the harness
doesn't see Playing in coin_cavern, and the pre-transition
Title→Playing timing in platformer is preserved); the platformer
audio hash needed one more refresh because the now-slightly-shorter
reset prologue shifts APU writes within each frame.

https://claude.ai/code/session_015kvJu3iEFLSRJoShPBfm3X
2026-04-17 11:58:02 +00:00
Claude
73dcf08c7a
analyzer+ir: automatically overlay state-local variables
Before this change, state-local variables (`state Foo { var x: u8 = 0 }`)
were silently no-ops: the analyzer allocated a ZP slot for them, but
the codegen's `var_addrs` map only covered IR globals and scope-qualified
function locals — so every `LoadVar` / `StoreVar` whose `VarId` pointed
at a state-local resolved to no address and emitted nothing. Existing
examples compiled and matched their goldens because none of them observed
the dropped writes within the 180-frame harness window.

The overlay changes the analyzer's state-local pass to snapshot both the
ZP and RAM cursors after the globals have been laid out, then rewind to
that snapshot before each state's locals and track the running max.
`ZP_CURRENT_STATE` keeps exactly one state active at runtime, so every
state's locals are mutually exclusive with every other state's and can
share the same bytes. The IR lowerer now pushes each state's locals into
the IR globals table (with `init_value=None`) so the codegen resolves
their addresses the same way it does program globals, and prepends the
declared initializers to each state's `on_enter` handler (synthesizing
an empty one where needed) so a freshly-entered state re-establishes its
bytes before user code runs.

`--memory-map` now tags each allocation with its owning state
(`[@Title]`, `[@Playing]`, ...) and counts distinct bytes rather than
summed allocation sizes so overlaid slots don't double-count. The
`AnalysisResult.state_local_owners` map exposes the ownership to any
tool that wants to group allocations the same way.

Only `state_machine.ne` and `platformer.ne` declare state-level vars,
so they're the only example ROMs whose bytes change. `platformer.ne`'s
audio golden shifts slightly (the now-working `blink` counter in Title
adds a few cycles per frame before the auto-transition to Playing, which
offsets APU register writes within each frame); its video golden and
every other example ROM stay byte-for-byte identical.

Fixes #22.

https://claude.ai/code/session_015kvJu3iEFLSRJoShPBfm3X
2026-04-17 02:20:07 +00:00
77d55bc16b
Merge pull request #30 from imjasonh/claude/resolve-issue-23-iuBod 2026-04-16 21:44:16 -04:00
Claude
a5f508a678
linker+ci: fix .dbg seg.ooffs to include iNES header + deepen probe
The `seg.ooffs` field in our ca65 .dbg output was off by 16 — it was
emitting the PRG-relative fixed-bank offset when ca65's convention
(and Mesen's DbgImporter.cs:301 math:
`Address = val - seg.start + ooffs - headerSize`) expects the raw
output-file offset, *including* the iNES header. The practical
consequence: every label Mesen resolved via the .dbg was 16 bytes
short of its true PRG offset, which silently corrupted source-line
mapping for the first bytes of each function.

Fix is a one-liner — drop the `saturating_sub(16)` and feed
`linked.fixed_bank_file_offset` straight into the ooffs field. Unit
tests in debug_symbols.rs updated to assert the new values (ooffs=16
for NROM, 16+16K*N for banked).

The Mesen probe (`tests/mesen/probe.lua`) is expanded in the same
change, because the sabotage test that caught this bug is also the
cleanest demonstration the probe is working:
 * checks all four entry-point labels resolve and land inside the
   fixed bank's CPU window ($C000-$FFFF);
 * asserts the linker's relative ordering (main_loop < Main_frame
   < nmi);
 * registers a startFrame callback, waits three frames, and verifies
   PC is still in the fixed bank + that `emu.read(main_loop.address,
   nesPrgRom)` returns 0xA5 (the LDA-zp opcode the runtime always
   places as main_loop's first instruction). The 0xA5 constant is
   what catches the ooffs regression — a less-specific "not 0xFF"
   check coincidentally passed even with ooffs=0 because the shifted
   address still landed on real code.

Verified locally by running the probe against hello_sprite's ROM
with four different `.dbg` mutations and confirming each triggers
the expected exit code.

https://claude.ai/code/session_01DfN3pKJLryr7vvNFBpcqmC
2026-04-17 01:27:50 +00:00
Claude
90e01bd197
ci: fold mesen-dbg job into the main CI workflow
Move the Mesen2 .dbg validation job from its own workflow file into
the existing ci.yml so all CI lives in one place. Same job content
and same workarounds (settings.json shim, GLOBALIZATION_INVARIANT,
xvfb-run) — only the file location changed. The MESEN_VERSION env
var moves up to the workflow-level env block.

https://claude.ai/code/session_01DfN3pKJLryr7vvNFBpcqmC
2026-04-17 01:10:24 +00:00
Claude
d075bc2c43
ci: add headless Mesen2 .dbg validation workflow
Run Mesen2's `--testRunner` mode in CI, point it at a NEScript-built
ROM + auto-loaded .dbg, and assert via Lua that the four entry-point
labels (`nmi`, `irq`, `Main_frame`, `main_loop`) we promised to emit
actually resolve. Failures encode which assertion broke into the exit
code so CI can report it without stdout (Mesen's emu.log is internal).

The setup needed three workarounds, each documented inline in the
workflow:
  * `touch settings.json` to skip Mesen's first-run GUI wizard, which
    blocks the --testRunner dispatch in Program.cs:74. Contents don't
    matter — Configuration.Deserialize falls back to defaults on parse
    error.
  * `DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1` to keep .NET from loading
    system libstdc++ via libicuuc. On Ubuntu 24.04 the dual libstdc++
    presence (system + MesenCore.so's bundled static copy) crashes
    MesenCore's static regex initialiser with std::bad_cast before any
    user code runs.
  * `xvfb-run` because Mesen's Avalonia UI calls XOpenDisplay before
    --testRunner is dispatched.

This is a separate workflow file from ci.yml because it depends on the
40 MB Mesen2 binary download + xvfb + sdl2, none of which the existing
jobs need. Cached by Mesen version so reruns are fast.

https://claude.ai/code/session_01DfN3pKJLryr7vvNFBpcqmC
2026-04-17 01:03:23 +00:00
Claude
e4751df143
linker: add ca65-compatible --dbg output for source-level debugging
Emit a `.dbg` debug-info file in the same format `ld65` produces, so
Mesen / Mesen2 / fceuX pick it up automatically and enable source-line
stepping, labelled variable inspection, and symbol-based breakpoints
without manual address lookups. Closes #23.

The new `render_dbg` helper stitches together metadata the compiler
already surfaces (linker label table, IR codegen `__src_<N>` markers,
analyzer variable allocations) into the file/mod/seg/scope/span/line/sym
records documented at https://cc65.github.io/doc/debugfile.html. Each
source-loc marker becomes a span that stretches to the next marker
(so breakpoints cover every byte the statement compiled into) plus a
line record pointing into it; `seg.ooffs` tracks the fixed bank's
PRG-relative start so banked MMC1/UxROM/MMC3 ROMs map cleanly too.

Reuses the `.mlb` symbol-name filter so internal skip/block labels
stay out of the debugger's symbol browser. `--dbg` implies the same
`__src_` marker emission as `--source-map` but leaves release builds
byte-identical when neither flag is passed.

https://claude.ai/code/session_01DfN3pKJLryr7vvNFBpcqmC
2026-04-16 22:39:08 +00:00
f9198ac52c
Merge pull request #29 from imjasonh/claude/analyze-minimal-rom-YawXr 2026-04-16 18:13:21 -04:00
Claude
c09f9c0caa
codegen: emit gate markers at end of generate() to protect peephole
Move the six gate-marker label emissions (__mul_used, __div_used,
__oam_used, __default_sprite_used, __p1_input_used, __p2_input_used)
out of the inline IR-op lowering paths and into a new
`emit_trailing_markers()` helper that runs once at the end of
`generate()`. The IR walk now just flips a bool per marker; the
label emit happens after every instruction has been lowered, so
the marker never lands in the middle of a peephole-sensitive
sequence.

Fixes a real peephole interaction that surfaced after rebasing on
main's `codegen: skip parameter-spill prologue for leaf functions`
+ `peephole: drop dead LDA #imm before mem-INC/DEC + JMP`
improvements: an inline `__oam_used:` label inside `IrOp::DrawSprite`
split the dead-load-elimination block, leaving the `STA $130 /
LDA $130` redundant store+load pair that main's peephole would
otherwise have collapsed to a plain `LDA #imm`. The stale bytes
shifted the NMI handler by a few bytes, which shifted `on frame`
execution enough that `examples/palette_and_background.ne` captured
phase 1 (WarmReds) at frame 180 instead of phase 2 (CoolBlues).

Regenerates every example ROM against the new codegen (all gate
behaviour is unchanged — the linker still sees the same markers,
just at the tail of the user stream instead of interleaved) and
updates the goldens that shifted: seven audio-hash drifts (all
audio-bearing programs, same cycle-accurate-APU-timing story as
every prior NMI layout change) and two pixel goldens — the one-
pixel sprite-position drift in `comparisons.png` that we already
tolerate, plus the phase-capture flip in
`palette_and_background.png`.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:31:47 +00:00
Claude
e5bc325a71
linker+runtime: code review cleanup of the seven gates
* Tighten the OAM-DMA gate: the `has_oam` flag now ORs
   `__sprite_cycle_used` in addition to `__oam_used`. A hypothetical
   program that calls `cycle_sprites` without ever drawing would
   otherwise compile to an NMI that advances \$07EF each frame but
   never actually runs the DMA the cycling is meant to perturb.
   The stronger gate keeps the two markers semantically coupled
   (cycling presupposes DMA) and adds a test that verifies the
   DMA trigger is emitted for a cycle-only program.

 * Drop the unused `NmiOptions::any_input()` helper. The only
   consumer (`gen_nmi`) reads the two flags inline and I never
   wired up a second caller.

 * Fix the cycle-count claim in `NmiOptions::has_p2_input` /
   `has_p1_input` docstrings: LDA abs + LSR A + ROL zp is 11
   cycles per port, ×8 loop iterations = ~88 cycles, not the "~30"
   I wrote in the original commit. Also notes the ~12-cycle strobe
   + scaffold overhead that disappears when both ports are unused.

No behavioural change — all 622 Rust tests and 33 emulator
goldens still pass unchanged.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:09 +00:00
Claude
53c454669d
runtime: gate controller-1 reads, skip whole input block when unused
With `has_p1_input` false, drop the three-instruction JOY1 shift
block from the NMI's input loop. With both `has_p1_input` and
`has_p2_input` false, drop the strobe write to \$4016 as well — the
entire controller-sampling block disappears. Audio- or compute-only
programs that never touch `button.*` pay zero cycles for input
sampling.

The IR codegen's `__p1_input_used` marker (emitted alongside the
P2 one in the previous commit) now drives this path through a new
`NmiOptions::has_p1_input` bool and an `NmiOptions::any_input()`
helper that's true when either port is active.

Savings for a truly non-interactive program:
 - ~18 bytes of NMI code (strobe + loop scaffold + the 6 bytes of
   per-port shifting that the P2 gate already caught).
 - ~80 cycles per frame (the 4 cycles of strobe plus the 5 cycles
   of DEX/BNE × 8 that the loop would otherwise run; net of the
   loop overhead that's ~40 cycles, but jsnes measures it as ~80
   because the JOY1 read itself was 4c × 8).

Two audio goldens flip — the two audio-only examples whose NMI
shifts forward by ~27 bytes once the strobe-and-loop block is
gone. Same cycle-accurate-APU-timing drift as every prior NMI
layout change.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:09 +00:00
Claude
0de1d60c33
runtime: gate controller-2 reads in NMI on __p2_input_used
Drop the three-instruction JOY2 shift block (`LDA $4017 / LSR A /
ROL ZP_INPUT_P2`) from inside the NMI's 8-iteration input loop
when user code never reads controller 2. IR codegen emits the
`__p2_input_used` marker from `IrOp::ReadInput(_, 1)`; the linker
threads the flag through a new `NmiOptions::has_p2_input` bool,
and `gen_nmi` writes the shift block only when the flag is set.

Savings for single-player programs:
 - ~6 bytes of NMI code.
 - ~30 cycles per frame (3 instructions × 8 loop iterations, each
   6-8 cycles depending on addressing — LDA abs is 4, LSR A is 2,
   ROL zp is 5, so ~11 cycles × 8 = ~88 cycles; rounded down for
   the page-crossing penalty landing differently in the new layout).

This commit also fixes the IR codegen to drop the matching
`__p1_input_used` marker from `IrOp::ReadInput(_, 0)`, even though
the next commit is the one that actually consumes it. Landing the
two markers together keeps the IR codegen's per-op bookkeeping
coherent.

Six audio goldens flip (every program that reads input + plays
audio) with the expected NMI-layout-shift cycle drift.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:09 +00:00
Claude
bd30ac3010
runtime: gate OAM DMA and OAM shadow init on __oam_used
Skip the OAM DMA (LDA#0/STA \$2003 + LDA#2/STA \$4014) inside the
NMI handler and the `\$FE` hide-sentinel fill of the \$0200 OAM
shadow inside `gen_init` for programs that never `draw`. Both are
gated on the `__oam_used` marker the IR codegen now drops at the
first `IrOp::DrawSprite`.

Savings per NMI for a non-drawing program:
 - ~520 cycles (the DMA is 513 cycles plus the 4 register writes),
 - ~9 bytes of NMI code,
 - ~4 bytes of init code (the \$FE swap is replaced by a plain
   zero-fill of \$0200-\$02FF alongside the rest of the 2 KB RAM
   clear).

Plumbed by:
 - New `NmiOptions::has_oam: bool`, threaded through `gen_nmi`.
 - `gen_init(has_oam: bool)` parameter controlling the inner-loop
   OAM fill. Existing runtime tests all migrate to `gen_init(true)`
   to preserve their legacy assertions.
 - Linker computes `has_oam = has_label(user_code, "__oam_used")`
   once and feeds it to both call sites, and the existing
   `has_visual_output` predicate reuses the same lookup rather than
   re-scanning user_code.

sfx_pitch_envelope is the one audio-only example; its audio
golden flips by the usual cycle-accurate-APU-register-write-timing
drift caused by the NMI layout shifting ~14 bytes earlier.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:09 +00:00
Claude
6561daff35
linker: gate default smiley CHR tile on __default_sprite_used marker
Drop the built-in smiley from CHR tile 0 unless something in the
program actually references it. The marker fires when either:

  1. `IrOp::DrawSprite` lowering falls back to tile 0 because the
     sprite name doesn't resolve to a user declaration, or
  2. The same lowering sees a runtime `frame:` override (which
     could index any tile, including 0).

A third source of dependency — a background nametable entry of 0 —
is detected in the linker by scanning `bg.tiles` for zeros. This
preserves the smiley for programs like `examples/friendly_assets`
that use tile 0 as a background placeholder, even though their
draws resolve to user-declared sprites.

Programs whose draws all resolve to explicitly-declared sprites
with static frames AND whose backgrounds reference tiles 1+ now
leave CHR tile 0 as an all-zero blank, freeing 16 CHR bytes that
the user can treat as an always-transparent background tile.
Verified against the current example set: `sprites_and_palettes`
and `auto_chr_background` reclaim tile 0; every other example
keeps it (either they fall back to tile 0 via an undeclared draw
name or their background tilemap references tile 0).

All 33 emulator goldens still pass — removing an unreferenced CHR
tile can't change observable output.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:09 +00:00
Claude
7533ac281e
linker: skip default palette + rendering enable for non-visual ROMs
Add an `__oam_used` marker dropped by IrOp::DrawSprite codegen, and
compute a `has_visual_output` flag in the linker from the marker
plus the presence of any user palette / sprite / background. When
that flag is false — i.e. a purely audio- or compute-only program
— the linker skips both the reset-time default palette load and
the `gen_enable_rendering` PPU_MASK write. `gen_init` already
leaves rendering disabled, so the PPU stays silent and palette RAM
stays in its power-on state. ~72 bytes reclaimed for non-visual
programs.

Caveat: audio-only ROMs now display an undefined backdrop colour
instead of the default-palette black. jsnes renders that as a
mid-grey; Mesen/real hardware may vary. Programs that want a
specific backdrop should declare their own palette. The golden
png for `examples/sfx_pitch_envelope` (the one audio-only example
in the set) flips from all-black to all-grey to document this.

`__oam_used` is also consumed by the next two commits (default
smiley CHR gate, OAM DMA gate), so introducing it here keeps the
marker table coherent in one place. Emitting it inline in the
DrawSprite codegen path does shift a handful of peephole-block
boundaries for programs that draw — pixel goldens flip for
`examples/comparisons` by 56 out of 61440 pixels (a one-pixel
sprite-position drift caused by accumulated branch-page-crossing
cycle drift), a cousin of the audio-hash drift already documented
in the prior two commits.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:08 +00:00
Claude
37974611ae
linker: shrink default palette load from inline stores to loop
The reset-time "no user palette" path was emitting 32 unrolled
`LDA #imm / STA $2007` pairs (~170 bytes) to write the built-in
palette. Replace it with the same indirect-loop loader the
user-palette path already uses (runtime::gen_initial_palette_load),
with the 32-byte default palette spliced into PRG under a
`__default_palette` data block. Net saving is ~120 bytes — ~20
bytes of code + 32 bytes of data vs ~170 bytes of unrolled stores.

Delete `Linker::gen_palette_load` (dead after the refactor) and its
unit test. Replace with two tests covering the observable
behaviour: the default palette bytes appear in PRG when no user
palette is declared, and the `__default_palette` label is
suppressed when the user does declare a palette.

Audio goldens flip again for audio_demo, noise_triangle_sfx, and
sfx_pitch_envelope. These are the three audio examples that don't
declare their own palette — shrinking the default-palette load
shifts their audio tick's absolute address by ~120 bytes, which
changes branch page-crossing timing and therefore the exact APU
register write sample offsets. Same class of drift as the
mul/divide gating commit.

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:08 +00:00
Claude
033d399565
runtime: gate __multiply / __divide on usage markers
Drop __mul_used from IrOp::Mul codegen and __div_used from IrOp::Div
/ IrOp::Mod codegen (modulo reuses the same routine). The linker
skips gen_multiply / gen_divide for programs that never emit the
markers, following the same pattern already used by __audio_used /
__ppu_update_used / __sprite_cycle_used.

The optimizer already rewrites multiplies and divides by constant
powers of two into shifts (and modulo by constant powers of two
into masks), so the markers only fire for genuinely runtime math.
A program like `examples/comparisons.ne` that never multiplies or
divides now reclaims ~56 bytes of PRG; programs that use only one
of the two reclaim the other's share.

Audio goldens flip for every example that uses audio. The .ne
sources are unchanged and the pixel goldens are byte-identical —
the audio stream differs only because removing the math routines
shifts the audio tick's absolute address in PRG by 56 bytes, which
changes which of its internal branches cross 6502 page boundaries
and therefore the per-frame cycle count of a single NMI by 1-5
clocks. Over 180 frames the accumulated drift shifts APU register
write timing enough to render a different digital sample stream
at the same logical wave shape. Expected consequence of ROM-layout
change under cycle-accurate emulation; documented path per
CLAUDE.md "Updating goldens".

https://claude.ai/code/session_016kM6P7PukktBDqTZexrrAN
2026-04-16 21:15:08 +00:00
9970a23673
Merge pull request #28 from imjasonh/claude/text-sha256-game-Zreui 2026-04-16 16:25:47 -04:00
Claude
b78ae498e7
performance.md: remove — all six wins shipped on this branch
Anchored on commit 33640f8; the milestone is closed:

  #1 33640f8..0b5470b  codegen: skip leaf prologue spill
  #4 0b5470b..726faef  sha256: specialize rotr_wk per amount
  #2 726faef..0600f5b  codegen: fuse compare-then-branch
  #3 df71c2b           peephole: drop dead LDA #imm
  #6 f2623cb           sha256/computing: track byte offsets directly
  #5 4afd196           ir: inline-asm {param} substitution after splice
     6696d79           codegen+ir: code-review followups (UTF-8, leaf,
                       tests)

The how-it-works writeups live in each commit's message. The
explanatory inline comments in `IrCodeGen::new` (leaf detection),
`gen_block` (cmp/branch fusion), `peephole.rs::remove_dead_loads`
(JMP-following), `try_inline_call_stmt` (inline-asm const-arg
constraint), and the new `function_is_leaf_detects_jsr_emitting_ops`
+ `inline_fun_with_asm_param_*` tests are the ongoing reference.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:46:43 +00:00
Claude
6696d790bb
codegen+ir: code-review followups (UTF-8 safety, leaf exhaustiveness, tests)
Three follow-ups from a fresh review of the perf milestone:

1. **UTF-8 safety in `substitute_asm_vars` and
   `substitute_inline_const_params`.** Both walked the asm body
   byte-by-byte and emitted each non-substituted byte via
   `out.push(bytes[i] as char)` — a Latin-1 reinterpretation that
   mangles non-ASCII characters in inline-asm comments. The brace-
   level scan stays byte-based (braces can't appear inside a UTF-8
   continuation), but the verbatim copy now uses
   `out.push_str(&body[i..i + ch_len])` with `ch_len` derived from
   the lead byte. Pre-existing latent bug in `substitute_asm_vars`,
   freshly introduced in `substitute_inline_const_params` —
   fixed in both, with a shared lead-byte length helper.

2. **`function_is_leaf` is now exhaustive on `IrOp`.** The match
   used to be selective: `Call`/`Mul`/`Div`/`Mod`/`Transition`/
   `InlineAsm` were checked, everything else fell through with
   `_ => {}`. A new variant added later that secretly emitted a
   JSR (e.g. a future `Mul16` calling `__multiply16`) would have
   silently broken any leaf function that touched it. Listed
   every current variant explicitly so the compiler errors at
   the match arm if a new variant ships, and added a
   `function_is_leaf_detects_jsr_emitting_ops` test that walks
   the known JSR-emitting constructs (Call, *, /, %, asm with
   JSR token) and asserts each disqualifies leafness.

3. **Cleanups.** `gen_block` now binds the fused-cmp dest temp
   inside the original tuple instead of re-matching
   `block.ops.last().unwrap()` to retire it. New
   `inline_fun_with_asm_param_cascades_through_nested_inline`
   test exercises the eval_const → const_args_stack path that
   lets the inner of two nested inline funs see its outer's
   parameter as the constant the top-level call passed. Defensive
   comment on `body_has_inline_asm` explaining why it deliberately
   doesn't recurse (relies on `is_splicable_void_stmt`'s
   no-control-flow guarantee).

ROMs and goldens unchanged — all the changes are non-observable
through the existing example surface. Verified: cargo
test/clippy/fmt clean on rustc 1.95.0; emulator harness 34/34;
reproducibility diff clean; demo gifs byte-match fresh captures.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:46:06 +00:00
Claude
4afd196d1e
ir: substitute inline-asm {param} at splice time for constant args
`inline fun` worked for plain-NEScript bodies but blew up on any
function that used inline asm with `{name}` substitution. The
codegen's `substitute_asm_vars` runs against the analyzer's
per-function scope; after splicing, the scope is the *caller*,
where the inlined fun's parameters don't exist. The result was
`{dst}` left as a literal token in the spliced asm body, and
the asm parser failing with `bad number `{dst}``.

Fix: at inline-expansion time (`try_inline_call_stmt`), when the
splicer detects a `Statement::InlineAsm` in the body and the
call site passed a compile-time constant for a parameter,
pre-substitute `{param}` with `#$<value>` so the spliced body
parses as an immediate-mode operand. The substitution is done
via a new `inline_const_args_stack` parallel to the existing
`inline_subs_stack`, populated from the args' `eval_const`
results.

When *any* arg is non-constant the splicer refuses to inline an
asm-containing function and falls back to a regular `Call` op —
preserving correctness, just at the cost of the JSR/RTS apparatus
the user was hoping to avoid. The fallback is exercised by the
new `inline_fun_with_asm_falls_back_for_runtime_arg` test.

`eval_const` is also extended to consult the same const-args
stack, so a nested inline like `inline_outer(K)` →
`inner(outer_param)` correctly recognises `outer_param` as the
constant `K` and recurses the substitution. Without this, the
cascade stopped at the first level.

Two new tests in `src/ir/tests.rs` lock in the behaviour:
  - `inline_fun_with_asm_param_substitutes_immediate` — verifies
    `{param}` becomes `#$<value>` in the spliced body and no
    Call op is left.
  - `inline_fun_with_asm_falls_back_for_runtime_arg` — verifies
    the fallback path emits a Call op.

The SHA-256 example doesn't itself opt into the new feature for
its primitives (full inlining would balloon ROM by 5-10 KB —
the call sites add up fast at 1500+ source-level uses); that's
left as a future opportunity. Hash output unchanged; emulator
harness 34/34; reproducibility diff clean.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:28:40 +00:00
Claude
f2623cb62b
sha256/computing: track byte offsets directly to skip per-iter shift
The phased compression driver was computing the schedule/round
*index* per iteration and then shifting it left by 2 to get the
byte offset (`schedule_one(i << 2)`, `round_one(r << 2)`). The
shift compiles to two ASLs per iteration — cheap, but pure dead
work since the byte offset is just the previous one + 4.

Track the byte offset as the loop counter and bump it by 4 each
iteration. The schedule and round APIs already wanted byte
offsets, so the call sites also get a touch shorter (no more
intermediate `var i: u8 = first_idx + step`).

Strict cycle savings are tiny — a handful per iteration — so
this is more about not leaving obviously redundant work in the
inner loop than a meaningful perf win. Hash output unchanged
(still AE9145DB…4E0D for "NES"); no other examples affected;
emulator harness 34/34.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:17:10 +00:00
Claude
df71c2bf50
peephole: drop dead LDA #imm before mem-INC/DEC + JMP
The IR codegen lowers `i -= 1` (and friends) into a `LoadImm temp,
1; Sub d, i, temp; StoreVar i, d` triple, and the optimizer
strength-reduces the Sub+StoreVar pair into `DEC i`. The
constant-load-into-A that used to feed the Sub stays around as a
dead `LDA #1`:

    LDA #1
    DEC ZeroPage(rem)
    JMP Label("__ir_blk_while_cond_…")

`remove_dead_loads` was set up to drop exactly this pattern but
gave up at the trailing `JMP` because it couldn't reason about
flow. Extend it to follow one unconditional `JMP <label>` to its
target and resume the dead-store scan from the next instruction.
The first instruction past the loop-condition label is reliably an
`LDA loop_var`, which overwrites A without reading it — so the
`LDA #1` is correctly identified as dead.

Conditional branches still end the scan (their not-taken path is
unconstrained) and only one JMP is followed (to keep the analysis
local). For SHA-256 specifically this drops two `LDA #1`s per
iteration of the rotate/shift bit-loops — about 1K cycles per
block. The same pattern fires across most examples' loop tails.

Verified: cargo test/clippy/fmt clean on rustc 1.95.0; emulator
harness 34/34; reproducibility diff clean; SHA-256 of "NES" still
computes to AE9145DB…4E0D. The cycle drift refreshes the four
audio hashes / golden frames timing-sensitive examples already
tracked.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:14:34 +00:00
Claude
0600f5b872
codegen: fuse compare-then-branch to drop boolean materialization
Every NEScript condition (`if x < N`, `while i < end`, etc.)
lowers in two IR ops: `CmpX(d, a, b)` materializes a 0/1
boolean into temp `d`, and the block's terminator
`Branch(d, t, f)` reads `d` and branches on it. The codegen
faithfully emitted both halves — `LDA / CMP / branch-to-true /
LDA #0 / JMP done / true: LDA #1 / done:`, then later
`LDA d_slot / BNE branch_t / JMP branch_f` — about 14 cycles +
13 bytes per condition.

The 6502's natural pattern is one `CMP` + one branch on the
flags it just set: 8 cycles, no register-clobber, no temp slot.
Detect the canonical pattern in `gen_block` (last op is an 8-bit
`CmpX` whose dest temp is what the terminator branches on, with
no other uses) and emit the fused form directly via a new
`gen_cmp_branch` helper. The temp's allocation, store, load, and
the terminator's branch fall away.

Bookkeeping subtlety: the source temps `a`/`b` must be retired
*after* the fused emit, not before — the original `gen_op` order
is "emit body of op, then `retire_op_sources`". Decrementing
their use counts before the CMP would free their slots while
they were still live; `load_temp(a)` would then re-allocate `a`
to whatever stale slot the free list popped next. Got hit by
this on the first attempt — the SHA-256 example dutifully
returned all-zero hashes until the order was fixed.

Updated `ir_codegen_local_label_suffix_is_bank_namespaced`: the
test was relying on `if x == 0` to emit `__ir_cmp_*` labels for
its bank-namespacing check, which the fusion now collapses into
direct branches. Switched the test source to a shift-by-variable
pattern (`x = x << n`), which always emits `__ir_shift_loop_*`
labels regardless of future cmp/branch optimizations.

Cycle savings: ~6 cycles per condition. The SHA-256 rotate
loops alone account for ~9K cycles per block. Across all
examples the cycle drift shows up as audio-tick phase shifts
in five timing-sensitive ROMs (`audio_demo`, `friendly_assets`,
`noise_triangle_sfx`, `platformer`, `sfx_pitch_envelope`); the
goldens for those are refreshed in this commit, plus
`platformer.gif` (the only demo gif whose bytes actually moved).

Verified: cargo test/clippy/fmt clean on rustc 1.95.0;
emulator harness 34/34; reproducibility diff clean; SHA-256 of
"NES" still computes to AE9145DB…4E0D.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 17:10:02 +00:00
Claude
726faef538
sha256: specialize rotr_wk and shr_wk per rotation amount
The generic `rotr_wk(dst, n)` does its byte/bit decomposition
with two runtime `while` loops — necessary in the abstract, but
wasteful in SHA-256 where every rotation amount is one of ten
fixed compile-time constants (2, 6, 7, 11, 13, 17, 18, 19, 22,
25). The loop overhead alone is ~80 cycles of bookkeeping per
call, on top of the actual rotation work.

Replace each `rotr_wk(SIG, 6)` call inside the sigma helpers
with a dedicated `rotr_wk_6(SIG)` (and similarly for the other
amounts and for `shr_wk_3` / `shr_wk_10`). Each new helper just
chains the right number of `byte_rotr_wk` and `rotr1_wk` calls
inline — no `rem` variable, no `>= 8` / `> 0` checks.

The original `rotr_wk` / `shr_wk` wrappers stay defined for
runtime-amount callers; nothing else in the example uses them
today, but keeping them documents the general-purpose form.

Per SHA-256 block: ~45K cycles saved across 384 sigma rotations.
Combined with commit 0b5470b's leaf-function fix, the per-block
compression cost drops by ~3.5 frames at NTSC.

The hash output is unchanged (still
AE9145DB5CABC41FE34B54E34AF8881F462362EA20FD8F861B26532FFBB84E0D
for "NES"), no other examples are affected, and the emulator
harness stays at 34/34.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:49:35 +00:00
Claude
0b5470b054
codegen: skip parameter-spill prologue for leaf functions
Leaf functions — those that never JSR another routine from inside
their body — don't need to spill the `$04..$07` parameter
transport slots into per-function RAM, because nothing inside the
body clobbers those slots. Detect them in `IrCodeGen::new` via a
linear scan over each function's IR ops, point their parameters
at `$04..$07` directly in `var_addrs` (and in a parallel
`leaf_param_overrides` map for inline-asm `{name}` substitution),
and have `gen_function` skip the spill prologue.

The "leaf" predicate is conservative: any of `IrOp::Call`, `Mul`,
`Div`, `Mod`, `Transition`, or an inline-asm body containing a
`JSR` token disqualifies the function. SetPalette /
LoadBackground / PlaySfx / StartMusic / DebugLog / DebugAssert
were verified by inspection to not emit JSRs.

Per call to a leaf primitive: `LDA $04 / STA <local> / LDA $05 /
STA <local+1>` is now omitted — saves 12 cycles and 12 bytes of
code per call. Across the SHA-256 example's ~5500 leaf-primitive
calls per block, that's ~66K cycles saved per compression — about
2.2 frames at NTSC.

The fix also touches every committed `examples/*.nes` (the leaf
prologue was emitted by every fun with params, not just the SHA
ones), so 9 ROMs and the same three timing-sensitive goldens
(war.png + platformer/pong/war audio hashes) get refreshed; the
two committed gifs that drifted do too.

Verified: cargo test/clippy/fmt clean on rustc 1.95.0; emulator
harness 34/34; reproducibility diff clean; SHA-256 of "NES" still
computes to AE9145DB…4E0D.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:47:07 +00:00
Claude
33640f824a
performance.md: scratchpad for the SHA-256 perf milestone
Six wins surfaced by the inner-loop analysis. This file gets
deleted when everything's shipped — anchors the work + the
after-each-change checklist while it's in flight.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:36:15 +00:00
Claude
4d1ebc0d18
compiler-bugs: clear out the FIXED entry for bug #1
Bug #1 (inline `asm { {param} }` resolving to an address nothing
writes to) landed in commit 76d0fd0 along with the SHA-256
workaround revert, so the log is back to its empty "no bugs
logged" state. The how-it-was-fixed writeup lives in commit
76d0fd0's message and the explanatory comment in
`IrCodeGen::new`, which is where future readers are likely to
need it.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:16:20 +00:00
Claude
20a244b9e7
examples: regenerate ROMs, gifs, and goldens after codegen local fix
Commit 76d0fd0 moved function-locals from a codegen-minted
`$0300+` absolute range into the analyzer's zero-page
allocations so inline-asm `{param}` substitutions resolve
correctly (compiler-bugs.md #1). Observable semantics are
preserved — the analyzer + codegen now agree, and every
primitive that used to work still does — but the emitted ROM
bytes change whenever a function reads or writes a local,
because zero-page addressing uses a 2-byte instruction and
absolute addressing uses 3.

Consequences that need regenerated artifacts:

- **Twelve committed `.nes` files are stale.** Same source, new
  compiler, different bytes. The `Build Examples` CI job
  rebuilds each example into a tmp path and diffs against the
  committed ROM, so any drift is a hard failure. Rebuilt all
  twelve (arrays_and_functions, bitwise_ops, coin_cavern,
  function_chain, loop_break_continue, mmc1_banked, platformer,
  pong, sprites_and_palettes, state_machine, structs_enums_for,
  war).

- **Three goldens drift by one animation frame.** Zero-page
  addressing shaves a cycle per local access, which over a full
  frame handler shifts timing-sensitive sequences by a cycle or
  two. war's dealing animation and platformer + pong's audio
  tick stream catch the shift at frame 180 — war's card under
  player A's deck is now one frame earlier in its slide, and all
  three programs' captured audio buffers start from slightly
  different envelope positions. The new goldens (`war.png` + the
  three `.audio.hash` files) reflect the same code compiled with
  the cycle-count-corrected primitives.

- **`platformer.gif` and `war.gif` rebuild.** Same one-frame
  timing drift, integrated across 360 frames of captured
  gameplay — the emulator job's gif-reproducibility check
  wouldn't pass without the refresh. `pong.gif` happened to
  byte-match the old capture after rebuild.

All verified:
  - `cargo clippy --all-targets -- -D warnings` clean on both
    rustc 1.94.1 and 1.95.0.
  - `cargo test --all-targets` — 616 + 3 + 75 tests pass.
  - Full emulator harness — 34/34 ROMs match goldens.
  - Committed-ROM reproducibility diff clean — every
    `examples/*.ne` compiles byte-identical to its committed
    `.nes`.
  - `docs/{platformer,war,pong}.gif` byte-match fresh captures.
  - SHA-256 of "NES" still computes to `AE9145DB…4E0D`.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:12:46 +00:00
Claude
76d0fd0d28
codegen: reuse analyzer's local allocations so inline asm {param} works
Fixes compiler-bugs.md #1 — the inline-asm `{name}` resolver
looks parameters up in the analyzer's `VarAllocation` table
(because that's the only address map it has), but `IrCodeGen::new`
was minting a parallel `$0300+` range for every function-local and
ignoring what the analyzer had picked. The spill prologue wrote the
param to the codegen's private address, the inline asm read from
the analyzer's zero-page address, and nothing ever bridged the two
— `LDA {param}` would silently load whatever the RAM clear left at
the stale slot (always `0`).

Fix: drop the `local_ram_next` loop and just look each local up in
`allocations` by the analyzer's qualified name
(`__local__{scope}__{local}`). The scope string that `gen_function`
already computed for `substitute_asm_vars` is now shared with the
new address-seeding loop via a `scope_prefix_for_fn(&str)` helper,
so the two call sites can't drift. The analyzer's layout already
satisfies the "no overlapping live locals" invariant the codegen
was relying on — it scopes every local under
`__local__<scope>__<name>` so two functions with a parameter named
`x` land in different slots.

Updated `gen_function_prologue_spills_params_to_local_ram`: the
regression test for the War-era param clobbering bug was asserting
the spill's destination specifically had to be an absolute address
at `$0300+`. That's no longer the mechanism — the spill lands in
whatever slot the analyzer assigned, which is zero page when
there's room. The test now asserts the destination is *any*
address outside `$04-$07`, which is the actual invariant.

Reverted the `LDX $04` / `LDY $05` workaround in
`examples/sha256/sha_core.ne` — every primitive there now uses
`{dst}` / `{src}` / `{w_ofs}` / `{h_ofs}` / `{k_ofs}` substitution
as originally intended. The "Parameter convention" comment that
documented the workaround is gone.

Regenerated `tests/emulator/goldens/inline_asm_demo.png`: that
example's `times_four(input)` was previously returning `input`
verbatim because the inline asm's `LDA {result}` / `ASL A` /
`ASL A` / `STA {result}` operated on a zero-page byte that was
disconnected from the NEScript-level `result` variable. With the
fix, `times_four` correctly returns `input * 4`, so the
smiley-tracker's frame-180 position shifts by the expected
`(frame_count * 4) mod 256` delta. The other 33 ROMs remain
byte-identical.

Verified:
  - `cargo clippy --all-targets -- -D warnings` clean on both
    rustc 1.94.1 and 1.95.0.
  - `cargo test --all-targets`: 616 + 3 + 75 tests pass.
  - `cargo fmt --check` clean.
  - Full emulator harness: 34/34 ROMs match goldens.
  - SHA-256 of "NES" still computes to
    `AE9145DB5CABC41FE34B54E34AF8881F462362EA20FD8F861B26532FFBB84E0D`.
  - `--memory-map` output now reflects what the generated code
    actually reads and writes (previously the codegen's $0300+
    override was invisible to the dump).

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:03:10 +00:00
Claude
f128170abf
compiler: satisfy four new clippy 1.95 lints so CI stays green
Rust stable rolled to 1.95.0 today (2026-04-16), which fires four
new warning-by-default lints on pre-existing code. All four have
mechanical fixes suggested by clippy itself:

  - `collapsible_match` (2x) in `src/analyzer/mod.rs` — merge the
    `if args.len() != N` guard into the `match` arm pattern. One
    diagnostic-push per arm, shape is identical to before.
  - `unnecessary_sort_by` in `src/optimizer/mod.rs` — replace
    `sort_by(|a, b| b.1.cmp(&a.1))` with
    `sort_by_key(|(_, c)| std::cmp::Reverse(*c))`.
  - `manual_checked_ops` in `src/optimizer/mod.rs::Div` folding —
    replace `if vb == 0 { 0 } else { va / vb }` with
    `va.checked_div(vb).unwrap_or(0)`. Same `x / 0 == 0` fallback.
  - `map_unwrap_or` in `src/main.rs` — `std::fs::metadata(...).
    map(|m| m.len()).unwrap_or(0)` → `.map_or(0, |m| m.len())`.

Verified with both the old 1.94.1 stable and the freshly-installed
1.95.0: `cargo clippy --all-targets -- -D warnings` is clean on
both; `cargo fmt --check` is clean; `cargo test --all-targets`
still passes 616 + 3 + 75; the emulator harness still matches
all 34 goldens byte-for-byte.

This unblocks the SHA-256 PR (imjasonh/nescript#28) and any
other PRs that run CI against Rust 1.95.

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 15:15:24 +00:00
Claude
5976b74b2f
compiler-bugs.md: document inline-asm {param} resolution bug
Full writeup of the codegen bug found while building the SHA-256
example: function parameters (and function-local `var`s)
referenced inside an `asm { ... }` block resolve to the
analyzer's zero-page allocation, but the codegen's prologue
spills the parameter transport slots ($04-$07) to a completely
different per-function RAM slot. Nothing copies between the two,
so `LDA {param}` always reads a stale zero-page byte.

The entry includes a minimal reproducer, a walk-through of the
two disagreeing address maps (analyzer's `var_allocations` vs.
codegen's `var_addrs` override in `Emitter::new`), the exact
workaround the SHA-256 primitives use (`LDX $04` / `LDY $05`
directly instead of `{dst}` / `{src}`), notes on the
inline_asm_demo golden silently encoding the bug
(`times_four(x)` returns `x`, not `x*4`), and a guess at the
fix (delete the codegen's override so both maps agree on the
analyzer's zero-page allocation).

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 15:04:15 +00:00
Claude
ba23f8578a
examples/sha256: interactive SHA-256 hasher with on-screen keyboard
An end-to-end FIPS 180-4 SHA-256 hasher running entirely on the NES.
The player types up to 16 ASCII characters on a 5x8 on-screen
keyboard, presses Enter, and the program computes and displays the
64-character hex digest.

Layout (`examples/sha256/*.ne`):
  constants.ne         layout + K[64] / H_INIT[8] tables
                       (declared as `var` with init_array because the
                       v0.1 compiler treats `const u8[N] = [...]` as
                       a no-op — noted in the file)
  assets.ne            44-tile Tileset (A..Z, 0..9, punctuation,
                       special keys, cursor) shared between BG and
                       sprite layers
  background.ne        static nametable (title, labels, keyboard
                       grid) painted at reset
  state.ne             globals
  sha_core.ne          32-bit byte primitives (copy, xor, and, add,
                       not, rotr, shr) in inline asm + sigma/Sigma
                       mixers + schedule/round steps + fold
  render.ne            OAM helpers for cursor, input buffer, and
                       64-nibble digest
  keyboard.ne          key dispatch table
  entering_state.ne    cursor navigation + typing + auto-demo
  computing_state.ne   phased driver (48 schedule steps + 64 rounds
                       + fold across ~30 frames at 4 iterations each)
  showing_state.ne     renders the 256-bit digest as 8 rows of 8
                       sprite glyphs

Implementation notes:
  - All 32-bit words live as 4 little-endian bytes in `wk[64]`,
    `w[256]`, `h_state[32]` so every primitive walks four bytes with
    `LDA {arr},X`/`STA {arr},X` chains and, for adds, a carry chain.
  - Every primitive reads its parameters straight out of the
    transport slots `$04`/`$05` rather than `{dst}`/`{src}`
    substitutions: the inline-asm resolver looks parameters up in
    the analyzer's allocation table but the codegen spills them to a
    different per-function RAM slot, so `{dst}` would resolve to a
    ZP slot nothing ever writes to. Bypassing the substitution
    entirely sidesteps the issue without a compiler change.
  - Rotate-right by any amount is a byte-rotate loop plus a bit-
    rotate loop so the 10 SHA amounts (2, 6, 7, 11, 13, 17, 18, 19,
    22, 25) all compile to a handful of chained `ROR`s.
  - The headless jsnes golden auto-types "NES" after 1 s of idle and
    captures its SHA-256 digest
    AE9145DB5CABC41FE34B54E34AF8881F462362EA20FD8F861B26532FFBB84E0D
    — byte-identical to `shasum` / `hashlib.sha256(b"NES")`.

Build: `cargo run --release -- build examples/sha256.ne`

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 14:02:58 +00:00
ee619681ff
Merge pull request #27 from imjasonh/claude/implement-pong-game-8jgsM 2026-04-16 07:01:19 -04:00
Claude
6d9ebc7d7b
docs: add docs/pong.gif demo to README
Record a 6-second gif of examples/pong.nes running in jsnes and
embed it alongside docs/platformer.gif and docs/war.gif as the
third project demo. The gif opens on Pong's title menu (CPU VS
CPU / 1 PLAYER / 2 PLAYERS) — warmup = 4 frames keeps the menu
as the thumbnail the way war's recording does, and then the
headless autopilot advances to gameplay partway through the
clip.

- docs/pong.gif committed (128 KB)
- README.md links it under the war demo
- scripts/pre-commit rebuilds it when examples/pong* or the
  recorder/harness change
- .github/workflows/ci.yml fails if the committed copy is stale
- CLAUDE.md and tests/emulator/record_gif.mjs reference the new
  gif in the "how to regenerate" sections

https://claude.ai/code/session_0134F5uwDEVTes2Ee9S7JeXy
2026-04-16 10:44:57 +00:00
Claude
21b91f6398
examples/pong: production-quality Pong game with powerups and multi-ball
A complete, playable Pong game split across examples/pong/*.ne files
and pulled in from a top-level examples/pong.ne. Features:

- **Title screen** with a 3-option menu (CPU VS CPU / 1 PLAYER /
  2 PLAYERS), a cursor sprite, blinking "PRESS A" prompt, brisk
  title march on pulse 2, and autopilot that auto-confirms CPU VS
  CPU after 45 frames of no input so the headless jsnes golden
  harness reaches gameplay by frame 180.

- **Ball physics** with signed-magnitude velocity (u8 magnitude +
  sign bit per axis), wall bounce at top/bottom, paddle AABB
  collision with push-out, and score-out detection at left/right
  exits.

- **Multi-ball** via parallel ball_* arrays (MAX_BALLS = 3). Each
  ball scores a point independently; the round continues until the
  last ball exits the playfield.

- **CPU AI** that tracks the nearest active ball heading toward its
  side with a per-frame step, 4 px dead zone, and CPU_SPEED = 1 so
  rallies can end naturally.

- **Three powerup types** that spawn every ~4 seconds, bounce off
  all four walls, and are caught by paddle AABB overlap:
  1. LONG — extends the catching paddle from 24 → 40 px for 5 hits
  2. FAST — doubles ball x-velocity on the catcher's next hit
  3. MULTI — spawns two extra balls on the catcher's next hit

- **Victory** at first-to-7 with a "PLAYER N WINS" banner and the
  builtin fanfare, auto-returning to Title.

- **Audio**: 5 user-declared sfx (WallBounce, PaddleHit, Score,
  PowerSpawn, PowerCatch) plus a title march and the builtin
  fanfare for victory.

Source layout mirrors examples/war:

    examples/pong.ne               top-level game shell
    examples/pong/PLAN.md          living design doc
    examples/pong/constants.ne     layout + gameplay constants
    examples/pong/assets.ne        45-tile Tileset (paddles, ball, alphabet,
                                   digits, cursor, center-line, powerup icons)
    examples/pong/audio.ne         sfx + music declarations
    examples/pong/state.ne         all mutable globals
    examples/pong/rng.ne           8-bit Galois LFSR
    examples/pong/render.ne        draw helpers
    examples/pong/input.ne         paddle step (human + CPU AI)
    examples/pong/ball.ne          multi-ball physics + paddle collision
    examples/pong/powerup.ne       powerup entity (spawn, bounce, catch, apply)
    examples/pong/title_state.ne   state Title + menu
    examples/pong/play_state.ne    state Playing (P_SERVE/P_PLAY/P_POINT)
    examples/pong/victory_state.ne state Victory

Verification:
- 616 compiler unit tests pass (cargo test --all-targets)
- cargo fmt / cargo clippy --all-targets -- -D warnings clean
- 33/33 emulator harness goldens match
- examples/pong.nes builds byte-identically from source

https://claude.ai/code/session_0134F5uwDEVTes2Ee9S7JeXy
2026-04-16 01:25:29 +00:00
53b5b021a6
Merge pull request #26 from imjasonh/claude/add-war-example-gif-BLJ2s
docs: add docs/war.gif demo to README
2026-04-15 20:41:03 -04:00
Claude
318a2f8bef
docs: add docs/war.gif demo to README
Captures the first ~6 s of examples/war.ne via the same
puppeteer + jsnes + gifenc pipeline that powers
docs/platformer.gif: title menu thumbnail, 52-card deal
animation, and a few rounds of CPU vs CPU play. Embedded
in the top-level README right under the platformer demo.

record_gif.mjs gains a 6th positional arg for the warmup
override (defaulting to the existing WARMUP env / 30) so
the war command can keep its title menu as the first frame
while platformer keeps skipping past its own title. The
CI emulator job and the pre-commit hook both rebuild the
gif into a tmp path and fail-with-fix-command if the
committed copy is stale; the war trigger covers war.ne,
war.nes, any examples/war/*.ne include, plus the recorder
and harness.
2026-04-16 00:37:23 +00:00
add0df6ff1
Merge pull request #21 from imjasonh/claude/war-card-game-PoMHA
examples/war: production-quality War card game + five compiler bug fixes
2026-04-15 19:56:03 -04:00
Claude
548787ac8a
W0110 inline fallback warning + docs refresh
W0110: when a function marked `inline` has a body shape the IR
lowerer can't splice (conditional early return, loops, nested
control flow, empty void body), the analyzer now emits a
warning at the declaration site so the declined hint is
visible instead of silently falling back to a regular JSR.

Implementation:
  - New `W0110` error code in `src/errors/diagnostic.rs` (warning level).
  - New `pub fn can_inline_fun(return_type, body) -> bool` in
    `src/ir/lowering.rs`, extracted from the existing capture
    logic so the analyzer and the IR lowerer share the same
    eligibility rules and can never drift.
  - New `check_inline_declinability` analyzer pass called from
    the tail of `analyze_program`, mirroring the existing
    `check_sprite_scanline_budget` / `check_unreachable_states`
    passes. Emits W0110 with help + note text pointing at the
    two accepted body shapes.
  - `capture_inline_bodies` now defers to `can_inline_fun`
    instead of duplicating the match pattern, so the two sides
    stay in lockstep by construction.

Four regression tests in `src/analyzer/tests.rs` cover the
conditional-return and while-loop declines plus the two
accepted shapes (single-return expression, void sequence).

Example source cleanups: `wrap52` in `examples/war/deck.ne`
and `abs_diff` in both `examples/arrays_and_functions.ne` and
`examples/loop_break_continue.ne` drop the `inline` keyword.
All three were dead hints — the `inline` was being silently
declined before this change, so removing it is source-only;
the three ROMs are byte-identical, all 32 emulator goldens
still match.

Docs refresh
  - `docs/language-guide.md`: rewrote the Inline Functions section
    (real behaviour + W0110), added W0105/W0106/W0107/W0108/W0109/
    W0110 to the warnings table, added the `debug.sprite_overflow*`
    builtins + sprite-per-scanline mitigations section to the
    Debug Mode docs, added a `cycle_sprites` statement entry and
    cross-referenced it from `draw`.
  - `docs/nes-reference.md`: fleshed out the "NEScript Memory
    Usage" block with the full ZP + high-RAM layout, including
    the new `$07EF` / `$07FC` / `$07FD` slots for sprite cycling
    and the debug sprite-overflow telemetry.
  - `docs/future-work.md`: documented all four debug query
    builtins in the "What ships today" block; updated the open
    "OAM allocation strategy" question to reference the shipped
    `cycle_sprites` path and ask about an automatic-flicker
    game attribute as a follow-up.
  - `docs/architecture.md`: updated the `ir/` and `optimizer/`
    module summaries to describe real inline splicing (now
    in lowering, not the optimizer).
  - `README.md`: reframed the `inline` bullet from "hint" to
    "real splicing for single-return / void-body shapes";
    expanded the debug-support bullet to mention the four
    query builtins and their stripping in release builds; added
    a new bullet for the three-layer sprite-per-scanline
    mitigations; bumped the test count from 497 → 694; updated
    the war.ne entry to mention the seven compiler bugs are all
    fixed and point readers at `git log` (instead of the
    deleted COMPILER_BUGS.md).
  - `examples/README.md`: same `git log`-pointing rewrite for
    the war.ne entry.

Deletions
  - `examples/war/COMPILER_BUGS.md` is removed. All seven
    catalogued bugs are fixed; the file's historical value
    lives in `git log` now. Every source-code comment and doc
    reference to the file has been updated to either point at
    `git log` or just describe the bug in place.

Test count: 616 unit + 75 integration + 3 doctests = 694 total.
Clippy / fmt clean. 32/32 emulator goldens match.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 23:19:07 +00:00
Claude
5e5bed39a5
sprite-per-scanline: add cycle_sprites runtime flicker + debug telemetry
W0109 (shipped last commit) catches the 8-sprites-per-scanline
hardware limit at compile time for static layouts, but the
dynamic case — enemy formations, projectile clusters, animated
NPCs where coordinates come from variables — was still silent.
This change adds two layers of defense on top of W0109:

Layer 2: `cycle_sprites` runtime flicker intrinsic
  New keyword statement that rotates the OAM DMA start offset
  one slot per call. When called once per `on frame`, the PPU's
  sprite evaluation picks up a different subset of the 12+
  overlapping sprites each frame, so the permanent-dropout
  failure mode becomes visible flicker — the classic NES
  technique used by Gradius, Battletoads, and every shmup.

  Implementation:
    - Lexer keyword `KwCycleSprites` and parser production.
    - AST `Statement::CycleSprites(Span)`.
    - `IrOp::CycleSprites` lowered by the IR pass.
    - Codegen emits `LDA $07EF / CLC / ADC #4 / STA $07EF` with
      natural u8 wrap, plus a one-shot `__sprite_cycle_used`
      marker label the first time it fires.
    - Linker detects the marker and switches `gen_nmi` to the
      cycling variant, which reads the rotating offset from
      `$07EF` into OAM_ADDR before the DMA instead of writing
      a literal 0. Programs that don't call `cycle_sprites`
      skip the marker and get byte-identical ROM output.

Layer 3: debug-mode sprite overflow telemetry
  Mirrors the frame-overrun pair (`debug.frame_overrun_count` /
  `debug.frame_overran`). In debug builds the NMI handler reads
  `$2002` at the top of vblank, masks bit 5 (the PPU's sprite
  overflow flag), and if set bumps a cumulative counter at
  `$07FD` plus a sticky bit at `$07FC`. The sticky bit clears
  on every `wait_frame`.

  New debug builtins:
    - `debug.sprite_overflow_count()` → u8 peek of $07FD
    - `debug.sprite_overflow()` → u8 peek of $07FC (sticky bit)

  The hardware flag has well-known quirks but is correct for
  the overwhelming majority of cases and costs ~15 cycles per
  frame to sample. Release builds emit no overflow-check code
  at all, so the four bytes at `$07EF` / `$07FC`-`$07FD` stay
  free for user allocation.

Related changes:
  - `gen_nmi` now takes an `NmiOptions` struct. Four bool
    parameters tripped clippy's `fn_params_excessive_bools`.
  - CLI `build` now renders analyzer warnings on a successful
    build. Previously warnings were silently dropped unless
    the user also ran `nescript check`, which made W0109
    effectively invisible to CI and local dev alike. Existing
    pre-existing W0103 / W0106 warnings on `coin_cavern`,
    `mmc3_per_state_split`, `sprites_and_palettes` surface
    too — not regressions, just now visible.

New example: `examples/sprite_flicker_demo.ne`
  Draws 12 sprites into a 4-pixel band, W0109 fires at compile
  time with nine labels pointing at the offenders, and a
  `cycle_sprites` call at the end of `on frame` turns the
  hardware dropout into flicker. The committed emulator golden
  captures one frame of the cycling pattern (deterministic).

Tests:
  - `runtime::tests::nmi_debug_mode_samples_sprite_overflow`
  - `runtime::tests::nmi_sprite_cycle_variant_reads_rotating_offset`
  - `ir_codegen::*::debug_sprite_overflow_count_loads_07fd`
  - `ir_codegen::*::debug_sprite_overflow_flag_loads_07fc`
  - `ir_codegen::*::wait_frame_clears_sprite_overflow_sticky_in_debug_mode`
  - `ir_codegen::*::wait_frame_release_does_not_touch_sprite_overflow_sticky`
  - `ir_codegen::*::cycle_sprites_emits_marker_and_add4`
  - `ir_codegen::*::cycle_sprites_marker_dedup_across_multiple_calls`
  - `ir_codegen::*::program_without_cycle_sprites_emits_no_marker`
  - `analyzer::*::accepts_debug_sprite_overflow_builtins`
  - `analyzer::*::rejects_unknown_debug_method_lists_all_four_known_names`
  - `analyzer::*::accepts_cycle_sprites_statement`

Docs: `examples/war/COMPILER_BUGS.md` §4 now describes all three
layers (W0109, `cycle_sprites`, debug telemetry) with reasoning
for when each applies. `README.md` and `examples/README.md` add
the new example to their tables.

All 32 emulator goldens still match — the cycling is opt-in
and programs that don't call `cycle_sprites` or enable debug
mode are byte-identical to the pre-change output.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 22:07:19 +00:00
Claude
d6cb84a5bd
compiler: close out bug #4 (W0109 sprite-per-scanline) and bug #5 (real inlining)
Fixes the last two deferred compiler bugs catalogued in
examples/war/COMPILER_BUGS.md, finishing the bug-cleanup arc on
the War branch.

Bug #5 — `inline fun` inliner
  Previously the `inline` keyword was parsed into `FunDecl.is_inline`
  and then dropped on the floor: every call site emitted a regular
  `JSR` through the $04-$07 transport slots. Now the IR lowerer
  captures inline function bodies up front in
  `LoweringContext::capture_inline_bodies` and rewrites call sites
  at lowering time. Two body shapes are supported:

    1. Single-return expression — the body is re-lowered in place
       of the `Call` op with the parameter names substituted to
       fresh IR temps for each argument.
    2. Void multi-statement body whose every statement is one of
       Assign/Call/Draw/Scroll/SetPalette/LoadBackground/WaitFrame/
       Play/StartMusic/StopMusic/InlineAsm/RawAsm/DebugLog/DebugAssert
       — the statements are spliced into the caller's block with
       the same parameter substitution machinery.

  Control-flow-heavy inline bodies (conditional early returns,
  loops, transitions) fall back to a regular out-of-line call with
  no diagnostic. That's predictable and documented in the bug-tracking
  doc. Nested inline expansion uses a substitution-frame stack so
  an inline calling another inline sees the right arguments.

  A codegen follow-up was needed because bug #3's scope-qualified
  local names broke `{result}` substitution in inline asm. The
  codegen now tracks `current_fn_scope_prefix` per function and the
  InlineAsm op tries the qualified name first before falling back
  to the bare name.

Bug #4 — W0109 sprite-per-scanline static check
  Adds a new warning code W0109 and an analyzer pass
  `check_sprite_scanline_budget` that walks each state's `on_frame`
  handler, collects literal-coordinate `draw` statements (including
  metasprite expansion via dx/dy offsets), and iterates scanlines
  0..240 to count how many 8x8 sprites overlap each line. When a
  scanline has > 8, the analyzer emits W0109 with labels pointing
  at each offending draw site plus a help message about staggering
  y-rows and a note explaining the hardware dropout. Non-literal
  coordinates are skipped (static analysis can't resolve them).
  Nested `if`/`while`/`for`/`loop` blocks are unioned conservatively.

Tests added
  src/ir/tests.rs
    - inline_fun_expression_body_emits_no_call_at_use_site
    - inline_fun_void_body_statements_are_spliced
    - inline_fun_with_conditional_return_compiles_as_regular_call
    - inline_fun_nested_inlines_substitute_correctly
  src/analyzer/tests.rs
    - analyze_sprite_scanline_budget_warns_over_eight
    - analyze_sprite_scanline_budget_ok_when_staggered
    - analyze_sprite_scanline_budget_skips_dynamic_coords
    - analyze_sprite_scanline_budget_expands_metasprites
    - analyze_sprite_scanline_budget_recurses_into_if

COMPILER_BUGS.md
  Bugs #4 and #5 marked **FIXED** in the status table, with full
  reproduction/root-cause/fix/regression-test write-ups updated in
  place. All seven catalogued bugs now have shipped fixes.

Artifact churn
  - examples/war.nes and examples/inline_asm_demo.nes rebuild
    byte-shifted (different JSR targets post-inliner).
  - tests/emulator/goldens/war.audio.hash shifts from 143660f to
    13443e28 — the inliner removes JSRs to set_phase, which nudges
    NMI sampling timing. No pixel diff; behavior is unchanged.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 21:33:00 +00:00
Claude
76dd8eacb0
compiler: fix three scoping bugs; war: revert all local/param workarounds
Three related scoping bugs from examples/war/COMPILER_BUGS.md,
all fixed in one pass because they're different layer
manifestations of the same "flat global namespace" problem:

## §3: function-local `var` declarations lived in one namespace

`src/analyzer/mod.rs::register_var` inserted every `var` it
saw — top-level, state-local, AND function-body local — into
the same `self.symbols: HashMap<String, Symbol>`. Two different
functions declaring `var i` collided on E0501, which is why
every local in war/*.ne had a function-prefix like `dfa_card`
or `dwp_px`.

Fix: add a `current_scope_prefix: Option<String>` to the
Analyzer, set it to `Some("<fn_name>")` when checking a
function body (or `Some("Title__frame")` for state handler
bodies), and have `register_var` store the declaration under
an internal key `"__local__{prefix}__{name}"`. New
`resolve_symbol` / `resolve_key` helpers try the
scope-qualified key first and fall back to the bare key for
globals / consts / enum variants / state-level vars / function
names. Every existing `self.symbols.get(name)` inside
body-checking code was swapped over.

Two `var i` declarations inside the SAME function body still
collide with E0501 — we scoped per function body, not per
nested block. Per-block scoping would require live-range
analysis to reuse RAM slots.

## §1b: same-named params across functions shared VarIds

`src/ir/lowering.rs::get_or_create_var` looked up names in a
single global `var_map`, so two functions both with a `card:
u8` parameter resolved to the same `VarId`. Whichever function
was lowered last won the zero-page slot mapping, silently
rerouting the other function's param reads to the wrong slot.

Fix: the IR lowerer now mirrors the analyzer's scope logic.
`LoweringContext` gains a `current_scope_prefix` field that
gets set in `lower_function` / `lower_handler`, and
`get_or_create_var` uses a new `scoped_key` helper that
prepends `"__local__{prefix}__"` when the qualified key exists
in `var_map` or `var_types`. Each function's parameters and
locals therefore get distinct VarIds, and the codegen's
`var_addrs` map naturally has no collisions.

## §2: param transport slots $04-$07 clobbered across nested JSRs

Parameters were passed AND kept in `$04-$07` for the lifetime
of a function. Any nested call overwrote those slots with its
own arguments, so the caller's params were silently corrupted
as soon as it invoked anything. Every war helper that took
params and called other helpers (draw_card_face, push_back_a,
etc) snapshotted its params into fresh locals at the top of
the body.

Fix: in `codegen/ir_codegen.rs::IrCodeGen::new`, every
function-local — including parameters — now gets a dedicated
per-function RAM slot at `$0300+`. Parameters are still passed
via the zero-page transport slots `$04-$07` as the calling
convention, but `gen_function` now emits a **prologue** at
every function entry:

    LDA $04
    STA <param_0_addr>
    LDA $05
    STA <param_1_addr>
    ... etc, up to 4 ...

By the time the body runs, every parameter lives in the
function's dedicated RAM slot, so any nested call can freely
clobber $04-$07 (writing its own arguments there) without
corrupting the caller's saved parameters. Costs 4 LDA/STA
pairs (≈ 20 bytes of ROM, 16 cycles) at every function entry
— worth it to make the calling convention sound.

## War cleanup

With all three fixes in place, every workaround prefix in
`examples/war/*.ne` is gone:

- `card_rank(card)` instead of `card_rank(crk_c)` — bug #1b
- `compare_cards(a, b)` instead of `compare_cards(cmp_a, cmp_b)`
- `push_back_a(card)` instead of `push_back_a(pba_in)` — bug #1b
- `var card: u8 = draw_front_a()` in bury_from_* — bug #3
- `var i: u8 = 0` freely in multiple functions — bug #3
- `fun push_back_a(card)` body no longer snapshots `card` into
  `pba_card` before calling wrap52 — bug #2
- `fun draw_card_face` body no longer snapshots x/y/card into
  locals before calling card_rank/card_suit — bug #2
- `draw_word_player` steps its own x without needing a
  `dwp_px` accumulator to avoid the `x + N` arg compilation
  quirk — that quirk was a downstream symptom of bug #2 and
  is also gone

The source is now about 300 lines shorter and significantly
more readable.

## Regression tests

Seven new tests nail these bugs down:

- `analyzer::tests::analyze_allows_same_local_name_in_two_functions`
- `analyzer::tests::analyze_allows_same_param_name_in_two_functions`
- `analyzer::tests::analyze_allows_same_local_name_in_two_state_handlers`
- `analyzer::tests::analyze_still_rejects_duplicate_local_in_same_function`
- `codegen::ir_codegen::gen_function_prologue_spills_params_to_local_ram`

Plus the four param-arity tests from the earlier E0506 fix
and the wide_hi-leak regression test from the previous
compiler fix. Total suite: 591 unit tests, all passing.

## Golden drift

The prologue change adds a few cycles to every function entry,
which shifts NMI sampling by a handful of cycles and flips
the audio-hash of any example that plays sfx or music
(platformer, war). `arrays_and_functions.png` also picks up a
1-pixel shift in its enemy positions due to the same timing
drift. All three golden updates are pure "compiler produces
different but functionally-identical output" — no game
behavior changed.

## What's still open in COMPILER_BUGS.md

- §4: 8-sprites-per-scanline hardware limit is invisible to
  user code. A static analyzer hint could help; deferred.
- §5: `inline` keyword is silently declined for short
  functions that the optimizer's inliner doesn't recognize
  (it only removes empty functions). Deferred pending a real
  single-return-expression inlining pass.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 20:33:41 +00:00
Claude
e10d09db76
examples/war: redesign card art — opaque bodies, 16x16 pips, checkerboard back
The first-pass card tiles were riddled with palette-0 transparent
pixels that let the felt background bleed through every rank
glyph, small suit, and big pip. At arm's length the cards looked
like they had green specks eating them. This commit rewrites all
of the card art from scratch:

- **Opaque card bodies.** Every pixel inside a card tile is now
  either white (palette 2), red (1), or black (3). No `.` values
  anywhere on a face or back tile. The green felt only shows
  outside the card rectangle, where it should.

- **Readable rank glyphs.** The 13 rank tiles (A, 2-9, 10, J, Q, K)
  are now drawn as bold black strokes on a solid white body. The
  "holes" of the letters (e.g. the triangle inside an "A") are
  white, not transparent.

- **16x16 big pips.** The big centre pip is now a 4-tile (16x16)
  shape split into TL/TR/BL/BR quadrants per suit. Previously it
  was a 2-tile (16x8) half-height strip that looked cramped. The
  TL/TR quadrants kept their existing tile indices (28-35) so the
  shift is local; the new BL/BR quadrants are appended after the
  BIG WAR letters at tiles 88-95 to avoid renumbering the entire
  alphabet / digits / UI tile range.

- **Distinct suit shapes.** Spade is a smooth teardrop with a
  short stem and base; heart is two symmetric lobes with a V
  bottom; diamond is a clean rhombus; club is three circles
  joined over a stem. Side-by-side they are unmistakable.

- **Clean checkerboard card back.** The old card back was a
  diamond lattice that had the same transparent-bleed problem
  and looked noisy anyway. Replaced with a crisp 2-pixel black-
  and-white checkerboard that tiles seamlessly across the card
  back's 16x24 footprint.

- **draw_card_face now emits 6 sprites in a rank/suit + 4-tile
  big-pip layout.** The previous 6-sprite layout was
  `[rank][ssuit] / [pipL][pipR] / [blankL][blankR]`; the new
  one is `[rank][ssuit] / [pipTL][pipTR] / [pipBL][pipBR]` with
  the bottom row carrying the bottom half of the big pip
  instead of being wasted blank tiles.

Also:
- New constants TILE_PIP_TL_BASE / TR / BL / BR replace the old
  TILE_PIP_L_BASE / TILE_PIP_R_BASE in constants.ne.
- Refreshed war.nes and the goldens. Every emulator harness
  test still passes (31/31).

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 18:31:15 +00:00
Claude
9137b1f713
examples/war: polish pass + README entry + plan close-out
End-of-implementation polish for the War example after the
compiler bugs were fixed:

- Title state now calls draw_big_war_banner instead of inlining
  12 draws — same pixel output, fewer lines.
- P_WAR_BURY redraws the previous round's face-up cards while
  the noise thumps fire so the table doesn't look empty for
  24 frames between the WAR banner and the new face-ups.
- Drop draw_word_war from render.ne (orphaned by the BIG WAR
  metasprite).
- Refresh comments in background.ne (now references the real
  felt tile) and deal_state.ne (drop the stale FRAMES_DEAL_STEP
  reference now that the deal pace is hard-coded at 2 frames).
- README.md and examples/README.md gain a war row.
- PLAN.md marks every implementation step complete and records
  the design revisions made along the way.
- Refresh the war audio hash to match the new ROM (the title
  screen helper change shifts one frame of pulse-2 timing
  enough to flip the FNV-1a). The frame-180 PNG is unchanged.

https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
2026-04-15 16:08:03 +00:00