analyzer: add four new warning diagnostics

Extends the analyzer with the warnings listed under "Error message
polish" in docs/future-work.md:

- W0102 (existing): now also fires for `while true { ... }` and
  `loop { if cond { continue } }`. `continue` is explicitly not
  counted as an exit — the loop still spins forever.
- W0105 (new): palette declarations whose sub-palette first-bytes
  disagree. The NES mirrors $3F10/$3F14/$3F18/$3F1C onto $3F00/
  $3F04/$3F08/$3F0C, so a 32-byte sequential write silently
  overwrites the background universal colour; the grouped
  `universal:` form auto-fixes this so only flat `colors: [...]`
  declarations can trip it.
- W0106 (new): a call at statement position whose callee has a
  declared return type — the value is silently dropped.
- W0107 (new): `fast` variables with fewer than three observed
  reads+writes, which don't justify holding a scarce zero-page
  slot. Leading-underscore names are exempt.

All four are warnings only — no IR, codegen, or runtime changes,
so every committed example ROM rebuilds byte-identical and no
emulator goldens flip. Tests added in src/analyzer/tests.rs.

One legitimate W0106 surfaces on examples/platformer.ne (the
`resolve_enemy_hit` helper uses early-return for control flow
but its return value is discarded by the caller); fixing it
would shift the ROM and flip goldens, so it is left in place
as an informational hint rather than a hard fix.

https://claude.ai/code/session_01MaNVcDmK9gsspRkdxowQAM

src/analyzer/mod.rs

@@ -45,6 +45,14 @@ const ZP_USER_CAP: u8 = 0x80;
 /// RAM at `$0000-$07FF`; the allocator uses up through `$07FF`.
 const RAM_END: u16 = 0x0800;
 
+/// W0107 threshold: a `fast` variable with fewer than this many
+/// observed accesses is flagged as wasting a zero-page slot. Writes
+/// and reads both count; the initializer of a `VarDecl` counts as a
+/// write. Three is a reasonable cutoff — anything below that barely
+/// justifies holding a scarce slot that codegen could use for a
+/// hotter variable.
+const W0107_MIN_ACCESSES: u32 = 3;
+
 /// Analyze a parsed program for semantic errors.
 pub fn analyze(program: &Program) -> AnalysisResult {
     // Pre-collect declared and builtin-matchable sfx/music names so
@@ -87,7 +95,9 @@ pub fn analyze(program: &Program) -> AnalysisResult {
         stack_depth_limit: DEFAULT_STACK_DEPTH,
         in_loop: false,
         used_vars: HashSet::new(),
+        var_access_counts: HashMap::new(),
         function_signatures: HashMap::new(),
+        function_return_types: HashMap::new(),
         current_return_type: None,
         in_function_body: false,
         struct_layouts: HashMap::new(),
@@ -129,9 +139,19 @@ struct Analyzer {
     /// Names of variables that have been read somewhere in the program.
     /// Used for the W0103 unused-variable warning.
     used_vars: HashSet<String>,
+    /// Count of observed references (reads + writes) for each
+    /// variable. Used for the W0107 fast-variable-underuse warning
+    /// to detect `fast var` declarations that hog a zero-page slot
+    /// without justifying it.
+    var_access_counts: HashMap<String, u32>,
     /// Function name to parameter types (in order). Used to validate
     /// call arity and argument types.
     function_signatures: HashMap<String, Vec<NesType>>,
+    /// Function name to declared return type. `None` here means the
+    /// function has no declared return type (i.e. "void"); functions
+    /// with a declared return type appear with `Some(ty)`. Used by
+    /// W0106 to detect implicit-drop of a return value.
+    function_return_types: HashMap<String, Option<NesType>>,
     /// Return type of the function currently being analyzed, or None
     /// when the function has no declared return type. Only meaningful
     /// when `in_function_body` is true.
@@ -214,6 +234,49 @@ impl Analyzer {
                     ));
                 }
             }
