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
`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
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
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
The IrLowerer's wide_hi map records "this u8 temp's high byte
lives at this other temp" pairs whenever a 16-bit value is
produced. Both lower_function and lower_handler reset next_temp
to 0 at the start of each function, but neither cleared wide_hi
— so stale (low_id -> high_id) entries from earlier functions
leaked into subsequent ones.
When a fresh function reused those temp IDs for unrelated u8
expressions, is_wide() returned spurious true and widen() handed
back stale (lo, hi) pairs whose hi happened to coincide with the
*next* temp ID fresh_temp() was about to allocate. The result
was 16-bit IR ops (CmpEq16 in particular) where the destination
temp aliased one of the source operand high bytes — for War this
made `match phase` arms past P_WIN_B impossible to enter and the
game would freeze with both face-up cards on the table forever.
Fix: clear wide_hi alongside the next_temp reset in both
lower_function and lower_handler. Adds a regression test
(ir::tests::wide_hi_does_not_leak_between_functions) that
constructs a function whose body has no u16 ops but follows a
function that does, and asserts no CmpEq16 op aliases its dest
with an operand high byte.
Also:
- Convert the war Playing state's phase machine from an
if-chain to a `match`, which is what tripped this bug to the
surface (it was lurking in earlier ROMs too but their layouts
never produced the dest/source collision shape).
- Refactor begin_draw_a/b to set fly_card / fly_face_up via
globals before calling arm_fly, since arm_fly only takes 4
params (the v0.1 ABI limit, now diagnosed by E0506).
- Hoist the P_RESOLVE comparison result to the global pf_result
to dodge the param-clobbering issue documented in
examples/war/COMPILER_BUGS.md §2.
- Document the bug as item #6 in COMPILER_BUGS.md with a
minimal repro and reproducer-test pointer.
- Refresh the war golden + audio hash to match the new ROM.
https://claude.ai/code/session_0143dTgh3UeRrtfHgQwzcv5z
A focused review of the branch surfaced two correctness bugs and
four important polish items in the new features. None of the
existing example goldens shift — every fix is gated on conditions
that don't fire in the committed examples.
**debug.frame_overran() never reset between frames in implicit
wait_frame programs.** The IR-level WaitFrame op cleared $07FE,
but the implicit main-loop flag-clear that runs between dispatch
iterations only cleared ZP_FRAME_FLAG. A program whose
`on frame { ... }` body had no explicit `wait_frame` would latch
$07FE to 1 on the first miss and never reset, breaking
`debug.assert(not debug.frame_overran())` guards. The dispatch
loop now also clears $07FE in debug builds, mirroring the
WaitFrame path. New regression test asserts the main loop emits
exactly one STA $07FE in a no-wait_frame debug build.
**Metasprite base-tile resolution silently miscompiled for
`@chr` / `@binary` sprites.** The IR lowering walks
`program.sprites` to compute base tile indices but assumes
1 tile per non-Inline source, while the real asset resolver
reads the file. The analyzer now hard-rejects the combination
with a clear "use inline pixels" hint instead of letting it
compile to a visual glitch. New analyzer test
`analyze_metasprite_with_external_chr_sprite_errors` covers it.
**next_sprite_tile capping silently allowed CHR overlap.** The
pipeline used `.min(255)` which would let a background tile
overwrite a sprite tile when the sprite range filled the
pattern table. Now hard-errors via CompileError::AssetResolution
when the sprite range >= 256 *and* the program declares any
`@nametable(...)` background. Inline backgrounds aren't affected.
**Linker silently truncated background CHR overflow.** The
`if end <= chr.len()` guard at the CHR copy site dropped any
auto-CHR bytes that would have run past the pattern table.
Replaced with a debug assertion since the resolver should
have caught it upstream — defense in depth.
**Stale comment in nested_structs.ne** said struct literals
"don't accept array fields yet" while the example itself
demonstrates inline array fields working through
`expand_struct_literal_init`. Comment updated.
**Misleading sentinel comment in audio.rs** described the pitch
envelope's trailing zero as a runtime sentinel; in practice the
volume tick `JMP`s to `__audio_sfx_done` first and the pitch
update block never reads the trailing byte. Rewrote the comment
to clarify it's padding for predictable blob length.
Also tidies up two minor items the reviewer flagged:
- `flatten_struct_fields` rebuilt the `struct_sizes` HashMap on
every leaf field; hoisted the snapshot to the function entry.
- Integration tests called `resolve_backgrounds(..., 0)` (the new
`next_sprite_tile` parameter); changed to `1` so a future
PNG-nametable test fixture won't accidentally overwrite the
runtime smiley at tile 0.
https://claude.ai/code/session_01KEczoNUX3WmcFLfq6iAQxB
Multi-tile sprites used to require one hand-written `draw` per tile,
e.g. the four-call sequence in `examples/platformer.ne`'s
`draw_player()`. The new `metasprite Name { ... }` declaration
collects parallel `dx`/`dy`/`frame` arrays plus a reference to the
underlying sprite, and `draw Name at: (x, y)` expands to one OAM
slot per tile in the IR lowering — the codegen sees N regular
DrawSprite ops, so the runtime OAM cursor allocator picks them up
without any metasprite-specific awareness.
The metasprite's `frame:` array is interpreted *relative to the
underlying sprite's base tile*: index 0 means "the first tile this
sprite owns", which is the natural reading for a 16×16 hero whose
pixel art the asset resolver split into four consecutive tiles.
The lowering walks `program.sprites` to compute base tile indices
the same way `assets::resolve_sprites` would, then folds the base
into each frame entry before storing the metasprite info. Sprites
sourced from external `@chr(...)` / `@binary(...)` files whose
bytes aren't available at parse time fall back to a one-tile
assumption — those programs are rare and can declare metasprites
against pixel-art sprites instead.
The new `examples/metasprite_demo.ne` declares a 16×16 hero sprite
and arranges its four tiles into a metasprite, then sweeps the
hero across the screen so the harness captures it mid-motion.
The new keyword is added to the lexer/token list, and the parser
accepts `sprite:` (the otherwise-keyword) as a property name in
metasprite bodies so the natural spelling parses.
https://claude.ai/code/session_01KEczoNUX3WmcFLfq6iAQxB
Struct field types beyond the v1 scalar set (`u8`, `i8`, `u16`,
`bool`) used to error out with `E0201: struct fields must be
u8/i8/u16/bool`. The size accumulator already handled them
correctly — what was missing was: (1) the analyzer side that
synthesizes per-leaf symbols and allocations for nested structs
plus a single array-typed symbol for array fields, (2) the
parser's chained-field-access path, and (3) the IR-lowering
recursion through nested struct literal initializers and array
literal field values.
The synthetic-variable model carries through unchanged: a
`var p: Player` where `Player { pos: Vec2, hp: u8, inv: u8[4] }`
and `Vec2 { x: u8, y: u8 }` produces flat allocations for
`p.pos.x`, `p.pos.y`, `p.hp`, and `p.inv`, plus an intermediate
`p.pos` Struct symbol so dotted-name lookups still resolve. Array
fields get a single allocation with the array type so the
existing `Expr::ArrayIndex` lowering path handles `p.inv[i]`
without changes. Array-of-structs is still rejected with E0201
because the synthetic model can't index per-element layouts
without further codegen work.
The parser change is the only structural move: `parse_primary`
and `parse_assign_or_call` now loop the dot chain into a single
joined identifier so `p.pos.x` becomes `FieldAccess("p.pos", "x")`
and `p.inv[0]` becomes `ArrayIndex("p.inv", 0)`. The downstream
analyzer and IR lowering use the same `format!("{name}.{field}")`
join they already used for one-level access — no plumbing
changes required.
Includes a new `examples/nested_structs.ne` that exercises both
features end-to-end with two `Hero` instances carrying nested
positions and inventory arrays. The reproducibility tripwire
ROM is committed alongside it and the emulator harness has a
matching pair of golden files.
https://claude.ai/code/session_01KEczoNUX3WmcFLfq6iAQxB
The frame-overrun counter at $07FF was previously only readable
via `peek(0x07FF)`, which forces every program that wants to
guard against missed frames to know the magic address. This adds
two named query expressions:
- `debug.frame_overrun_count()` — cumulative miss count since reset
- `debug.frame_overran()` — sticky bit cleared by the next wait_frame,
so `debug.assert(not debug.frame_overran())` catches a miss in the
previous window without waiting for the counter to roll over.
The sticky bit lives at $07FE alongside the existing counter and
is set inside the same NMI-time overrun branch. Release builds
emit none of the runtime side: the NMI handler still skips both
writes, the codegen `wait_frame` only clears $07FE in debug mode,
and committed example ROMs stay byte-identical.
The new expression form parses through `parse_primary`'s `KwDebug`
arm, so the existing `debug.log(...)` / `debug.assert(...)`
*statement* parser stays untouched. The analyzer rejects unknown
methods with E0201 and stray arguments with E0203 so typos don't
silently compile to a zero load.
https://claude.ai/code/session_01KEczoNUX3WmcFLfq6iAQxB
Re-adds `palette Name { colors: [...] }` and
`background Name { tiles: [...], attributes: [...] }` as first-class
declarations, plus `set_palette Name` and `load_background Name`
statements for runtime swaps. Unlike the previous iteration that
quietly no-op'd, this one is fully wired through the pipeline and
its behavior is pinned by both unit tests and an emulator golden.
Pipeline:
- Lexer: re-adds `palette`, `background`, `set_palette`,
`load_background` keywords and tokenizes them.
- AST: `PaletteDecl` (name + 1..=32 colour bytes) and `BackgroundDecl`
(name + 0..=960 tile bytes + 0..=64 attribute bytes) live in
`Program`. `Statement::SetPalette` and `Statement::LoadBackground`
name-reference these declarations.
- Parser: `palette Name { colors: [...] }` / `background Name
{ tiles: [...], attributes: [...] }` blocks and their statement
forms parse via the existing byte-array helper.
- Analyzer: validates colour indices ($00-$3F), palette length
(<=32), nametable length (<=960), attribute length (<=64), and
duplicate decl names. `set_palette` / `load_background` targets
must reference a declared name (E0502 otherwise). When a program
declares palette or background, the analyzer bumps the user
zero-page allocator's starting address from `$10` to `$18` to
reserve `$11-$17` for the runtime update handshake — programs
that don't use the feature keep the old layout so their emulator
goldens stay byte-exact.
- Assets: `PaletteData` and `BackgroundData` resolve declarations
into zero-padded fixed-size blobs (32 / 960 / 64 bytes) and
expose `label()` / `tiles_label()` / `attrs_label()` for codegen
to reference.
- IR: new `IrOp::SetPalette(String)` and
`IrOp::LoadBackground(String)`; lowering forwards the names
verbatim.
- Codegen: `gen_set_palette` writes the palette label pointer into
ZP `$12/$13` and ORs bit 0 into the update flags at `$11`;
`gen_load_background` does the same for tile/attribute pointers
at `$14/$15/$16/$17` with bit 1. Both emit a `__ppu_update_used`
marker so the linker splices in the NMI apply helper only when
the feature is actually used.
- Runtime: `gen_initial_palette_load` and
`gen_initial_background_load` write the first declared
palette/background at reset time (before rendering is enabled,
where PPU writes are safe). `gen_nmi(has_ppu_updates)` takes a
new flag; when true it splices `gen_ppu_update_apply` at the top
of the NMI body, which checks the `$11` flags byte and copies
pending palette / nametable data to `$3F00` / `$2000` inside
vblank. All helpers use only ZP $02/$03 as scratch at reset time
and never clobber ZP slots live across NMI.
- Linker: new `link_banked_with_ppu` takes slice of `PaletteData` /
`BackgroundData`; splices each blob as a labelled data block in
PRG ROM, picks the first-declared as the reset-time load target,
enables background rendering automatically when a background is
declared, and threads `has_ppu_updates` into `gen_nmi`. Old
`link_banked` remains as a thin wrapper for callers without
palette/background data so existing tests don't shift.
Tests:
- Lexer: tokenization of the 4 new keywords (single added test case).
- Parser: 5 new tests for `palette` / `background` decls with and
without attributes, plus `set_palette` / `load_background`
statements.
- Analyzer: 9 new tests covering acceptance of declared
palettes/backgrounds, E0502 for unknown names, E0201 for
out-of-range NES colors and oversized blobs, E0501 for duplicate
names, and the zero-page-layout guard (palette/bg decls bump ZP
start; no decls keeps it at $10).
- Resolver: 3 new tests for zero-padding, truncation of oversized
decls, and label derivation.
- IR: 2 new lowering tests for `set_palette` and `load_background`.
- Integration: 5 new tests — blob contents spliced verbatim into
PRG, `STA $12` / `STA $14` emitted by set_palette /
load_background codegen, and a regression guard that programs
without palette/background still land user vars at $10.
- Emulator: new `examples/palette_and_background.ne` driven by a
frame counter that toggles between `CoolBlues` / `WarmReds` and
`TitleScreen` / `StageOne` every 90 frames. Golden PNG and audio
hash checked in under `tests/emulator/goldens/` and verified via
`node run_examples.mjs` — rendered image shows the blue
`CoolBlues` palette with the nametable populated from
`TitleScreen`.
Docs:
- `README.md` adds the feature to the headline list and the example
table.
- `docs/language-guide.md` restores the palette/background sections
with the full 32-byte layout table and `set_palette` /
`load_background` statement references.
- `docs/future-work.md` replaces the "removed as dead code" entry
with the remaining gaps (PNG-sourced palette and nametable
assets, cross-vblank large background updates, memory-map
reporting).
- `spec.md` restores the grammar productions and usage examples.
- `examples/README.md` lists the new demo.
All 497 unit + integration tests pass. Clippy clean. All 21
emulator goldens match after the update pass.
https://claude.ai/code/session_012fKB251HvEUQwG3tizFyqt
Two correctness bugs were silently producing wrong ROMs:
- `x << n` / `x >> n` always shifted by 1, regardless of `n`, because
the IR lowering for `BinOp::ShiftLeft`/`ShiftRight` hardcoded the
count. Now eval_const the RHS into a compile-time count; fall back
to a new `IrOp::ShiftLeftVar` / `ShiftRightVar` (runtime loop) when
the amount isn't constant. Strength reduction folds the variable
form back to a fixed count once the optimizer knows the value.
- `x / n` / `x % n` always returned 0, because the lowering emitted
`LoadImm(t, 0)` for `BinOp::Div`/`Mod` with a comment saying the
runtime call was "TODO for now". Added real `IrOp::Div` and
`IrOp::Mod`, wired them through use-counting and DCE, gave codegen
`__divide`-based implementations, and taught strength reduction to
rewrite power-of-two divisors into shifts and modulo-by-2ⁿ into
AND masks. Constant folding now handles `Mul`/`Div`/`Mod`/shifts
too, which were previously left for the codegen to emit inefficient
software calls.
Dead code removed (no backward-compat shims kept):
- `src/debug/` entirely. `DebugSymbols`, `SourceMap`, and the
Mesen/.sym emitters had no callers outside their own tests;
`main.rs` never wrote a symbol file. Documented the intent in
`docs/future-work.md` so it comes back intentionally if needed.
- `ErrorCode::E0202` (invalid cast) and `E0403` (unreachable state):
defined, formatted, and marked `#[allow(dead_code)]` but never
emitted. W0104 now carries the unreachable-state semantics too.
- `Level::Info`: never constructed.
- `load_background` / `set_palette` statements and their
`BackgroundDecl` / `PaletteDecl` parser support: parsed and
silently dropped by IR lowering (`// TODO: implement in asset
pipeline`). Removed keywords, AST variants, parser paths, analyzer
arms, and tests. `docs/future-work.md` documents the runtime
palette/nametable design for when it comes back.
Doc cleanup:
- `docs/architecture.md` was describing files that don't exist
(`analyzer/types.rs`, `optimizer/const_fold.rs`, `codegen/regalloc.rs`,
`rom/header.rs`, `debug/symbols.rs`, …). Rewrote it to match the
real flat `mod.rs` + `tests.rs` layout and the real pipeline order.
- `docs/future-work.md` was a hybrid of open work and "recently
completed" entries that duplicated the active stubs at the top of
the file. Collapsed to just the gaps that are actually still open.
- `README.md` claimed Mesen symbol export and 210 tests; updated both.
- `docs/language-guide.md` and `spec.md` described `palette` decls,
`set_palette` / `load_background`, `debug.overlay`, and error codes
that were never emitted. Trimmed.
- Stale comments on `Statement::Play`/`StartMusic`/`StopMusic`
claimed the audio subsystem was "a no-op at codegen time".
Tests:
- Regression tests for every fix above (`lower_shift_left_with_literal
_count_uses_that_count`, `lower_shift_right_with_variable_count
_uses_runtime_variant`, `lower_divide_emits_div_op_not_load_imm
_zero`, `lower_modulo_emits_mod_op_not_load_imm_zero`,
`strength_reduce_div_by_power_of_two`, `strength_reduce_mod_by
_power_of_two`, `strength_reduce_shift_var_with_constant_amount`).
- Renamed the `program_with_sprites_and_palette` integration test
(which was exercising the now-removed `load_background`/`set_palette`)
to `program_with_inline_sprite_chr`.
`examples/sprites_and_palettes.ne` lost its `palette`/`set_palette`
usage. Nothing in the emulator test presses A, so the headless
jsnes render shouldn't move, but the golden may need regeneration
via `UPDATE_GOLDENS=1` if it does.
https://claude.ai/code/session_012fKB251HvEUQwG3tizFyqt
`IrCodeGen::next_oam_slot` incremented at *compile time*: one
`draw` statement = one fixed OAM slot, baked into absolute-mode
stores at codegen. A `draw` inside a `while`/`for`/`loop` body
was lowered once and then always wrote to the same four OAM
bytes every iteration, so only the last iteration was ever
visible. The writeup in the earlier PR called this "bug B".
Fix: reserve ZP `$09` as `ZP_OAM_CURSOR`, reset it to 0 at the
top of every frame handler (right after the existing OAM clear
loop), and lower each `DrawSprite` IR op to:
LDY $09 ; load cursor
LDA <y_temp>
STA $0200,Y ; sprite Y
LDA #tile
STA $0201,Y ; tile
LDA #0
STA $0202,Y ; attr
LDA <x_temp>
STA $0203,Y ; sprite X
INC $09 x4 ; bump cursor by 4
Cost is ~+6 bytes per `draw` over the old static form. At 64
slots the u8 cursor wraps naturally, giving classic NES
"too many sprites" flicker instead of a silent compile-time
drop. `next_oam_slot` and its resets are gone from the IR
codegen entirely.
Secondary fix: `for i in 0..N` counters are now registered as
handler locals. `lower_statement` created a `VarId` for the
counter via `get_or_create_var` but never pushed it onto
`current_locals`, so the IR codegen's `var_addrs` lookup
returned `None` for every `StoreVar(i)` / `LoadVar(i)` and
silently emitted nothing. The counter stayed at 0 forever,
the loop spun indefinitely, and every iteration wrote the
first array element into OAM — turning all 64 sprites into
the same smiley. Same class as the handler-local `var` decl
bug from the earlier PR, just for for-loop variables.
Smoke-test deltas (all 14/14 still pass):
- arrays_and_functions: 104 -> 260 (player + 4 enemies)
- bitwise_ops: 104 -> 416 (player + flag sprites + pips)
- loop_break_continue: 208 -> 208 (already fixed by the earlier pass)
- structs_enums_for: 104 -> 260 (player + 4 enemies)
Regression tests:
- `ir_codegen::more_tests::ir_codegen_draw_sprite` — checks a
single `draw` emits `LDY cursor`, four `STA $020N,Y`, and
four `INC cursor`.
- `ir_codegen::more_tests::ir_codegen_multi_oam_uses_sequential_slots`
— rewritten for the new form: each draw gets its own
`LDY cursor` + 4 `INC cursor`.
- `ir_codegen::more_tests::ir_codegen_draw_in_loop_...` —
proves a `draw` inside a `while` compiles to ONE cursor-based
draw (not N unrolled statics and not zero), and asserts no
stray `STA $0204`/`$0208`/... absolute stores — those would
indicate bug B has regressed.
- `ir::tests::for_loop_counter_is_registered_as_handler_local`
— verifies `for i in 0..N` pushes `i` onto `current_locals`
so the IR codegen allocates it.
Smoke-test tightening: `tests/emulator/run_examples.mjs` now
has per-example `minNonBlack` floors. `arrays_and_functions`,
`structs_enums_for`, `loop_break_continue`, and `bitwise_ops`
all require multi-sprite rendering — if the OAM cursor bug
comes back, the smoke test fails loudly instead of passing on
the default `nonBlack > 0` check.
The legacy AST codegen in `src/codegen/mod.rs` still uses the
compile-time `next_oam_slot` approach. It's only reachable via
`--use-ast`, none of the examples use it, and its integration
tests only check iNES structure — left alone on purpose.
https://claude.ai/code/session_014Z5y3Q9krLcAxYpZQJhZ5V
Landing bug A from the previous writeup plus two adjacent bugs
that the fix exposed. All three miscompile anything that uses a
u8[N] global with a literal initializer.
1. Array-literal globals are now actually initialized.
`lower_program` only expanded `Expr::StructLiteral` into per-
field synthetic globals — `Expr::ArrayLiteral` hit
`eval_const`, returned `None`, and the array boot-cleared to
zero. `IrGlobal` now carries an `init_array: Vec<u8>`
populated by lowering, and the IR codegen startup loop emits
one `LDA #byte; STA base+i` pair per element.
2. Local variables no longer overlap array globals.
`IrCodeGen::new` advanced `local_ram_next` past
`max_global_base + 1` — for an array at `$0300-$0303` it
placed the first handler-local at `$0301`, inside the array.
The frame handler's stores through the local then corrupted
the array mid-frame. The allocator now walks the analyzer's
`VarAllocation` list and advances past `address + size` for
every RAM global, not just the base.
3. Peephole `remove_redundant_loads` honors indexed LDAs.
The pass tracked `LDA Immediate/ZeroPage/Absolute` but let
`LDA AbsoluteX/AbsoluteY/ZeroPageX/IndirectX/IndirectY` fall
through the match, leaving the A-equivalence tracker
unchanged. A later `LDA #v` that happened to match a stale
entry from BEFORE the indexed load would then be dropped as
"already in A" — a silent miscompile that turned every
`draw Sprite at: (arr[i], arr[j])` pattern into garbage
(the second array index would be computed from `arr[i]`'s
value, reading way out of bounds). Indexed LDAs now clear
the tracker.
Regression tests:
- `src/codegen/peephole.rs`: a synthetic
`LDA #0; TAX; LDA AbsX(arr1); STA temp; LDA #0; TAX;
LDA AbsX(arr2); ...` sequence asserts both `LDA #0`s survive.
- `src/ir/tests.rs`: verifies `var xs: u8[4] = [1,2,3,4]`
populates `IrGlobal::init_array` with `[1,2,3,4]`.
- `tests/integration_test.rs`: two IR-codegen tests — one checks
the startup instructions contain `LDA #v; STA base+i` for
every element, the other compiles a handler-local var
alongside an array global and asserts no post-init stores
land inside the array.
Smoke test impact (14/14 still passing, now more visible):
- arrays_and_functions: 56 -> 104 nonBlack, now animated
- loop_break_continue: 52 -> 208 (player + 3 hazards visible)
- structs_enums_for: 52 -> 104 (player + enemy visible)
Existing examples unchanged; no remaining work for bug B
(static OAM slot allocation in loops) — that's the next PR.
https://claude.ai/code/session_014Z5y3Q9krLcAxYpZQJhZ5V
Constants can now reference other constants and use arithmetic /
bitwise / comparison operators. The lowering runs \`eval_const\` on
each \`ConstDecl\` and \`VarDecl\` initializer, threading the existing
\`const_values\` map for identifier lookups.
Examples that now work:
const BASE: u8 = 10
const OFFSET: u8 = BASE + 5 // 15
const MASK: u8 = FLAG_A | FLAG_B // bitwise-or of two earlier consts
var counter: u8 = BASE // initializer is now 10
Enum variants are registered before constants so a const like
\`const FIRST: u8 = VariantA\` also resolves at compile time.
https://claude.ai/code/session_01W6eQFStA66EuMKHUFo2rx3
ReadInput now takes an explicit player index (0 = P1, 1 = P2). The IR
lowering for ButtonRead threads the player through, and the IR codegen
selects the correct zero-page input byte ($01 for P1, $08 for P2).
https://claude.ai/code/session_01W6eQFStA66EuMKHUFo2rx3
New src/codegen/ir_codegen.rs walks IrProgram and emits 6502 instructions.
This enables optimizer passes to actually affect the output ROM.
Design:
- Each IR temp gets a zero-page slot at $80 + temp_index
- Functions reset the temp counter at entry (temps don't outlive functions)
- Globals map by name to their analyzer-assigned zero-page addresses
- Operands are loaded into A, computed, stored back to the dest slot
Handles all IrOp variants:
- LoadImm, LoadVar, StoreVar (basic loads/stores)
- Add/Sub (CLC+ADC / SEC+SBC)
- Mul (JSR __multiply runtime routine)
- And/Or/Xor (zero-page operands)
- ShiftLeft/ShiftRight (repeated ASL/LSR)
- Negate/Complement (EOR #$FF + optional two's complement)
- CmpEq/Ne/Lt/Gt/LtEq/GtEq (CMP + conditional branch + 0/1)
- ArrayLoad/ArrayStore (TAX + ZeroPageX/AbsoluteX)
- Call (ZP param passing + JSR)
- DrawSprite (OAM slot 0 write, uses sprite_tiles map)
- ReadInput (LDA $01, P1 input)
- WaitFrame (poll frame flag at $00)
All terminators:
- Jump (JMP to block label)
- Branch (LDA temp + BNE true / JMP false)
- Return (optional value in A + RTS)
- Unreachable (BRK)
IR lowering fixes:
- ReadInput now has a destination IrTemp (was a side-effect-only op)
- ButtonRead uses the proper input temp instead of uninitialized register
- Logical AND/OR use new emit_move helper (OR with zero) instead of
bogus raw VarId for path merging
CLI:
- New --use-ir flag on `build` subcommand to opt in to IR codegen
- Default remains AST codegen (for now); IR codegen is experimental
All 7 examples compile through the IR pipeline and produce valid iNES ROMs.
Tests: 266 total (7 new ir_codegen unit + 2 new integration).
https://claude.ai/code/session_01W6eQFStA66EuMKHUFo2rx3
Analyzer extensions:
- Call graph construction from function bodies and state handlers
- DFS-based recursion detection (direct and mutual) with E0402 errors
- Max call depth computation per entry point with E0401 enforcement
- Function declarations registered as symbols (E0503 for undefined calls)
- Collects calls from all statement/expression types recursively
Optimizer (new module):
- Constant folding: evaluate known-constant arithmetic at compile time
- Dead code elimination: remove ops with unused destination temps
- Both operate per-basic-block in a single pass
171 tests total (22 new: 6 analyzer + 11 IR lowering + 5 optimizer)
https://claude.ai/code/session_01W6eQFStA66EuMKHUFo2rx3
