imjasonh/nescript
1
0
Fork 0
mirror of https://github.com/imjasonh/nescript synced 2026-07-09 09:18:01 +00:00
Code Activity

pipeline: share a single compile function across CLI, bench, and tests

Browse source 
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
This commit is contained in:
Claude 2026-04-14 13:02:58 +00:00
parent 889074a415
commit 2966a6ab17
No known key found for this signature in database
6 changed files with 362 additions and 359 deletions
Download patch file Download diff file Expand all files Collapse all files

1
src/lib.rs
View file

@ -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;

254
src/main.rs
View file

@ -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<Vec<u8>, ()> {
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_<name>` 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<String, Vec<nescript::asm::Instruction>> =
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<String, Vec<BankTrampoline>> =
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<PrgBank> = 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<(), ()> {

238
src/pipeline.rs Normal file
View file

@ -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_<N>` 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<u8>,
/// 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<SpriteData>,
/// Resolved sfx envelopes.
pub sfx: Vec<SfxData>,
/// Resolved music note streams.
pub music: Vec<MusicData>,
/// Resolved palette blobs.
pub palettes: Vec<PaletteData>,
/// Resolved background blobs.
pub backgrounds: Vec<BackgroundData>,
/// Final post-peephole fixed-bank instruction stream. Used by
/// `--asm-dump`.
pub instructions: Vec<Instruction>,
/// Source-location markers (`__src_<N>`, 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<Diagnostic>),
/// Parser returned `None` with no explicit errors (empty
/// input or similarly pathological).
ParseProducedNothing,
/// Analyzer produced one or more error-level diagnostics.
Analyze(Vec<Diagnostic>),
/// 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<CompileOutput, CompileError> {
// 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<String, Vec<Instruction>> = codegen.banked_streams().clone();
for stream in banked_streams.values_mut() {
peephole::optimize(stream);
}
let mut bank_trampolines: HashMap<String, Vec<BankTrampoline>> = 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<PrgBank> = 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,
})
}