+            // W0105: the NES mirrors $3F10/$3F14/$3F18/$3F1C onto
+            // $3F00/$3F04/$3F08/$3F0C, so the "first byte" of every
+            // sub-palette at indices 0, 4, 8, 12, 16, 20, 24, 28 is
+            // really a single shared universal background colour.
+            // Writing a 32-byte blob sequentially with inconsistent
+            // values at those offsets silently overwrites $3F00 with
+            // whatever the last sprite sub-palette's first byte is —
+            // a classic "my screen is suddenly black" bug. The
+            // grouped-form palette parser auto-fixes this via its
+            // `universal:` field, so only the flat-form `colors:`
+            // path can trip this warning in practice.
+            let universals: Vec<u8> = [0, 4, 8, 12, 16, 20, 24, 28]
+                .into_iter()
+                .filter_map(|i| palette.colors.get(i).copied())
+                .collect();
+            if universals.len() >= 2 {
+                let first = universals[0];
+                if universals.iter().any(|&b| b != first) {
+                    self.diagnostics.push(
+                        Diagnostic::warning(
+                            ErrorCode::W0105,
+                            format!(
+                                "palette '{}' has inconsistent universal-background bytes across \
+                                 its sub-palettes",
+                                palette.name,
+                            ),
+                            palette.span,
+                        )
+                        .with_note(
+                            "the NES PPU mirrors $3F10/$3F14/$3F18/$3F1C onto \
+                             $3F00/$3F04/$3F08/$3F0C, so a 32-byte sequential \
+                             palette write overwrites the background universal \
+                             colour with sub-palette sp3's first byte",
+                        )
+                        .with_help(
+                            "set every sub-palette's first byte (indices 0, 4, 8, 12, \
+                             16, 20, 24, 28) to the same universal colour, or switch \
+                             to the grouped form (`universal:` + `bg0..sp3`) which \
+                             fixes the mirror automatically",
+                        ),
+                    );
+                }
+            }
         }
         let mut seen_backgrounds = HashSet::new();
         for bg in &program.backgrounds {
@@ -359,13 +422,78 @@ impl Analyzer {
             }
         }
 
+        // Check for under-used `fast` variables (W0107). A `fast`
+        // declaration reserves one of the NES's scarce zero-page
+        // slots, so it should see frequent traffic; below the
+        // threshold the author is spending a precious slot on a
+        // cold variable. Globals and state-locals both count.
+        for var in &program.globals {
+            self.check_fast_var_usage(var);
+        }
+        for state in &program.states {
+            for var in &state.locals {
+                self.check_fast_var_usage(var);
+            }
+        }
+
         // Check for unreachable states (W0104).
         self.check_unreachable_states(program);
     }
 
     /// Mark a variable name as having been read somewhere in the program.
+    /// Also bumps the W0107 access counter.
     fn mark_var_used(&mut self, name: &str) {
         self.used_vars.insert(name.to_string());
+        self.bump_var_access(name);
+    }
+
+    /// Increment the observed access count for `name`. Called for
+    /// both reads (via [`Analyzer::mark_var_used`]) and writes (at
+    /// `Statement::Assign` handling). Used for W0107.
+    fn bump_var_access(&mut self, name: &str) {
+        *self.var_access_counts.entry(name.to_string()).or_insert(0) += 1;
+    }
+
+    /// Emit W0107 if `var` is declared `fast` but sees fewer than
+    /// [`W0107_MIN_ACCESSES`] observed reads + writes across the
+    /// whole program. Variables that are already unused (W0103
+    /// fires) are skipped to avoid double-reporting; leading-`_`
+    /// names are also exempt by the same convention that W0103 uses.
+    /// Array-typed `fast` variables are skipped because they never
+    /// end up in zero page anyway (allocation kicks them to main
+    /// RAM), so the slot argument doesn't apply.
+    fn check_fast_var_usage(&mut self, var: &VarDecl) {
+        if var.placement != Placement::Fast {
+            return;
+        }
+        if var.name.starts_with('_') {
+            return;
+        }
+        if !self.used_vars.contains(&var.name) {
+            return;
+        }
+        if matches!(var.var_type, NesType::Array(_, _)) {
+            return;
+        }
+        let count = self.var_access_counts.get(&var.name).copied().unwrap_or(0);
+        if count < W0107_MIN_ACCESSES {
+            self.diagnostics.push(
+                Diagnostic::warning(
+                    ErrorCode::W0107,
+                    format!(
+                        "`fast` variable '{}' is accessed {count} time{plural}; \
+                         it wastes a zero-page slot",
+                        var.name,
+                        plural = if count == 1 { "" } else { "s" },
+                    ),
+                    var.span,
+                )
+                .with_help(
+                    "drop the `fast` qualifier so the variable lives in main \
+                     RAM and the zero-page slot can go to a hotter variable",
+                ),
+            );
+        }
     }
 
     /// Emit W0103 if `var` is never read anywhere. Variables named
