diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index d855cd8..e7ae770 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -255,7 +255,34 @@ impl Analyzer { } self.walk_expr_reads(idx); } - Expr::BinaryOp(lhs, _, rhs, _) => { + Expr::BinaryOp(lhs, op, rhs, span) => { + // W0101: warn about multiply/divide/modulo with a non- + // constant operand. These lower to calls into the + // software multiply/divide routines, which are far more + // expensive than the simple inline opcodes used for + // add/sub. A literal like `x * 2` can be strength- + // reduced to a shift and is therefore cheap. + if matches!(op, BinOp::Mul | BinOp::Div | BinOp::Mod) + && !is_small_constant(lhs) + && !is_small_constant(rhs) + { + let op_name = match op { + BinOp::Mul => "multiply", + BinOp::Div => "divide", + BinOp::Mod => "modulo", + _ => unreachable!(), + }; + self.diagnostics.push( + Diagnostic::warning( + ErrorCode::W0101, + format!("{op_name} with two non-constant operands is expensive"), + *span, + ) + .with_help( + "consider precomputing or using a power-of-2 constant for strength reduction", + ), + ); + } self.walk_expr_reads(lhs); self.walk_expr_reads(rhs); } @@ -930,6 +957,13 @@ fn collect_calls_block(block: &Block, calls: &mut Vec) { /// unbounded `loop { }` is actually an infinite spin. We recurse into /// nested control-flow blocks so that a `break` inside a conditional /// body still counts as "can exit". +/// True if the expression is a small integer literal — used to avoid +/// emitting W0101 for multiply/divide where at least one operand can be +/// handled by strength reduction (e.g. `x * 2`, `x / 4`). +fn is_small_constant(expr: &Expr) -> bool { + matches!(expr, Expr::IntLiteral(_, _)) +} + fn block_can_exit_or_yield(block: &Block) -> bool { block.statements.iter().any(stmt_can_exit_or_yield) } diff --git a/src/analyzer/tests.rs b/src/analyzer/tests.rs index b711e1c..cd8d768 100644 --- a/src/analyzer/tests.rs +++ b/src/analyzer/tests.rs @@ -298,6 +298,38 @@ fn analyze_return_wrong_type() { ); } +#[test] +fn analyze_expensive_multiply_warns() { + let errors = analyze_errors( + r#" + game "Test" { mapper: NROM } + var a: u8 = 3 + var b: u8 = 5 + var c: u8 = 0 + on frame { c = a * b } + start Main + "#, + ); + assert!( + errors.contains(&ErrorCode::W0101), + "variable*variable multiply should emit W0101, got: {errors:?}" + ); +} + +#[test] +fn analyze_multiply_by_constant_ok() { + // Multiply by a literal is cheap (strength reduced to shifts). + analyze_ok( + r#" + game "Test" { mapper: NROM } + var a: u8 = 3 + var c: u8 = 0 + on frame { c = a * 4 } + start Main + "#, + ); +} + #[test] fn analyze_on_scanline_requires_mmc3() { let errors = analyze_errors( diff --git a/src/errors/diagnostic.rs b/src/errors/diagnostic.rs index 6f207cc..e03ff46 100644 --- a/src/errors/diagnostic.rs +++ b/src/errors/diagnostic.rs @@ -42,7 +42,6 @@ pub enum ErrorCode { E0505, // multiple start declarations // W01xx: Warnings - #[allow(dead_code)] W0101, // expensive multiply/divide operation W0102, // loop without break or wait_frame W0103, // unused variable