From 2966a6ab1790a19b834ee8ee2b5f8f2b6b2b6deb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Apr 2026 13:02:58 +0000 Subject: [PATCH] pipeline: share a single compile function across CLI, bench, and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compile bench had a hand-maintained parallel copy of `src/main.rs::compile`, and that copy went out of sync after bank switching landed — the bench kept handing the linker `PrgBank::empty(...)` slots even though the CLI started populating per-bank instruction streams + trampoline requests. The assembler then panicked with `unresolved label: '__tramp_step_animation'` on `uxrom_user_banked.ne` under `cargo test --all-targets`, which is what CI runs. A plain `cargo test --release` (what CLAUDE.md used to document) never builds the bench so the bug slipped through local validation. Fix: - New `nescript::pipeline` module with `compile_source(source, source_dir, &CompileOptions)` that owns the full `parse → analyze → lower → optimize → codegen → peephole → link` pipeline including the per-bank stream + trampoline reconstruction. Returns a `CompileOutput` carrying the ROM, the linker result, analysis, IR, assets, instructions, and source-loc markers so downstream tools have one place to pull metadata from. - `src/main.rs::compile` reduces to file I/O + preprocessing + a single `compile_source` call + CLI-only side effects (`--dump-ir`, `--call-graph`, `--asm-dump`, `--memory-map`, `--symbols`, `--source-map`). - `benches/compile.rs::compile_pipeline` becomes a one-line `compile_source` call. It is now structurally impossible for the bench to drift from the CLI path. - `tests/integration_test.rs::compile_with_debug_artifacts` likewise delegates to `compile_source`. This also fixes a latent bug in the helper where it used `Linker::with_mapper` without `.with_header(...)` — programs opting into `header: nes2` would have quietly got an iNES 1.0 header through this path. - `CLAUDE.md`: updated the "Running the basics" section to specify `cargo test --all-targets` (plain `cargo test` skips benches) and to point at `scripts/pre-commit` with the exact install command. Also installed the hook in this worktree. All 24 existing `examples/*.nes` rebuild byte-identical through the new pipeline. 624 tests + all 25 emulator goldens pass. https://claude.ai/code/session_01MaNVcDmK9gsspRkdxowQAM --- CLAUDE.md | 27 +++- benches/compile.rs | 131 ++++---------------- src/lib.rs | 1 + src/main.rs | 254 +++++++++----------------------------- src/pipeline.rs | 238 +++++++++++++++++++++++++++++++++++ tests/integration_test.rs | 70 +++-------- 6 files changed, 362 insertions(+), 359 deletions(-) create mode 100644 src/pipeline.rs diff --git a/CLAUDE.md b/CLAUDE.md index c36c8d4..8319a54 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,13 +44,32 @@ re-derive the project conventions from scratch. ## Running the basics ```bash -cargo build --release # build the compiler -cargo test # all Rust tests (lib + integration) -cargo fmt # mandatory before committing -cargo clippy --all-targets # mandatory before committing; fix or #[allow] +cargo build --release # build the compiler +cargo test --all-targets # all Rust tests — MUST include --all-targets +cargo fmt # mandatory before committing +cargo clippy --all-targets -- -D warnings # mandatory; fix or #[allow] ./target/release/nescript build examples/hello_sprite.ne # build one ROM ``` +**Always pass `--all-targets` to `cargo test`.** CI runs `cargo test +--all-targets`, which additionally compiles and smoke-runs the `compile` +benchmark under `benches/compile.rs`. A plain `cargo test` skips that, +so a bench that doesn't compile will pass locally and red-flag CI — +this exact failure mode bit us once already (commit `889074a`). + +The repo ships a pre-commit hook at `scripts/pre-commit` that runs +`cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, +`cargo test --all-targets`, and the committed-ROM reproducibility diff. +Install it once per worktree with: + +```bash +cp scripts/pre-commit .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit +``` + +Do this before your first commit in a new worktree. The hook catches +stale ROMs, stale platformer gif, and divergent bench/compile pipelines +before they hit CI. + Compile every example at once: ```bash diff --git a/benches/compile.rs b/benches/compile.rs index c95c26d..e4edd8e 100644 --- a/benches/compile.rs +++ b/benches/compile.rs @@ -1,30 +1,31 @@ //! End-to-end compilation benchmarks. //! //! Each `examples/*.ne` file becomes its own Criterion group that -//! times the full `parse → analyze → lower → optimize → codegen → -//! peephole → link` pipeline the `nescript build` CLI runs. The goal -//! is to catch compile-time regressions — today every example -//! compiles in well under 100 ms, so a change that doubles that -//! shows up as a large red bar in `cargo bench`'s output. +//! times the full `preprocess → parse → analyze → lower → +//! optimize → codegen → peephole → link` pipeline the `nescript +//! build` CLI runs. The goal is to catch compile-time regressions +//! — today every example compiles in well under 100 ms, so a +//! change that doubles that shows up as a large red bar in +//! `cargo bench`'s output. //! //! The harness pre-reads every source file into memory before any //! measurement starts. Criterion's sample iterations then run only //! the in-memory compile path, so disk I/O never shows up on the //! hot loop. +//! +//! The bench calls [`nescript::pipeline::compile_source`] +//! directly so it's impossible for it to drift away from the CLI +//! compile path — a 2026-04 regression where a hand-maintained +//! parallel copy of the pipeline missed a new bank-switching step +//! is exactly what this refactor prevents. use std::fs; use std::path::{Path, PathBuf}; use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; -use nescript::analyzer; -use nescript::assets; -use nescript::codegen::{peephole, IrCodeGen}; -use nescript::ir; -use nescript::linker::{BankTrampoline, Linker, PrgBank}; -use nescript::optimizer; use nescript::parser; -use nescript::parser::ast::BankType; +use nescript::pipeline::{compile_source, CompileOptions}; /// Pre-loaded `.ne` source plus the directory it was read from. The /// directory matters because sprite `@binary` / `@chr` paths resolve @@ -55,8 +56,12 @@ fn load_examples() -> Vec { entries .into_iter() .map(|path| { - let source = fs::read_to_string(&path) + let raw = fs::read_to_string(&path) .unwrap_or_else(|e| panic!("failed to read {}: {e}", path.display())); + // Preprocess once up front (include inlining, etc.) + // so the hot loop never touches the filesystem. + let source = parser::preprocess_source(&raw, Some(&path)) + .unwrap_or_else(|e| panic!("preprocess failed for {}: {e}", path.display())); let name = path .file_stem() .and_then(|s| s.to_str()) @@ -74,102 +79,14 @@ fn load_examples() -> Vec { .collect() } -/// Run the full CLI compile pipeline on an in-memory source string. -/// Mirrors `compile` in `src/main.rs`: parse → analyze → IR lower → -/// optimize → IR codegen → peephole → link. Panics on any error so -/// a regression that breaks the pipeline surfaces immediately instead -/// of silently skewing the measurements. +/// Run the full compile pipeline on an in-memory source string +/// via the shared library entry point so the bench can't drift +/// away from the CLI build path. fn compile_pipeline(source: &str, source_dir: &Path) -> Vec { - let preprocessed = parser::preprocess_source(source, None) - .unwrap_or_else(|e| panic!("preprocess failed: {e}")); - - let (program, parse_diags) = parser::parse(&preprocessed); - assert!( - !parse_diags - .iter() - .any(nescript::errors::Diagnostic::is_error), - "parse errors: {parse_diags:?}" - ); - let program = program.expect("parse produced no program"); - - let analysis = analyzer::analyze(&program); - assert!( - !analysis - .diagnostics - .iter() - .any(nescript::errors::Diagnostic::is_error), - "analysis errors: {:?}", - analysis.diagnostics - ); - - let mut ir_program = ir::lower(&program, &analysis); - optimizer::optimize(&mut ir_program); - - let sprites = assets::resolve_sprites(&program, source_dir).expect("sprite resolution failed"); - let sfx = assets::resolve_sfx(&program).expect("sfx resolution failed"); - let music = assets::resolve_music(&program).expect("music resolution failed"); - let palettes = - assets::resolve_palettes(&program, source_dir).expect("palette resolution failed"); - let backgrounds = - assets::resolve_backgrounds(&program, source_dir).expect("background resolution failed"); - - let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program) - .with_sprites(&sprites) - .with_audio(&sfx, &music); - let mut instructions = codegen.generate(&ir_program); - peephole::optimize(&mut instructions); - - // Pull the per-bank instruction streams out of the codegen and - // reconstruct the trampoline requests for each banked function, - // mirroring the real CLI compile path in `src/main.rs`. A - // bench that left the switchable banks empty would panic in - // the assembler's fixup pass for any program that nests a - // function inside a `bank` block (e.g. `uxrom_user_banked`), - // because the `__tramp_` label emitted by IR codegen - // would have nowhere to resolve to. - let mut banked_streams = codegen.banked_streams().clone(); - for stream in banked_streams.values_mut() { - peephole::optimize(stream); + match compile_source(source, source_dir, &CompileOptions::default()) { + Ok(out) => out.rom, + Err(e) => panic!("pipeline failed: {e:?}"), } - let mut bank_trampolines: std::collections::HashMap> = - std::collections::HashMap::new(); - for func in &ir_program.functions { - if let Some(bank_name) = &func.bank { - bank_trampolines - .entry(bank_name.clone()) - .or_default() - .push(BankTrampoline { - tramp_label: format!("__tramp_{}", func.name), - entry_label: format!("__ir_fn_{}", func.name), - }); - } - } - - let linker = Linker::with_mapper(program.game.mirroring, program.game.mapper) - .with_header(program.game.header); - let switchable_banks: Vec = program - .banks - .iter() - .filter(|b| b.bank_type == BankType::Prg) - .map(|b| { - let stream = banked_streams.remove(&b.name).unwrap_or_default(); - let tramps = bank_trampolines.remove(&b.name).unwrap_or_default(); - if stream.is_empty() && tramps.is_empty() { - PrgBank::empty(&b.name) - } else { - PrgBank::with_instructions(&b.name, stream, tramps) - } - }) - .collect(); - linker.link_banked_with_ppu( - &instructions, - &sprites, - &sfx, - &music, - &palettes, - &backgrounds, - &switchable_banks, - ) } /// Criterion entry point. One benchmark group per example so the diff --git a/src/lib.rs b/src/lib.rs index 4270261..58ebf40 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,5 +8,6 @@ pub mod lexer; pub mod linker; pub mod optimizer; pub mod parser; +pub mod pipeline; pub mod rom; pub mod runtime; diff --git a/src/main.rs b/src/main.rs index 869d096..93ebaa4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,14 +3,10 @@ use std::io::Write as _; use std::path::{Path, PathBuf}; use nescript::analyzer; -use nescript::assets; use nescript::assets::{BackgroundData, PaletteData}; -use nescript::codegen::IrCodeGen; use nescript::errors::render_diagnostics; -use nescript::ir; -use nescript::linker::{render_mlb, render_source_map, BankTrampoline, LinkedRom, Linker, PrgBank}; -use nescript::optimizer; -use nescript::parser::ast::BankType; +use nescript::linker::{render_mlb, render_source_map, LinkedRom}; +use nescript::pipeline::{compile_source, CompileError, CompileOptions as PipelineOptions}; #[derive(Parser)] #[command(name = "nescript", about = "NEScript compiler — NES game development")] @@ -341,216 +337,84 @@ struct CompileOptions { } fn compile(input: &PathBuf, opts: &CompileOptions) -> Result, ()> { - let debug = opts.debug; - let asm_dump = opts.asm_dump; - let dump_ir = opts.dump_ir; - let memory_map = opts.memory_map; - let call_graph = opts.call_graph; - let no_opt = opts.no_opt; - let symbols_path = opts.symbols.as_ref(); - let source_map_path = opts.source_map.as_ref(); + // File I/O + preprocessing lives here so the pipeline module + // itself doesn't need to touch `std::fs`. That keeps the + // pipeline usable from a future WASM host that routes asset + // reads through a trait. let raw_source = std::fs::read_to_string(input).map_err(|e| { eprintln!("error: failed to read {}: {e}", input.display()); })?; - - // Preprocess: inline include directives let source = nescript::parser::preprocess_source(&raw_source, Some(input)).map_err(|e| { eprintln!("error: {e}"); })?; - let filename = input.to_string_lossy(); - - // Parse - let (program, parse_diags) = nescript::parser::parse(&source); - if !parse_diags.is_empty() { - render_diagnostics(&source, &filename, &parse_diags); - } - if parse_diags - .iter() - .any(nescript::errors::Diagnostic::is_error) - { - return Err(()); - } - let program = program.ok_or(())?; - - // Analyze - let analysis = analyzer::analyze(&program); - if !analysis.diagnostics.is_empty() { - render_diagnostics(&source, &filename, &analysis.diagnostics); - } - if analysis - .diagnostics - .iter() - .any(nescript::errors::Diagnostic::is_error) - { - return Err(()); - } - - // IR lowering and (optionally) optimization. `--no-opt` skips - // the IR optimizer pass entirely so optimizer-introduced - // miscompiles can be bisected against the unoptimized output. - let mut ir_program = ir::lower(&program, &analysis); - if !no_opt { - optimizer::optimize(&mut ir_program); - } - - if dump_ir { - print!("{}", ir_program.pretty()); - } - - if call_graph { - print_call_graph(&analysis); - } - - // Resolve sprite assets (CHR data + tile indices) relative to the - // source file's directory, so `@binary` / `@chr` paths work naturally. let source_dir = input.parent().unwrap_or_else(|| Path::new(".")); - let sprites = assets::resolve_sprites(&program, source_dir).map_err(|e| { - eprintln!("error: {e}"); - })?; - // Resolve audio assets: user-declared sfx/music plus any - // builtins referenced via `play foo` / `start_music bar` for - // names that aren't in the program's sfx/music declarations. - let sfx = assets::resolve_sfx(&program).map_err(|e| { - eprintln!("error: {e}"); - })?; - let music = assets::resolve_music(&program).map_err(|e| { - eprintln!("error: {e}"); - })?; - - // Resolve palette and background declarations into fixed-size - // ROM data blobs. These are purely compile-time — either the - // parser handed us an inline byte array, or the declaration - // named a PNG to decode relative to the source file's directory - // (`@palette("art/main.png")` / `@nametable("levels/1.png")`). - let palettes = assets::resolve_palettes(&program, source_dir).map_err(|e| { - eprintln!("error: {e}"); - })?; - let backgrounds = assets::resolve_backgrounds(&program, source_dir).map_err(|e| { - eprintln!("error: {e}"); - })?; - - // IR-based code generation. Lower → optimize → emit 6502. - // - // We hold on to the codegen after `generate()` because it - // carries the source-location marker list — one entry per - // `SourceLoc` IR op — which the CLI reads to emit a source - // map. Dropping the codegen before then would throw that - // metadata away. Source-marker emission is opt-in (the label - // pseudo-ops shift peephole block boundaries, which would - // flip release-mode ROM bytes if it was always on) — so we - // only enable it when the user actually asked for a source - // map on the command line. - let emit_source_map = source_map_path.is_some(); - let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program) - .with_sprites(&sprites) - .with_audio(&sfx, &music) - .with_debug(debug) - .with_source_map(emit_source_map); - let mut instructions = codegen.generate(&ir_program); - - // Peephole pass: cleans up the IR codegen's temp-heavy output — - // dead stores, redundant loads, short-branch folds, etc. - nescript::codegen::peephole::optimize(&mut instructions); - - if asm_dump { - dump_asm(&instructions); - } - - // Link into ROM with both graphic assets (sprite CHR) and audio - // assets (sfx envelopes, music note streams) spliced in. We use - // `Linker::with_mapper` so the iNES header's mapper byte - // reflects the source's `mapper:` declaration — without this - // the CLI always shipped mapper 0 (NROM) regardless of whether - // the program actually needed MMC1/MMC3 bank switching. - // - // For banked mappers (MMC1, UxROM, MMC3) we collect the - // declared `bank X: prg` entries and turn each into a 16 KB - // switchable slot. Programs that nest functions inside a - // `bank Foo { fun bar() { ... } }` block populate the matching - // slot with the IR codegen's per-bank instruction stream, plus - // a trampoline request for every nested function so the linker - // emits a `__tramp_` stub in the fixed bank for each - // cross-bank call site. Programs without nested functions still - // produce empty slots, which keeps existing banked ROMs - // (mmc1_banked, uxrom_banked, mmc3_per_state_split) byte-for-byte - // identical to the pre-banked-codegen output. - let linker = Linker::with_mapper(program.game.mirroring, program.game.mapper) - .with_header(program.game.header); - // Run the peephole optimizer on each per-bank stream the same - // way we did for the fixed-bank stream. Mismatched optimization - // levels would only matter to programs with banked code (no - // existing example), but it's the right default. - let mut banked_streams: std::collections::HashMap> = - codegen.banked_streams().clone(); - for stream in banked_streams.values_mut() { - nescript::codegen::peephole::optimize(stream); - } - // For each declared bank, collect the trampoline requests from - // every banked function whose body lives in this bank. The - // codegen's `function_banks` map is the source of truth — but we - // don't expose it on the public API, so reconstruct the same - // mapping here by walking the IR. This keeps the linker - // independent of any codegen internal state beyond - // `banked_streams`. - let mut bank_trampolines: std::collections::HashMap> = - std::collections::HashMap::new(); - for func in &ir_program.functions { - if let Some(bank_name) = &func.bank { - bank_trampolines - .entry(bank_name.clone()) - .or_default() - .push(BankTrampoline { - tramp_label: format!("__tramp_{}", func.name), - entry_label: format!("__ir_fn_{}", func.name), - }); + // Hand everything else off to the shared pipeline function + // so the CLI, the `compile` bench, and the integration-test + // helper all run the same compile path. When this block + // needs a new feature (new flag, new output, whatever), the + // change lands in `pipeline::compile_source` and every + // caller picks it up automatically. + let pipeline_opts = PipelineOptions { + debug: opts.debug, + no_opt: opts.no_opt, + emit_source_map: opts.source_map.is_some(), + }; + let out = compile_source(&source, source_dir, &pipeline_opts).map_err(|e| match e { + CompileError::Parse(diags) => { + render_diagnostics(&source, &filename, &diags); } - } - let switchable_banks: Vec = program - .banks - .iter() - .filter(|b| b.bank_type == BankType::Prg) - .map(|b| { - let stream = banked_streams.remove(&b.name).unwrap_or_default(); - let tramps = bank_trampolines.remove(&b.name).unwrap_or_default(); - if stream.is_empty() && tramps.is_empty() { - PrgBank::empty(&b.name) - } else { - PrgBank::with_instructions(&b.name, stream, tramps) - } - }) - .collect(); - let link_result = linker.link_banked_with_ppu_detailed( - &instructions, - &sprites, - &sfx, - &music, - &palettes, - &backgrounds, - &switchable_banks, - ); + CompileError::ParseProducedNothing => { + // The parser returned `None` with no diagnostics. + // Extremely unusual (empty input or similar) and + // there's nothing for the user to act on beyond a + // generic message. + eprintln!("error: parser produced no program"); + } + CompileError::Analyze(diags) => { + render_diagnostics(&source, &filename, &diags); + } + CompileError::AssetResolution(msg) => { + eprintln!("error: {msg}"); + } + })?; - // Memory map is reported after linking so the palette / - // background PRG ROM addresses are available in `link_result.labels`. - if memory_map { - print_memory_map(&analysis, Some(&link_result), &palettes, &backgrounds); + // Post-link CLI-only side effects: the various `--dump-*` + // flags and the two optional file outputs. These are not + // part of the pipeline because they're stdout / filesystem + // I/O, not compilation. + if opts.dump_ir { + print!("{}", out.ir_program.pretty()); } - - if let Some(path) = symbols_path { - let mlb = render_mlb(&link_result, &analysis.var_allocations); + if opts.call_graph { + print_call_graph(&out.analysis); + } + if opts.asm_dump { + dump_asm(&out.instructions); + } + if opts.memory_map { + print_memory_map( + &out.analysis, + Some(&out.link_result), + &out.palettes, + &out.backgrounds, + ); + } + if let Some(path) = opts.symbols.as_ref() { + let mlb = render_mlb(&out.link_result, &out.analysis.var_allocations); std::fs::write(path, mlb).map_err(|e| { eprintln!("error: failed to write symbol file {}: {e}", path.display()); })?; } - if let Some(path) = source_map_path { - let map = render_source_map(&link_result, codegen.source_locs(), &source); + if let Some(path) = opts.source_map.as_ref() { + let map = render_source_map(&out.link_result, &out.source_locs, &source); std::fs::write(path, map).map_err(|e| { eprintln!("error: failed to write source map {}: {e}", path.display()); })?; } - Ok(link_result.rom) + Ok(out.rom) } fn check(input: &PathBuf) -> Result<(), ()> { diff --git a/src/pipeline.rs b/src/pipeline.rs new file mode 100644 index 0000000..fefd4e9 --- /dev/null +++ b/src/pipeline.rs @@ -0,0 +1,238 @@ +//! End-to-end compile pipeline. +//! +//! One shared function ([`compile_source`]) drives the full +//! `preprocess → parse → analyze → IR lower → optimize → +//! codegen → peephole → link` sequence on an in-memory source +//! string. The CLI ([`crate::main`]), the compile benchmark +//! (`benches/compile.rs`), and the integration-test helper +//! (`tests/integration_test.rs::compile_with_debug_artifacts`) +//! all route through this one function, so any future change to +//! the pipeline is picked up everywhere without hand-maintained +//! parallel copies. +//! +//! The CI `cargo test --all-targets` job used to panic for a +//! release where the bench's hand-maintained copy diverged from +//! the CLI after the banked-codegen landing — that class of bug +//! can't recur now that the bench calls [`compile_source`] +//! directly. +//! +//! This module deliberately takes **already-preprocessed source +//! text** and an **explicit source directory** rather than a +//! filesystem path, so it stays friendly to future WASM hosting: +//! the caller is the only layer that needs to touch `std::fs`. + +use std::collections::HashMap; +use std::path::Path; + +use crate::analyzer::{self, AnalysisResult}; +use crate::asm::Instruction; +use crate::assets::{self, BackgroundData, MusicData, PaletteData, SfxData}; +use crate::codegen::{peephole, IrCodeGen}; +use crate::errors::Diagnostic; +use crate::ir::{self, IrProgram}; +use crate::lexer::Span; +use crate::linker::{BankTrampoline, LinkedRom, Linker, PrgBank, SpriteData}; +use crate::optimizer; +use crate::parser; +use crate::parser::ast::BankType; + +/// Knobs that mirror the CLI `build` flags. New knobs should +/// default to the "release build" value so that old callers pick +/// up sensible behaviour on upgrade. +#[derive(Debug, Default, Clone, Copy)] +pub struct CompileOptions { + /// Enable `--debug` mode: bounds checks, frame-overrun + /// counter, `debug.log` / `debug.assert` emission. + pub debug: bool, + /// Skip the IR optimizer. Matches `--no-opt`. + pub no_opt: bool, + /// Emit `__src_` label pseudo-ops for every lowered IR + /// statement and record their spans on the codegen's + /// [`IrCodeGen::source_locs`] side table. The CLI turns this + /// on when `--source-map` is passed; the bench and release + /// builds leave it off because the labels become peephole + /// block boundaries and would shift ROM bytes. + pub emit_source_map: bool, +} + +/// Everything the CLI, the bench, and the integration tests need +/// from a full compile run. Carries the raw ROM plus enough +/// metadata to render a memory map, emit a `.mlb` symbol file, or +/// emit a source map — whatever the caller wants to do with it. +pub struct CompileOutput { + /// Final assembled iNES ROM bytes (header + PRG + CHR). + pub rom: Vec, + /// Full linker result including the label table + fixed-bank + /// PRG file offset. Used for `.mlb` / source-map rendering. + pub link_result: LinkedRom, + /// Analyzer result, kept around for post-link reporters that + /// need the symbol table (`.mlb`) or the variable allocation + /// map (`--memory-map`). + pub analysis: AnalysisResult, + /// The IR program post-(optional) optimization, kept so + /// `--dump-ir` and the call-graph reporter have something to + /// print without re-running the lowering. + pub ir_program: IrProgram, + /// Resolved sprite data (CHR + tile indices). + pub sprites: Vec, + /// Resolved sfx envelopes. + pub sfx: Vec, + /// Resolved music note streams. + pub music: Vec, + /// Resolved palette blobs. + pub palettes: Vec, + /// Resolved background blobs. + pub backgrounds: Vec, + /// Final post-peephole fixed-bank instruction stream. Used by + /// `--asm-dump`. + pub instructions: Vec, + /// Source-location markers (`__src_`, span) the codegen + /// emitted when [`CompileOptions::emit_source_map`] is set. + /// Empty when source maps are off. + pub source_locs: Vec<(String, Span)>, +} + +/// Why the pipeline couldn't finish. The CLI translates each +/// variant into a human-readable error; tests and benches can +/// `unwrap()` with a sensible panic message. +#[derive(Debug)] +pub enum CompileError { + /// Parser produced one or more error-level diagnostics. The + /// caller gets the full diagnostic vector so it can render + /// whatever UI it wants. + Parse(Vec), + /// Parser returned `None` with no explicit errors (empty + /// input or similarly pathological). + ParseProducedNothing, + /// Analyzer produced one or more error-level diagnostics. + Analyze(Vec), + /// One of the asset resolvers (sprites, sfx, music, palette, + /// background) returned `Err`. + AssetResolution(String), +} + +/// Run the full compile pipeline on an already-preprocessed +/// source string. +/// +/// `source_dir` is used to resolve `@chr("…")` / `@palette("…")` +/// / `@nametable("…")` / `@binary("…")` paths that the parser +/// stored verbatim. Pass `Path::new(".")` when the program +/// doesn't reference any external assets. +/// +/// Returns either a full [`CompileOutput`] or a [`CompileError`] +/// describing the first phase that refused to continue. The +/// caller is responsible for rendering diagnostics — this +/// function never prints to stdout or stderr. +pub fn compile_source( + source: &str, + source_dir: &Path, + opts: &CompileOptions, +) -> Result { + // Parse. + let (program, parse_diags) = parser::parse(source); + if parse_diags.iter().any(Diagnostic::is_error) { + return Err(CompileError::Parse(parse_diags)); + } + let program = program.ok_or(CompileError::ParseProducedNothing)?; + + // Analyze. + let analysis = analyzer::analyze(&program); + if analysis.diagnostics.iter().any(Diagnostic::is_error) { + return Err(CompileError::Analyze(analysis.diagnostics)); + } + + // IR lowering plus (optionally) optimization. + let mut ir_program = ir::lower(&program, &analysis); + if !opts.no_opt { + optimizer::optimize(&mut ir_program); + } + + // Asset resolution. Each asset category reads its paths + // relative to `source_dir`, so the caller picks which file + // system view is "current". + let sprites = assets::resolve_sprites(&program, source_dir) + .map_err(|e| CompileError::AssetResolution(format!("sprites: {e}")))?; + let sfx = assets::resolve_sfx(&program) + .map_err(|e| CompileError::AssetResolution(format!("sfx: {e}")))?; + let music = assets::resolve_music(&program) + .map_err(|e| CompileError::AssetResolution(format!("music: {e}")))?; + let palettes = assets::resolve_palettes(&program, source_dir) + .map_err(|e| CompileError::AssetResolution(format!("palettes: {e}")))?; + let backgrounds = assets::resolve_backgrounds(&program, source_dir) + .map_err(|e| CompileError::AssetResolution(format!("backgrounds: {e}")))?; + + // IR → 6502 codegen. We hold on to the codegen after + // `generate()` because it carries the per-bank instruction + // streams and the source-location markers. + let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program) + .with_sprites(&sprites) + .with_audio(&sfx, &music) + .with_debug(opts.debug) + .with_source_map(opts.emit_source_map); + let mut instructions = codegen.generate(&ir_program); + peephole::optimize(&mut instructions); + + // Pull the per-bank streams out, run peephole on each, and + // reconstruct the trampoline requests. Programs with no + // banked functions get empty maps here and the linker emits + // byte-identical output to the pre-banked-codegen baseline. + let mut banked_streams: HashMap> = codegen.banked_streams().clone(); + for stream in banked_streams.values_mut() { + peephole::optimize(stream); + } + let mut bank_trampolines: HashMap> = HashMap::new(); + for func in &ir_program.functions { + if let Some(bank_name) = &func.bank { + bank_trampolines + .entry(bank_name.clone()) + .or_default() + .push(BankTrampoline { + tramp_label: format!("__tramp_{}", func.name), + entry_label: format!("__ir_fn_{}", func.name), + }); + } + } + + let linker = Linker::with_mapper(program.game.mirroring, program.game.mapper) + .with_header(program.game.header); + let switchable_banks: Vec = program + .banks + .iter() + .filter(|b| b.bank_type == BankType::Prg) + .map(|b| { + let stream = banked_streams.remove(&b.name).unwrap_or_default(); + let tramps = bank_trampolines.remove(&b.name).unwrap_or_default(); + if stream.is_empty() && tramps.is_empty() { + PrgBank::empty(&b.name) + } else { + PrgBank::with_instructions(&b.name, stream, tramps) + } + }) + .collect(); + + let link_result = linker.link_banked_with_ppu_detailed( + &instructions, + &sprites, + &sfx, + &music, + &palettes, + &backgrounds, + &switchable_banks, + ); + + let source_locs = codegen.source_locs().to_vec(); + + Ok(CompileOutput { + rom: link_result.rom.clone(), + link_result, + analysis, + ir_program, + sprites, + sfx, + music, + palettes, + backgrounds, + instructions, + source_locs, + }) +} diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 68549ab..749a028 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -2101,60 +2101,24 @@ fn no_opt_still_produces_valid_rom() { /// `--symbols`, and `--source-map` paths. Returns the ROM bytes /// along with the rendered `.mlb` and source-map text so the /// integration tests can assert against the whole chain. +/// +/// Routes through the shared [`nescript::pipeline::compile_source`] +/// so this helper can never drift away from the CLI compile path +/// — the bench had a hand-maintained parallel copy and it missed +/// the bank-switching wiring in commit `2fe943b`, which is the +/// regression that pushed us to share a single pipeline. fn compile_with_debug_artifacts(source: &str, debug: bool) -> (Vec, String, String) { - let (program, diags) = nescript::parser::parse(source); - assert!( - diags.is_empty(), - "unexpected parse errors: {diags:?}\nsource:\n{source}" - ); - let program = program.expect("parse should succeed"); - - let analysis = analyzer::analyze(&program); - assert!( - analysis.diagnostics.iter().all(|d| !d.is_error()), - "unexpected analysis errors: {:?}", - analysis.diagnostics - ); - - let mut ir_program = ir::lower(&program, &analysis); - optimizer::optimize(&mut ir_program); - - let sprites = assets::resolve_sprites(&program, Path::new(".")) - .expect("sprite resolution should succeed"); - let sfx = assets::resolve_sfx(&program).expect("sfx resolution should succeed"); - let music = assets::resolve_music(&program).expect("music resolution should succeed"); - let palettes = assets::resolve_palettes(&program, Path::new(".")) - .expect("palette resolution should succeed"); - let backgrounds = assets::resolve_backgrounds(&program, Path::new(".")) - .expect("background resolution should succeed"); - - let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program) - .with_sprites(&sprites) - .with_audio(&sfx, &music) - .with_debug(debug) - .with_source_map(true); - let mut instructions = codegen.generate(&ir_program); - nescript::codegen::peephole::optimize(&mut instructions); - - let linker = Linker::with_mapper(program.game.mirroring, program.game.mapper); - let switchable_banks: Vec = program - .banks - .iter() - .filter(|b| b.bank_type == BankType::Prg) - .map(|b| PrgBank::empty(&b.name)) - .collect(); - let link_result = linker.link_banked_with_ppu_detailed( - &instructions, - &sprites, - &sfx, - &music, - &palettes, - &backgrounds, - &switchable_banks, - ); - let mlb = nescript::linker::render_mlb(&link_result, &analysis.var_allocations); - let map = nescript::linker::render_source_map(&link_result, codegen.source_locs(), source); - (link_result.rom, mlb, map) + use nescript::pipeline::{compile_source, CompileOptions}; + let opts = CompileOptions { + debug, + no_opt: false, + emit_source_map: true, + }; + let out = compile_source(source, Path::new("."), &opts) + .unwrap_or_else(|e| panic!("pipeline failed: {e:?}")); + let mlb = nescript::linker::render_mlb(&out.link_result, &out.analysis.var_allocations); + let map = nescript::linker::render_source_map(&out.link_result, &out.source_locs, source); + (out.rom, mlb, map) } #[test]