@@ -789,6 +917,8 @@ impl Analyzer {
         let param_types: Vec<NesType> = fun.params.iter().map(|p| p.param_type.clone()).collect();
         self.function_signatures
             .insert(fun.name.clone(), param_types);
+        self.function_return_types
+            .insert(fun.name.clone(), fun.return_type.clone());
     }
 
     /// Attempt to allocate `size` bytes of RAM for a variable declared
@@ -925,6 +1055,11 @@ impl Analyzer {
                 if let Some(init) = &var.init {
                     self.walk_expr_reads(init);
                     self.check_expr_type(init, &var.var_type);
+                    // The initializer is a write to the variable;
+                    // count it toward W0107's access tally without
+                    // marking the variable "read" (W0103 still wants
+                    // to fire on a declared-but-unread var).
+                    self.bump_var_access(&var.name);
                 }
             }
             Statement::Assign(lvalue, _, expr, span) => {
@@ -939,6 +1074,13 @@ impl Analyzer {
                                     *span,
                                 ));
                             }
+                            // A plain scalar write doesn't count as a
+                            // read for W0103 (an unused variable might
+                            // only be written to, never read), but it
+                            // is still an access of the variable's
+                            // storage for W0107's "is this `fast`
+                            // slot worth it?" check.
+                            self.bump_var_access(name);
                         } else {
                             // Assigning to an undeclared name is an
                             // error — the lowering would otherwise
@@ -1000,13 +1142,29 @@ impl Analyzer {
                     self.check_block(block, state_names);
                 }
             }
