diff --git a/src/asm/mod.rs b/src/asm/mod.rs index 018f891..2f08f47 100644 --- a/src/asm/mod.rs +++ b/src/asm/mod.rs @@ -145,7 +145,12 @@ impl Assembler { let from = i32::from(self.base_address) + fixup.offset as i32 + 1; let to = i32::from(addr); let offset = to - from; - self.output[fixup.offset] = offset.cast_unsigned().to_le_bytes()[0]; + assert!( + (-128..=127).contains(&offset), + "branch offset {offset} out of range (-128..127) for label '{}'", + fixup.label + ); + self.output[fixup.offset] = (offset as i8).cast_unsigned(); } FixupKind::Lo => { self.output[fixup.offset] = addr as u8; @@ -154,6 +159,8 @@ impl Assembler { self.output[fixup.offset] = (addr >> 8) as u8; } } + } else { + panic!("unresolved label: '{}'", fixup.label); } } } diff --git a/src/asm/tests.rs b/src/asm/tests.rs index c75d473..d296fe7 100644 --- a/src/asm/tests.rs +++ b/src/asm/tests.rs @@ -373,3 +373,29 @@ fn assemble_add_immediate() { let result = assemble(&instructions, 0x8000); assert_eq!(result.bytes, vec![0xA5, 0x10, 0x18, 0x69, 0x02, 0x85, 0x10]); } + +// ── Code review regression tests ── + +#[test] +#[should_panic(expected = "branch offset")] +fn branch_out_of_range_panics() { + // Regression: branch offsets > 127 bytes used to silently wrap + let mut instructions = vec![Instruction::new(NOP, AM::Label("start".into()))]; + // Insert 200 NOPs to push the branch target out of range + for _ in 0..200 { + instructions.push(Instruction::implied(NOP)); + } + instructions.push(Instruction::new(BEQ, AM::LabelRelative("start".into()))); + let _ = assemble(&instructions, 0x8000); +} + +#[test] +#[should_panic(expected = "unresolved label")] +fn unresolved_label_panics() { + // Regression: unresolved labels used to silently produce offset 0 + let instructions = vec![Instruction::new( + BEQ, + AM::LabelRelative("nonexistent".into()), + )]; + let _ = assemble(&instructions, 0x8000); +} diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 0f5415d..dd0682c 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -302,9 +302,18 @@ impl CodeGen { self.emit(LDA, AM::Immediate(u8::from(*v))); } Expr::UnaryOp(UnaryOp::Not, inner, _) => { + // Logical NOT: if condition is nonzero → 0, if zero → 1 self.gen_condition(inner); - self.emit(EOR, AM::Immediate(0xFF)); - self.emit(AND, AM::Immediate(0x01)); + let true_label = self.fresh_label("not_true"); + let end_label = self.fresh_label("not_end"); + self.emit(BEQ, AM::LabelRelative(true_label.clone())); + // Condition was true (nonzero), result is false (0) + self.emit(LDA, AM::Immediate(0)); + self.emit(JMP, AM::Label(end_label.clone())); + // Condition was false (zero), result is true (1) + self.emit_label(&true_label); + self.emit(LDA, AM::Immediate(1)); + self.emit_label(&end_label); } _ => { self.gen_expr(expr); diff --git a/src/parser/mod.rs b/src/parser/mod.rs index be7509d..665a9cc 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -328,7 +328,7 @@ impl Parser { var_type, init, placement, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -347,7 +347,7 @@ impl Parser { name, const_type, value, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -395,7 +395,7 @@ impl Parser { return_type, body, is_inline, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -458,7 +458,7 @@ impl Parser { on_exit, on_frame, on_scanline, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -484,7 +484,7 @@ impl Parser { Ok(BankDecl { name, bank_type, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -538,7 +538,7 @@ impl Parser { Ok(SpriteDecl { name, chr_source, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -597,7 +597,7 @@ impl Parser { Ok(PaletteDecl { name, colors, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -638,7 +638,7 @@ impl Parser { Ok(BackgroundDecl { name, chr_source, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -715,7 +715,7 @@ impl Parser { Ok(Block { statements, - span: Span::new(start.file_id, start.start, self.current_span().start), + span: Span::new(start.file_id, start.start, self.current_span().end), }) } @@ -981,7 +981,7 @@ impl Parser { // Function call self.advance(); let mut args = Vec::new(); - while *self.peek() != TokenKind::RParen { + while *self.peek() != TokenKind::RParen && *self.peek() != TokenKind::Eof { args.push(self.parse_expr()?); if *self.peek() == TokenKind::Comma { self.advance(); @@ -1015,6 +1015,18 @@ impl Parser { self.advance(); Ok(AssignOp::MinusAssign) } + TokenKind::AmpAssign => { + self.advance(); + Ok(AssignOp::AmpAssign) + } + TokenKind::PipeAssign => { + self.advance(); + Ok(AssignOp::PipeAssign) + } + TokenKind::CaretAssign => { + self.advance(); + Ok(AssignOp::CaretAssign) + } _ => Err(Diagnostic::error( ErrorCode::E0201, format!("expected assignment operator, found '{}'", self.peek()), @@ -1266,7 +1278,7 @@ impl Parser { if *self.peek() == TokenKind::LParen { self.advance(); let mut args = Vec::new(); - while *self.peek() != TokenKind::RParen { + while *self.peek() != TokenKind::RParen && *self.peek() != TokenKind::Eof { args.push(self.parse_expr()?); if *self.peek() == TokenKind::Comma { self.advance(); diff --git a/src/parser/tests.rs b/src/parser/tests.rs index 4b4992a..aed4312 100644 --- a/src/parser/tests.rs +++ b/src/parser/tests.rs @@ -915,3 +915,31 @@ fn lexer_no_panic_on_garbage() { let _ = lex(input); // must not panic } } + +// ── Code review regression tests ── + +#[test] +fn unterminated_call_args_no_hang() { + // Regression: parser used to infinite-loop on missing RParen in call args + let _ = parse(r#"game "T" { mapper: NROM } on frame { foo(1, 2 } start Main"#); + let _ = parse(r#"game "T" { mapper: NROM } var x: u8 = foo(1"#); + // Must return (possibly with errors), not hang +} + +#[test] +fn array_index_compound_assign() { + // Regression: parse_assign_op was missing &= |= ^= for array elements + let src = r#" + game "T" { mapper: NROM } + var buf: u8[4] = [0, 0, 0, 0] + on frame { + buf[0] &= 0x0F + buf[1] |= 0x80 + buf[2] ^= 0xFF + } + start Main + "#; + let prog = parse_ok(src); + let frame = prog.states[0].on_frame.as_ref().unwrap(); + assert_eq!(frame.statements.len(), 3); +}