-            Statement::While(cond, body, _) => {
+            Statement::While(cond, body, span) => {
                 self.walk_expr_reads(cond);
                 self.check_expr_type(cond, &NesType::Bool);
                 let was_in_loop = self.in_loop;
                 self.in_loop = true;
                 self.check_block(body, state_names);
                 self.in_loop = was_in_loop;
+                // W0102: a `while true { ... }` (or the rarely-written
+                // `while 1 { ... }`) that never breaks, returns,
+                // transitions, or waits for a frame is an infinite
+                // spin — same hazard as the bare `loop { ... }` below.
+                if is_always_true(cond) && !block_can_exit_or_yield(body) {
+                    self.diagnostics.push(
+                        Diagnostic::warning(
+                            ErrorCode::W0102,
+                            "infinite loop with no break, return, transition, or wait_frame",
+                            *span,
+                        )
+                        .with_help(
+                            "add `wait_frame`, `break`, `return`, or `transition` somewhere in the body",
+                        ),
+                    );
+                }
             }
             Statement::For {
                 var,
@@ -1109,6 +1267,23 @@ impl Analyzer {
                     self.check_intrinsic_args(name, args, *span);
                 } else if self.symbols.contains_key(name) {
                     self.check_call_signature(name, args, *span);
+                    // W0106: a call at statement position whose
+                    // callee has a declared return type silently
+                    // drops the value. Flag it so the author at
+                    // least acknowledges the discard.
+                    if matches!(self.function_return_types.get(name), Some(Some(_))) {
+                        self.diagnostics.push(
+                            Diagnostic::warning(
+                                ErrorCode::W0106,
+                                format!("return value of '{name}' is discarded"),
+                                *span,
+                            )
+                            .with_help(
+                                "bind the result to a variable (e.g. `var _result: u8 = f()`), \
+                                 or remove the return type from the function if the value isn't useful",
+                            ),
+                        );
+                    }
                 } else {
                     self.diagnostics.push(Diagnostic::error(
                         ErrorCode::E0503,
@@ -1561,6 +1736,17 @@ fn block_can_exit_or_yield(block: &Block) -> bool {
     block.statements.iter().any(stmt_can_exit_or_yield)
 }
 
+/// True if `expr` is an always-true constant condition — i.e. the
+/// literal `true`, or a non-zero integer literal. Used by W0102 so
+/// `while true { ... }` gets the same treatment as bare `loop`.
+fn is_always_true(expr: &Expr) -> bool {
+    match expr {
+        Expr::BoolLiteral(true, _) => true,
+        Expr::IntLiteral(v, _) => *v != 0,
+        _ => false,
+    }
+}
+
 fn stmt_can_exit_or_yield(stmt: &Statement) -> bool {
     match stmt {
         Statement::Break(_)

src/analyzer/tests.rs

@@ -1148,3 +1148,359 @@ fn analyze_does_not_reserve_zero_page_without_palette_or_bg() {
         .expect("x should be allocated");
     assert_eq!(x.address, 0x10);
 }
+
+// ── W0102 extended: `while true` + continue-only loops ──────
+
+#[test]
+fn analyze_while_true_without_exit_warns() {
+    // `while true { x = x + 1 }` — no break/return/wait_frame,
+    // so the same W0102 that fires on bare `loop { ... }` must
+    // also fire here.
+    let errors = analyze_errors(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        on frame {
+            while true { x = x + 1 }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        errors.contains(&ErrorCode::W0102),
+        "`while true` with no exit should produce W0102, got: {errors:?}"
+    );
+}
+
+#[test]
+fn analyze_while_true_with_wait_frame_ok() {
+    // `while true { wait_frame }` yields control to the NMI each
+    // iteration, so the NES actually makes progress — no warning.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        on frame {
+            while true { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0102),
+        "`while true` + wait_frame should NOT warn, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_while_true_with_break_ok() {
+    // A reachable `break` satisfies W0102 just like it does for
+    // bare `loop`.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        on frame {
+            while true {
+                x = x + 1
+                if x == 10 { break }
+            }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0102),
+        "`while true` + break should NOT warn, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_loop_with_only_continue_still_warns() {
+    // `continue` is *not* an exit — the loop still spins forever.
+    // W0102 must still fire here.
+    let errors = analyze_errors(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        on frame {
+            loop {
+                if x == 0 { continue }
+            }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        errors.contains(&ErrorCode::W0102),
+        "`loop` whose only exit is `continue` should still produce W0102, got: {errors:?}"
+    );
+}
+
+// ── W0105: palette universal-byte consistency ───────────────
+
+#[test]
+fn analyze_palette_consistent_universals_ok() {
+    // Flat-form palette where every sub-palette's first byte is
+    // the same universal colour ($0F = black). No W0105.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        palette Consistent {
+            colors: [
+                0x0F, 0x11, 0x12, 0x13,
+                0x0F, 0x21, 0x22, 0x23,
+                0x0F, 0x31, 0x32, 0x33,
+                0x0F, 0x01, 0x02, 0x03,
+                0x0F, 0x05, 0x06, 0x07,
+                0x0F, 0x15, 0x16, 0x17,
+                0x0F, 0x25, 0x26, 0x27,
+                0x0F, 0x35, 0x36, 0x37
+            ]
+        }
+        on frame { wait_frame }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0105),
+        "consistent palette should NOT warn, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_palette_inconsistent_universals_warns() {
+    // Flat-form palette whose sub-palette first bytes disagree
+    // (index 0 = $0F, index 16 = $30, etc.) — the $3F10 mirror
+    // will overwrite the background universal colour at runtime.
+    let errors = analyze_errors(
+        r#"
+        game "T" { mapper: NROM }
+        palette Broken {
+            colors: [
+                0x0F, 0x11, 0x12, 0x13,
+                0x0F, 0x21, 0x22, 0x23,
+                0x0F, 0x31, 0x32, 0x33,
+                0x0F, 0x01, 0x02, 0x03,
+                0x30, 0x05, 0x06, 0x07,
+                0x0F, 0x15, 0x16, 0x17,
+                0x0F, 0x25, 0x26, 0x27,
+                0x0F, 0x35, 0x36, 0x37
+            ]
+        }
+        on frame { wait_frame }
+        start Main
+    "#,
+    );
+    assert!(
+        errors.contains(&ErrorCode::W0105),
+        "inconsistent palette universals should produce W0105, got: {errors:?}"
+    );
+}
+
+#[test]
+fn analyze_grouped_palette_is_always_consistent() {
+    // The grouped form uses the `universal:` field to drive
+    // every sub-palette's first byte, so it can never trip
+    // W0105 — even when the sub-palette bodies differ wildly.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        palette Grouped {
+            universal: 0x0F
+            bg0: [0x11, 0x12, 0x13]
+            bg1: [0x21, 0x22, 0x23]
+            bg2: [0x31, 0x32, 0x33]
+            bg3: [0x01, 0x02, 0x03]
+            sp0: [0x05, 0x06, 0x07]
+            sp1: [0x15, 0x16, 0x17]
+            sp2: [0x25, 0x26, 0x27]
+            sp3: [0x35, 0x36, 0x37]
+        }
+        on frame { wait_frame }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0105),
+        "grouped palette should never trip W0105, got: {:?}",
+        result.diagnostics
+    );
+}
+
+// ── W0106: implicit drop of a function return value ─────────
+
+#[test]
+fn analyze_discarded_non_void_return_warns() {
+    // `double(x)` returns u8 but the caller drops the result.
+    let errors = analyze_errors(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        fun double(n: u8) -> u8 { return n + n }
+        on frame { double(x) }
+        start Main
+    "#,
+    );
+    assert!(
+        errors.contains(&ErrorCode::W0106),
+        "discarded non-void return should produce W0106, got: {errors:?}"
+    );
+}
+
+#[test]
+fn analyze_discarded_void_call_ok() {
+    // Void function at statement position is the happy path —
+    // no discarded value, no warning.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        fun bump() { x = x + 1 }
+        on frame { bump() }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0106),
+        "void call should NOT produce W0106, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_non_void_return_used_as_rhs_ok() {
+    // Same signature as the discarded case, but the return value
+    // is consumed by an assignment — no warning.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        var x: u8 = 0
+        var y: u8 = 0
+        fun double(n: u8) -> u8 { return n + n }
+        on frame { y = double(x) }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0106),
+        "assigned return value should NOT produce W0106, got: {:?}",
+        result.diagnostics
+    );
+}
+
+// ── W0107: `fast` variable slot under-use ───────────────────
+
+#[test]
+fn analyze_fast_var_underused_warns() {
+    // `counter` is declared `fast` but only one read (in the
+    // `if` condition), so its access count is 1 — below the
+    // threshold of 3. W0107 should fire.
+    let errors = analyze_errors(
+        r#"
+        game "T" { mapper: NROM }
+        fast var counter: u8 = 0
+        on frame {
+            if counter == 0 { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        errors.contains(&ErrorCode::W0107),
+        "under-used `fast` var should produce W0107, got: {errors:?}"
+    );
+}
+
+#[test]
+fn analyze_fast_var_heavy_use_ok() {
+    // Three-plus accesses (one init + one read + one write-back)
+    // is enough to justify the slot — no W0107.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        fast var counter: u8 = 0
+        on frame {
+            counter = counter + 1
+            if counter == 0 { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0107),
+        "hot `fast` var should NOT warn, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_non_fast_var_never_warns() {
+    // Only `fast` declarations are checked — a plain `var` with
+    // the same (light) access pattern must not fire W0107.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        var counter: u8 = 0
+        on frame {
+            if counter == 0 { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0107),
+        "plain `var` should never trip W0107, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_fast_var_underscore_exempt() {
+    // Leading-underscore names are exempt from W0107, mirroring
+    // the W0103 convention for deliberately-unused variables.
+    let result = analyze_ok(
+        r#"
+        game "T" { mapper: NROM }
+        fast var _reserved: u8 = 0
+        on frame {
+            if _reserved == 0 { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0107),
+        "underscore-prefixed `fast` var should be exempt, got: {:?}",
+        result.diagnostics
+    );
+}

src/errors/diagnostic.rs

@@ -39,6 +39,9 @@ pub enum ErrorCode {
     W0102, // loop without break or wait_frame
     W0103, // unused variable
     W0104, // unreachable code after terminator, or unreachable state
+    W0105, // palette sub-palette universal mismatch (mirror collision)
+    W0106, // implicit drop of non-void function return value
+    W0107, // `fast` variable rarely accessed (wastes zero-page slot)
 }
 
 impl fmt::Display for ErrorCode {
@@ -62,6 +65,9 @@ impl fmt::Display for ErrorCode {
             Self::W0102 => "W0102",
             Self::W0103 => "W0103",
             Self::W0104 => "W0104",
+            Self::W0105 => "W0105",
+            Self::W0106 => "W0106",
+            Self::W0107 => "W0107",
         };
         write!(f, "{code}")
     }
@@ -70,7 +76,13 @@ impl fmt::Display for ErrorCode {
 impl ErrorCode {
     pub fn level(self) -> Level {
         match self {
-            Self::W0101 | Self::W0102 | Self::W0103 | Self::W0104 => Level::Warning,
+            Self::W0101
+            | Self::W0102
+            | Self::W0103
+            | Self::W0104
+            | Self::W0105
+            | Self::W0106
+            | Self::W0107 => Level::Warning,
             _ => Level::Error,
         }
     }