diff --git a/LANGREF.md b/LANGREF.md index 7234678..5446ae3 100644 --- a/LANGREF.md +++ b/LANGREF.md @@ -69,6 +69,16 @@ Multiple statements can appear on one line separated by colons: A = 1 : B = 2 : PRINT A + B ``` +### Line Continuation + +A trailing underscore joins a statement to the next line. Nothing may follow it +on the line it ends: + +```basic +Total = Price * Quantity + _ + Shipping +``` + ### Block Terminators Each multi-line block may be closed with either the two-word form or a single @@ -260,13 +270,21 @@ case-sensitive and a prefix sorts before the longer string (`"ab" < "abc"`). | `XOR` | Bitwise/logical XOR | | `NOT` | Bitwise/logical NOT | -These operate bitwise on integers, allowing both logical tests and bit manipulation: +These operate bitwise on integers, allowing both logical tests and bit +manipulation. Their operands are converted to integers first, and the result is +an integer: ```basic IF A > 0 AND B > 0 THEN PRINT "Both positive" Flags% = Flags% OR &H01 ' Set bit 0 +Mask% = NOT &H00FF ' -256: every bit flipped ``` +`NOT` complements every bit, so `NOT 1` is `-2`, which is non-zero and therefore +*true*. This matters only when testing a value that is not already a truth +value: comparisons yield -1 or 0, and `NOT` maps those to each other, so +`IF NOT (A > 0)` behaves as expected while `IF NOT 1` does not. + ### String Concatenation ```basic @@ -287,6 +305,9 @@ From highest to lowest: Because `^` binds tighter than unary negation, `-2 ^ 2` is `-(2 ^ 2)` = -4. +Operators of equal precedence associate left to right, `^` included: `2 ^ 3 ^ 2` +is `(2 ^ 3) ^ 2` = 64, and `100 - 10 - 5` is 85. + Use parentheses to override precedence: ```basic Result = (A + B) * C @@ -382,10 +403,24 @@ Read user input: ```basic INPUT X ' Prompt with "? " -INPUT "Enter name: ", N$ ' Custom prompt +INPUT "Enter name: "; N$ ' Prints: Enter name: ? +INPUT "Enter name: ", N$ ' Prints: Enter name: INPUT "X, Y: ", X, Y ' Multiple values ``` +The separator decides the question mark: a `;` after the prompt adds `? `, a +`,` suppresses it, and a prompt-less `INPUT` prints `? ` on its own. + +A `;` *before* the prompt is accepted and ignored: + +```basic +INPUT ; "Enter name: "; N$ +``` + +In GW-BASIC it suppressed the newline echoed when the operator pressed Return. +That newline comes from the terminal here rather than from the program, so +there is nothing for it to suppress. + ### LINE INPUT Read entire line as string (no parsing): @@ -394,6 +429,9 @@ Read entire line as string (no parsing): LINE INPUT "Enter text: ", Text$ ``` +`LINE INPUT` never adds a question mark; write one into the prompt if you want +one. + ### IF...THEN...ELSE **Single-line form:** @@ -402,6 +440,23 @@ IF X > 0 THEN PRINT "Positive" IF X > 0 THEN Y = 1 ELSE Y = 0 ``` +Both branches take a list of statements separated by colons. Everything after +`THEN` up to `ELSE` or the end of the line is conditional, and everything after +`ELSE` is too: + +```basic +IF X > 0 THEN Y = 1 : PRINT "Positive" ELSE Y = 0 : PRINT "Not positive" +``` + +A bare line number after `THEN` or `ELSE` is an implied `GOTO`: + +```basic +10 IF X < 0 THEN 90 +20 IF X = 0 THEN 90 ELSE 80 +80 PRINT "Positive" +90 PRINT "Done" +``` + **Block form:** ```basic IF X > 0 THEN @@ -454,13 +509,24 @@ FOR K = 0 TO 1 STEP 0.1 NEXT K ``` -The loop variable name after `NEXT` is optional: +The loop variable name after `NEXT` is optional, and a bare `NEXT` closes the +innermost open loop: ```basic FOR I = 1 TO 10 PRINT I NEXT ``` +If the name *is* given it must be the one that loop counts, so `FOR I ... NEXT J` +is an error rather than a loop closed by surprise. One `NEXT` may close several +nested loops, innermost first: +```basic +FOR I = 1 TO 3 + FOR J = 1 TO 3 + PRINT I * J +NEXT J, I +``` + ### WHILE...WEND Pre-test loop: @@ -577,6 +643,18 @@ RESTORE ' Reset data pointer to beginning RESTORE 100 ' Resume at the DATA on line 100 ``` +A DATA item needs quotes only if it contains a comma, a colon, or spaces that +matter. Otherwise write it plainly; surrounding spaces are trimmed and the text +is taken exactly as written, case included. An omitted item reads as 0 or `""`: + +```basic +DATA hello, World, "a,b", " padded " +DATA 1,,3 +``` + +A colon ends a DATA statement, so another statement may follow it on the same +line. + ### CLS Clear screen: @@ -925,7 +1003,10 @@ blanks and line breaks, so several fields may come from one line and one field may span several. A field wrapped in quotes may contain commas. `LINE INPUT #` takes a whole line, commas and all. -`EOF()` gives the usual read-until-the-end loop: +Reading past the end of a file is an error (`Input past end of file`), as is +opening a file that is not there (`File not found`) or re-using a file number +that is still open (`File already open`). `EOF()` gives the usual +read-until-the-end loop: ```basic OPEN "data.txt" FOR INPUT AS #1 @@ -1080,9 +1161,13 @@ END SUB ' Call the subroutine PrintGreeting "World" -PrintGreeting("World") ' Parentheses optional +PrintGreeting("World") ' Parentheses optional +CALL PrintGreeting("World") ' CALL is accepted too ``` +`CALL` is recognised only before a name at the start of a statement, so a +program may still use it as a variable. + ### FUNCTION Procedures that return a value: diff --git a/src/codegen.rs b/src/codegen.rs index 73ab9ec..8206751 100644 --- a/src/codegen.rs +++ b/src/codegen.rs @@ -204,13 +204,30 @@ // SPDX-License-Identifier: MIT use crate::abi::{Abi, PlatformAbi}; +use crate::lexer::normalized; use crate::parser::TypeRef; use crate::parser::*; use crate::sema::{Scope as SemaScope, Symbols}; use crate::using; use std::collections::{BTreeMap, HashMap}; +use std::fmt::Write as _; use std::sync::LazyLock; +/// Emit one formatted instruction straight into the output buffer. +/// +/// The spelling this replaces was `self.emit(&format!(...))`, which built a +/// throwaway `String` for every instruction only to copy it into `output` and +/// drop it. A 200k-line BASIC program assembles to five million lines, so that +/// was five million allocations spent handing text to `push_str`. +/// +/// `writeln!` into a `String` cannot fail -- `fmt::Write` for `String` is +/// infallible -- so the result is discarded rather than unwrapped. +macro_rules! emit { + ($self:expr, $($arg:tt)*) => {{ + let _ = writeln!($self.output, $($arg)*); + }}; +} + /// Simple math functions: BASIC name -> libc function name static LIBC_MATH_FNS: LazyLock> = LazyLock::new(|| { HashMap::from([ @@ -635,7 +652,7 @@ impl CodeGen { fn emit_arg_reg(&mut self, arg_n: usize, src_reg: &str) { let dst = Self::arg_reg(arg_n); if dst != src_reg { - self.emit(&format!(" mov {}, {}", dst, src_reg)); + emit!(self, " mov {}, {}", dst, src_reg); } } @@ -666,16 +683,16 @@ impl CodeGen { fn emit_arg_imm(&mut self, arg_n: usize, value: i64) { let dst = Self::arg_reg(arg_n); if (0..=u32::MAX as i64).contains(&value) { - self.emit(&format!(" mov {}, {}", Self::reg32(dst), value)); + emit!(self, " mov {}, {}", Self::reg32(dst), value); } else { - self.emit(&format!(" mov {}, {}", dst, value)); + emit!(self, " mov {}, {}", dst, value); } } /// Emit a lea instruction to set up an integer argument from a memory reference fn emit_arg_lea(&mut self, arg_n: usize, mem: &str) { let dst = Self::arg_reg(arg_n); - self.emit(&format!(" lea {}, {}", dst, mem)); + emit!(self, " lea {}, {}", dst, mem); } /// Call a libc function, whose arguments are already in place. @@ -704,23 +721,23 @@ impl CodeGen { bytes => (bytes + 15) / 16 * 16, }; if reserve > 0 { - self.emit(&format!(" sub rsp, {}", reserve)); + emit!(self, " sub rsp, {}", reserve); } for (i, src) in srcs.iter().enumerate().rev() { match regs.get(i) { Some(reg) if reg == src => {} // already there - Some(reg) => self.emit(&format!(" mov {}, {}", reg, src)), + Some(reg) => emit!(self, " mov {}, {}", reg, src), None => { let off = shadow + (i - regs.len()) as i32 * 8; - self.emit(&format!(" mov QWORD PTR [rsp + {}], {}", off, src)); + emit!(self, " mov QWORD PTR [rsp + {}], {}", off, src); } } } - self.emit(&format!(" call {}", sym)); + emit!(self, " call {}", sym); if reserve > 0 { - self.emit(&format!(" add rsp, {}", reserve)); + emit!(self, " add rsp, {}", reserve); } } @@ -818,7 +835,7 @@ impl CodeGen { return; } let operand = self.f64_operand(value); - self.emit(&format!(" movsd xmm0, {}", operand)); + emit!(self, " movsd xmm0, {}", operand); } /// The constant value of a numeric literal, for the fast paths that need @@ -835,7 +852,7 @@ impl CodeGen { fn const_double(&self, expr: &Expr) -> Option { let lit = match expr { Expr::Literal(lit) => lit, - Expr::Variable(name) => self.symbols.consts.get(&name.to_uppercase())?, + Expr::Variable(name) => self.symbols.consts.get(normalized(name))?, Expr::Unary { op: UnaryOp::Neg, operand, @@ -857,7 +874,7 @@ impl CodeGen { fn const_i32(&self, expr: &Expr) -> Option { let lit = match expr { Expr::Literal(lit) => lit, - Expr::Variable(name) => self.symbols.consts.get(&name.to_uppercase())?, + Expr::Variable(name) => self.symbols.consts.get(normalized(name))?, // `checked_neg` rather than `-`: negating i32::MIN is not an i32, // and the general path handles it correctly. Expr::Unary { @@ -1081,7 +1098,7 @@ impl CodeGen { Expr::Variable(name) if crate::sema::is_zero_arg_builtin(name) => { self.fn_return_type(name) } - Expr::Variable(name) => match self.symbols.consts.get(&name.to_uppercase()) { + Expr::Variable(name) => match self.symbols.consts.get(normalized(name)) { Some(Literal::Integer(_)) => DataType::Long, Some(Literal::Float(_)) => DataType::Double, Some(Literal::String(_)) => DataType::String, @@ -1101,6 +1118,11 @@ impl CodeGen { Expr::ArrayAccess { name, .. } => DataType::from_suffix(name), Expr::FnCall { name, args } => self.call_return_type(name, args), Expr::Field { .. } => self.field_expr_type(expr), + // Unary minus keeps its operand's type; NOT is a bitwise operator + // and yields an integer, exactly as AND, OR and XOR do. + Expr::Unary { + op: UnaryOp::Not, .. + } => DataType::Long, Expr::Unary { operand, .. } => self.expr_type(operand), Expr::Binary { left, right, op } => { let lt = self.expr_type(left); @@ -1138,7 +1160,7 @@ impl CodeGen { match self .symbols .procs - .get(&name.to_uppercase()) + .get(normalized(name)) .and_then(|p| p.ret_ty.as_ref()) { Some(ty) => DataType::from_type_ref(ty), @@ -1198,6 +1220,19 @@ impl CodeGen { return DataType::Long; } + // The bitwise operators convert both operands to integers and produce + // an integer, exactly as `\` and MOD above do. + // + // Without this they fell through to ordinary numeric promotion and came + // out Double whenever either operand was -- which is what an unsuffixed + // variable is. Codegen still emitted `and eax, ecx`, so the answer sat + // in EAX while every consumer read xmm0 and found the *left operand* + // still there: `A = 12 : B = 10 : PRINT A AND B` printed 12. Literal + // operands are folded before reaching here, which is why it hid. + if matches!(op, BinaryOp::And | BinaryOp::Or | BinaryOp::Xor) { + return DataType::Long; + } + // Power (^) always produces Double (uses libm pow()) if op == BinaryOp::Pow { return DataType::Double; @@ -1329,7 +1364,10 @@ impl CodeGen { // Emit data section self.emit_data_section(); - self.output.clone() + // Handed over rather than cloned. The buffer is the whole program's + // assembly -- 145 MB for a 200k-line source -- so copying it to return + // it doubled the compiler's peak memory for nothing. + std::mem::take(&mut self.output) } /// Reserve descriptors for every array declared in `scope`. @@ -1380,10 +1418,11 @@ impl CodeGen { // Initialize GOSUB return stack if needed if self.gosub_used { self.emit(" # Initialize GOSUB return stack"); - self.emit(&format!( + emit!( + self, " lea rax, [rip + _gosub_stack + {}]", GOSUB_STACK_SIZE - )); // Point to end (stack grows down) + ); // Point to end (stack grows down) self.emit(" mov QWORD PTR [rip + _gosub_sp], rax"); } @@ -1462,6 +1501,57 @@ impl CodeGen { self.emit_check(cond, RtError::Domain); } + /// Refuse a `^` that has no real answer. + /// + /// `pow` returns NaN for a negative base raised to a fractional exponent, + /// which printed as `-nan`. Testing the result rather than the operands is + /// both cheaper and exact: `ucomisd` sets the parity flag for an unordered + /// compare, and a value is unordered with itself only when it is NaN. + fn emit_pow_domain_check(&mut self) { + if !self.opts.checks { + return; + } + self.emit(" ucomisd xmm0, xmm0"); + self.emit_check("jp", RtError::Domain); + } + + /// Refuse a builtin argument below `min`, as GW-BASIC's "Illegal function + /// call" does. + /// + /// The argument is expected in `reg`, already widened to 64 bits. A signed + /// comparison matters: `_rt_left` and friends compare the count against the + /// length *unsigned*, so a negative one reads as enormous, clamps to the + /// length and returns the whole string rather than failing. + fn emit_arg_min_check(&mut self, reg: &str, min: i64) { + self.emit_arg_range_check(reg, min, None); + } + + /// The same, with an upper bound as well. + fn emit_arg_range_check(&mut self, reg: &str, min: i64, max: Option) { + if !self.opts.checks { + return; + } + emit!(self, " cmp {}, {}", reg, min); + self.emit_check("jl", RtError::Domain); + if let Some(max) = max { + emit!(self, " cmp {}, {}", reg, max); + self.emit_check("jg", RtError::Domain); + } + } + + /// The range a table-driven `CallLong` builtin accepts, if it is narrower + /// than a Long. HEX$ and OCT$ take any value; the others do not. + fn long_arg_range(name: &str) -> Option<(i64, Option)> { + match name { + // A character code, not a byte: CHR$ used to keep only the low one, + // so CHR$(256) was CHR$(0) and CHR$(-1) was CHR$(255). + "CHR$" => Some((0, Some(255))), + // A count of spaces. Negative used to clamp to the empty string. + "SPACE$" => Some((0, None)), + _ => None, + } + } + /// Guard an integer divide: `idiv` raises #DE (a SIGFPE crash) both when /// the divisor is zero and for INT_MIN / -1, which overflows the quotient. /// The divisor is expected in `ecx`. @@ -1474,7 +1564,7 @@ impl CodeGen { // INT_MIN / -1 overflows; both operands must match for it to trap. self.emit(" cmp ecx, -1"); let skip = self.new_label("nodivovf"); - self.emit(&format!(" jne {}", skip)); + emit!(self, " jne {}", skip); self.emit(" cmp eax, -2147483648"); self.emit_check("je", RtError::Overflow); self.emit_label(&skip); @@ -1499,7 +1589,7 @@ impl CodeGen { if self.symbols.option_base == 0 { return; } - self.emit(&format!(" cmp {}, {}", reg, self.symbols.option_base)); + emit!(self, " cmp {}, {}", reg, self.symbols.option_base); self.emit_check("jb", RtError::Subscript); } @@ -1510,7 +1600,7 @@ impl CodeGen { fn const_dim(&self, e: &Expr) -> Option { let lit = match e { Expr::Literal(l) => l.clone(), - Expr::Variable(n) => self.symbols.consts.get(&n.to_uppercase())?.clone(), + Expr::Variable(n) => self.symbols.consts.get(normalized(n))?.clone(), _ => return None, }; match lit { @@ -1748,8 +1838,8 @@ impl CodeGen { } for (reg, mem) in regs { self.emit(" sub rsp, 16 # save a descriptor register"); - self.emit(&format!(" mov QWORD PTR [rsp], {}", reg)); - self.emit(&format!(" mov {}, {}", reg, mem)); + emit!(self, " mov QWORD PTR [rsp], {}", reg); + emit!(self, " mov {}, {}", reg, mem); } } @@ -1763,7 +1853,7 @@ impl CodeGen { regs.push(reg.clone()); } for reg in regs.into_iter().rev() { - self.emit(&format!(" mov {}, QWORD PTR [rsp]", reg)); + emit!(self, " mov {}, QWORD PTR [rsp]", reg); self.emit(" add rsp, 16"); } } @@ -1777,7 +1867,7 @@ impl CodeGen { if let Some(reg) = self.hoisted_base(name) { return reg; } - self.emit(&format!(" mov r10, {}", loc.q(0))); + emit!(self, " mov r10, {}", loc.q(0)); "r10".to_string() } @@ -1979,19 +2069,19 @@ impl CodeGen { match ct { DataType::Integer => { let r = if second { "ecx" } else { "eax" }; - self.emit(&format!(" movsx {}, {}", r, mem)); + emit!(self, " movsx {}, {}", r, mem); } DataType::Long => { let r = if second { "ecx" } else { "eax" }; - self.emit(&format!(" mov {}, {}", r, mem)); + emit!(self, " mov {}, {}", r, mem); } DataType::Single => { let r = if second { "xmm1" } else { "xmm0" }; - self.emit(&format!(" movss {}, {}", r, mem)); + emit!(self, " movss {}, {}", r, mem); } _ => { let r = if second { "xmm1" } else { "xmm0" }; - self.emit(&format!(" movsd {}, {}", r, mem)); + emit!(self, " movsd {}, {}", r, mem); } } } @@ -2002,10 +2092,10 @@ impl CodeGen { /// same as every other INTEGER assignment in the language. fn emit_for_store(&mut self, ct: DataType, mem: &str) { match ct { - DataType::Integer => self.emit(&format!(" mov {}, ax", mem)), - DataType::Long => self.emit(&format!(" mov {}, eax", mem)), - DataType::Single => self.emit(&format!(" movss {}, xmm0", mem)), - _ => self.emit(&format!(" movsd {}, xmm0", mem)), + DataType::Integer => emit!(self, " mov {}, ax", mem), + DataType::Long => emit!(self, " mov {}, eax", mem), + DataType::Single => emit!(self, " movss {}, xmm0", mem), + _ => emit!(self, " movsd {}, xmm0", mem), } } @@ -2053,16 +2143,16 @@ impl CodeGen { (DataType::Integer, ForOperand::Slot(off)) => { // The slot holds sixteen bits; the counter in eax is already // sign-extended, so the limit has to be too before they meet. - self.emit(&format!(" movsx ecx, WORD PTR [rbp + {}]", off)); + emit!(self, " movsx ecx, WORD PTR [rbp + {}]", off); self.emit(" cmp eax, ecx"); } (DataType::Integer | DataType::Long, _) => { - self.emit(&format!(" cmp eax, {}", operand.text(ct))); + emit!(self, " cmp eax, {}", operand.text(ct)); } (DataType::Single, _) => { - self.emit(&format!(" ucomiss xmm0, {}", operand.text(ct))); + emit!(self, " ucomiss xmm0, {}", operand.text(ct)); } - _ => self.emit(&format!(" ucomisd xmm0, {}", operand.text(ct))), + _ => emit!(self, " ucomisd xmm0, {}", operand.text(ct)), } } @@ -2076,17 +2166,17 @@ impl CodeGen { fn emit_for_add(&mut self, ct: DataType, operand: &ForOperand) { match ct { DataType::Integer => { - self.emit(&format!(" add ax, {}", operand.text(ct))); + emit!(self, " add ax, {}", operand.text(ct)); self.emit_check("jo", RtError::Overflow); } DataType::Long => { - self.emit(&format!(" add eax, {}", operand.text(ct))); + emit!(self, " add eax, {}", operand.text(ct)); self.emit_check("jo", RtError::Overflow); } DataType::Single => { - self.emit(&format!(" addss xmm0, {}", operand.text(ct))); + emit!(self, " addss xmm0, {}", operand.text(ct)); } - _ => self.emit(&format!(" addsd xmm0, {}", operand.text(ct))), + _ => emit!(self, " addsd xmm0, {}", operand.text(ct)), } } @@ -2100,22 +2190,22 @@ impl CodeGen { match ct { // Narrowed on the way in, exactly as the store to a 16-bit slot // would have narrowed it. - DataType::Integer => self.emit(&format!(" movsx {}, ax", Self::counter_reg32(reg))), - DataType::Long => self.emit(&format!(" mov {}, eax", Self::counter_reg32(reg))), + DataType::Integer => emit!(self, " movsx {}, ax", Self::counter_reg32(reg)), + DataType::Long => emit!(self, " mov {}, eax", Self::counter_reg32(reg)), // movaps rather than movss: a register-to-register movss merges // into the destination, so it would depend on what was there. - DataType::Single => self.emit(&format!(" movaps {}, xmm0", reg)), - _ => self.emit(&format!(" movapd {}, xmm0", reg)), + DataType::Single => emit!(self, " movaps {}, xmm0", reg), + _ => emit!(self, " movapd {}, xmm0", reg), } } /// Write a promoted counter back to the variable's storage. fn emit_counter_writeback(&mut self, ct: DataType, reg: &str, mem: &str) { match ct { - DataType::Integer => self.emit(&format!(" mov {}, {}w", mem, reg)), - DataType::Long => self.emit(&format!(" mov {}, {}", mem, Self::counter_reg32(reg))), - DataType::Single => self.emit(&format!(" movss {}, {}", mem, reg)), - _ => self.emit(&format!(" movsd {}, {}", mem, reg)), + DataType::Integer => emit!(self, " mov {}, {}w", mem, reg), + DataType::Long => emit!(self, " mov {}, {}", mem, Self::counter_reg32(reg)), + DataType::Single => emit!(self, " movss {}, {}", mem, reg), + _ => emit!(self, " movsd {}, {}", mem, reg), } } @@ -2123,20 +2213,20 @@ impl CodeGen { fn emit_counter_compare(&mut self, ct: DataType, reg: &str, operand: &ForOperand) { match (ct, operand) { (DataType::Integer, ForOperand::Slot(off)) => { - self.emit(&format!(" movsx ecx, WORD PTR [rbp + {}]", off)); - self.emit(&format!(" cmp {}, ecx", Self::counter_reg32(reg))); + emit!(self, " movsx ecx, WORD PTR [rbp + {}]", off); + emit!(self, " cmp {}, ecx", Self::counter_reg32(reg)); } (DataType::Integer | DataType::Long, _) => { let text = operand.text(ct); - self.emit(&format!(" cmp {}, {}", Self::counter_reg32(reg), text)); + emit!(self, " cmp {}, {}", Self::counter_reg32(reg), text); } (DataType::Single, _) => { let text = operand.text(ct); - self.emit(&format!(" ucomiss {}, {}", reg, text)); + emit!(self, " ucomiss {}, {}", reg, text); } _ => { let text = operand.text(ct); - self.emit(&format!(" ucomisd {}, {}", reg, text)); + emit!(self, " ucomisd {}, {}", reg, text); } } } @@ -2147,40 +2237,38 @@ impl CodeGen { let text = operand.text(ct); match ct { DataType::Integer => { - self.emit(&format!(" add {}w, {}", reg, text)); + emit!(self, " add {}w, {}", reg, text); self.emit_check("jo", RtError::Overflow); // The narrowing store used to keep an INTEGER counter within // sixteen bits for free; in a register it has to be said. - self.emit(&format!(" movsx {}, {}w", r32, reg)); + emit!(self, " movsx {}, {}w", r32, reg); } DataType::Long => { - self.emit(&format!(" add {}, {}", r32, text)); + emit!(self, " add {}, {}", r32, text); self.emit_check("jo", RtError::Overflow); } - DataType::Single => self.emit(&format!(" addss {}, {}", reg, text)), - _ => self.emit(&format!(" addsd {}, {}", reg, text)), + DataType::Single => emit!(self, " addss {}, {}", reg, text), + _ => emit!(self, " addsd {}, {}", reg, text), } } /// Load a promoted variable's storage into its register, entering a loop. fn emit_promotion_load(&mut self, p: &Promoted) { match p.ty { - DataType::Integer => self.emit(&format!( + DataType::Integer => emit!( + self, " movsx {}, {}", Self::counter_reg32(&p.reg), p.loc.at("WORD PTR", 0) - )), - DataType::Long => self.emit(&format!( + ), + DataType::Long => emit!( + self, " mov {}, {}", Self::counter_reg32(&p.reg), p.loc.at("DWORD PTR", 0) - )), - DataType::Single => self.emit(&format!( - " movss {}, {}", - p.reg, - p.loc.at("DWORD PTR", 0) - )), - _ => self.emit(&format!(" movsd {}, {}", p.reg, p.loc.q(0))), + ), + DataType::Single => emit!(self, " movss {}, {}", p.reg, p.loc.at("DWORD PTR", 0)), + _ => emit!(self, " movsd {}, {}", p.reg, p.loc.q(0)), } } @@ -2202,10 +2290,10 @@ impl CodeGen { fn emit_counter_read(&mut self, ct: DataType, reg: &str) { match ct { DataType::Integer | DataType::Long => { - self.emit(&format!(" mov eax, {}", Self::counter_reg32(reg))) + emit!(self, " mov eax, {}", Self::counter_reg32(reg)) } - DataType::Single => self.emit(&format!(" movaps xmm0, {}", reg)), - _ => self.emit(&format!(" movapd xmm0, {}", reg)), + DataType::Single => emit!(self, " movaps xmm0, {}", reg), + _ => emit!(self, " movapd xmm0, {}", reg), } } @@ -2218,20 +2306,20 @@ impl CodeGen { let mem = operand.text(ct); match ct { DataType::Integer | DataType::Long => { - self.emit(&format!(" cmp {}, 0", mem)); - self.emit(&format!(" jl {}", target)); + emit!(self, " cmp {}, 0", mem); + emit!(self, " jl {}", target); } DataType::Single => { - self.emit(&format!(" movss xmm1, {}", mem)); + emit!(self, " movss xmm1, {}", mem); self.emit(" xorps xmm2, xmm2"); self.emit(" ucomiss xmm1, xmm2"); - self.emit(&format!(" jb {}", target)); + emit!(self, " jb {}", target); } _ => { - self.emit(&format!(" movsd xmm1, {}", mem)); + emit!(self, " movsd xmm1, {}", mem); self.emit(" xorpd xmm2, xmm2"); self.emit(" ucomisd xmm1, xmm2"); - self.emit(&format!(" jb {}", target)); + emit!(self, " jb {}", target); } } } @@ -2257,7 +2345,7 @@ impl CodeGen { self.gen_coercion(ty, DataType::Long); self.emit(" movsxd rax, eax"); self.emit(" dec rax"); - self.emit(&format!(" cmp rax, {}", rank)); + emit!(self, " cmp rax, {}", rank); self.emit_check("jae", RtError::Subscript); self.emit(" inc rax"); } @@ -2279,7 +2367,7 @@ impl CodeGen { return; } let label = self.error_label(kind); - self.emit(&format!(" {} {}", cond, label)); + emit!(self, " {} {}", cond, label); } /// Emit every error trampoline collected during code generation. @@ -2293,8 +2381,8 @@ impl CodeGen { for ((kind, line), label) in sites { self.emit_label(&label); let sym = kind.symbol(); - self.emit(&format!(" lea {}, [rip + {}]", Self::arg_reg(0), sym)); - self.emit(&format!(" mov {}, {}", Self::arg_reg(1), line)); + emit!(self, " lea {}, [rip + {}]", Self::arg_reg(0), sym); + emit!(self, " mov {}, {}", Self::arg_reg(1), line); self.emit(" call _rt_error"); } } @@ -2316,13 +2404,13 @@ impl CodeGen { self.emit(" # zero locals: [rsp, rbp)"); self.emit(" mov r11, rsp"); self.emit(" mov r10, rbp"); - self.emit(&format!(" jmp {}", check)); + emit!(self, " jmp {}", check); self.emit_label(&body); self.emit(" mov QWORD PTR [r11], 0"); self.emit(" add r11, 8"); self.emit_label(&check); self.emit(" cmp r11, r10"); - self.emit(&format!(" jb {}", body)); + emit!(self, " jb {}", body); } fn gen_procedure(&mut self, name: &str, params: &[Param], body: &[Stmt], is_function: bool) { @@ -2372,17 +2460,14 @@ impl CodeGen { let src = match place.ptr { Slot::Reg(i) => int_regs[i].to_string(), Slot::Stk(i) => { - self.emit(&format!( - " mov r11, QWORD PTR [rbp + {}]", - 16 + 8 * i as i32 - )); + emit!(self, " mov r11, QWORD PTR [rbp + {}]", 16 + 8 * i as i32); "r11".to_string() } }; - self.emit(&format!(" mov r10, {}", src)); + emit!(self, " mov r10, {}", src); for w in 0..words { - self.emit(&format!(" mov rax, QWORD PTR [r10 + {}]", w * 8)); - self.emit(&format!(" mov {}, rax", loc.q(w))); + emit!(self, " mov rax, QWORD PTR [r10 + {}]", w * 8); + emit!(self, " mov {}, rax", loc.q(w)); } self.proc_record_vars.insert(param.clone(), loc); self.proc_types.insert(param.clone(), ty); @@ -2418,33 +2503,33 @@ impl CodeGen { match place.ty { DataType::String => { let p = fetch(self, place.ptr); - self.emit(&format!(" mov {}, {}", loc.q(0), p)); + emit!(self, " mov {}, {}", loc.q(0), p); let l = fetch(self, place.len.expect("string parameter has a length slot")); - self.emit(&format!(" mov {}, {}", loc.q(1), l)); + emit!(self, " mov {}, {}", loc.q(1), l); } DataType::Double => { let p = fetch(self, place.ptr); - self.emit(&format!(" mov {}, {}", loc.q(0), p)); + emit!(self, " mov {}, {}", loc.q(0), p); } // Numeric arguments arrive as f64 bit patterns; narrow to the // declared type. rax/xmm0 are safe scratch: neither is an // argument register on either ABI. DataType::Single => { let p = fetch(self, place.ptr); - self.emit(&format!(" movq xmm0, {}", p)); + emit!(self, " movq xmm0, {}", p); self.emit(" cvtsd2ss xmm0, xmm0"); - self.emit(&format!(" movss {}, xmm0", loc.at("DWORD PTR", 0))); + emit!(self, " movss {}, xmm0", loc.at("DWORD PTR", 0)); } DataType::Integer | DataType::Long => { let p = fetch(self, place.ptr); - self.emit(&format!(" movq xmm0, {}", p)); + emit!(self, " movq xmm0, {}", p); self.emit(" cvttsd2si eax, xmm0"); let (size, reg) = if place.ty == DataType::Integer { ("WORD PTR", "ax") } else { ("DWORD PTR", "eax") }; - self.emit(&format!(" mov {}, {}", loc.at(size, 0), reg)); + emit!(self, " mov {}, {}", loc.at(size, 0), reg); } } } @@ -2480,21 +2565,21 @@ impl CodeGen { let data_type = ret_info.data_type; match data_type { DataType::Integer => { - self.emit(&format!(" movsx eax, {}", loc.at("WORD PTR", 0))); + emit!(self, " movsx eax, {}", loc.at("WORD PTR", 0)); } DataType::Long => { - self.emit(&format!(" mov eax, {}", loc.at("DWORD PTR", 0))); + emit!(self, " mov eax, {}", loc.at("DWORD PTR", 0)); } DataType::Single => { - self.emit(&format!(" movss xmm0, {}", loc.at("DWORD PTR", 0))); + emit!(self, " movss xmm0, {}", loc.at("DWORD PTR", 0)); } DataType::Double => { - self.emit(&format!(" movsd xmm0, {}", loc.q(0))); + emit!(self, " movsd xmm0, {}", loc.q(0)); } DataType::String => { // Load string (ptr, len) into rax, rdx - self.emit(&format!(" mov rax, {}", loc.q(0))); - self.emit(&format!(" mov rdx, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(0)); + emit!(self, " mov rdx, {}", loc.q(1)); } } } @@ -2547,8 +2632,8 @@ impl CodeGen { let src_ty = self.typed_var(src).expect("sema checked the source"); let src_loc = self.get_record_loc(src, &src_ty); for w in 0..words { - self.emit(&format!(" mov rax, {}", src_loc.q(w))); - self.emit(&format!(" mov {}, rax", loc.q(w))); + emit!(self, " mov rax, {}", src_loc.q(w)); + emit!(self, " mov {}, rax", loc.q(w)); } return; } @@ -2562,8 +2647,8 @@ impl CodeGen { self.gen_array_addr(name, &indices); self.emit(" mov r10, rax"); for w in 0..words { - self.emit(&format!(" mov rax, QWORD PTR [r10 + {}]", w * 8)); - self.emit(&format!(" mov {}, rax", loc.q(w))); + emit!(self, " mov rax, QWORD PTR [r10 + {}]", w * 8); + emit!(self, " mov {}, rax", loc.q(w)); } return; } @@ -2613,16 +2698,16 @@ impl CodeGen { let loc = &var_info.loc; match var_info.data_type { DataType::Integer => { - self.emit(&format!(" mov {}, ax", loc.at("WORD PTR", 0))); + emit!(self, " mov {}, ax", loc.at("WORD PTR", 0)); } DataType::Long => { - self.emit(&format!(" mov {}, eax", loc.at("DWORD PTR", 0))); + emit!(self, " mov {}, eax", loc.at("DWORD PTR", 0)); } DataType::Single => { - self.emit(&format!(" movss {}, xmm0", loc.at("DWORD PTR", 0))); + emit!(self, " movss {}, xmm0", loc.at("DWORD PTR", 0)); } DataType::Double => { - self.emit(&format!(" movsd {}, xmm0", loc.q(0))); + emit!(self, " movsd {}, xmm0", loc.q(0)); } DataType::String => { // Should be handled by gen_string_assign above @@ -2692,11 +2777,20 @@ impl CodeGen { StmtKind::Input { prompt, + query, vars, file_num, } => { - if let Some(pstr) = prompt { - self.gen_console_prompt(pstr); + // The question mark is part of the prompt text, so it costs + // nothing at run time and needs no runtime support. + let text = match (prompt.as_deref(), query) { + (Some(p), true) => Some(format!("{}? ", p)), + (Some(p), false) => Some(p.to_string()), + (None, true) => Some("? ".to_string()), + (None, false) => None, + }; + if let Some(text) = text { + self.gen_console_prompt(&text); } let fnum = file_num.as_ref().map(|e| self.gen_file_num(e)); for var in vars { @@ -2712,7 +2806,7 @@ impl CodeGen { (None, true) => "_rt_input_string", (None, false) => "_rt_input_number", }; - self.emit(&format!(" call {}", rt)); + emit!(self, " call {}", rt); self.gen_store_lvalue(var); } } @@ -2751,7 +2845,7 @@ impl CodeGen { for s in then_branch { self.gen_stmt(s); } - self.emit(&format!(" jmp {}", end_label)); + emit!(self, " jmp {}", end_label); self.emit_label(&else_label); if let Some(eb) = else_branch { @@ -2804,7 +2898,7 @@ impl CodeGen { if let Some(reg) = &counter_reg { if ct.is_integer() { self.emit(" sub rsp, 16 # save a counter register"); - self.emit(&format!(" mov QWORD PTR [rsp], {}", reg)); + emit!(self, " mov QWORD PTR [rsp], {}", reg); } } @@ -2866,7 +2960,7 @@ impl CodeGen { for p in &accumulators.clone() { if p.ty.is_integer() { self.emit(" sub rsp, 16 # save an accumulator register"); - self.emit(&format!(" mov QWORD PTR [rsp], {}", p.reg)); + emit!(self, " mov QWORD PTR [rsp], {}", p.reg); } self.emit_promotion_load(p); } @@ -2915,11 +3009,12 @@ impl CodeGen { match direction { Some(ascending) => { compare(self); - self.emit(&format!( + emit!( + self, " {} {}", Self::for_exit_branch(ct, ascending), end_label - )); + ); } None => { // Step known only at run time, so both directions have @@ -2930,20 +3025,22 @@ impl CodeGen { self.emit_for_test_step_sign(ct, &step_operand, &neg); compare(self); - self.emit(&format!( + emit!( + self, " {} {}", Self::for_exit_branch(ct, true), end_label - )); - self.emit(&format!(" jmp {}", body_label)); + ); + emit!(self, " jmp {}", body_label); self.emit_label(&neg); compare(self); - self.emit(&format!( + emit!( + self, " {} {}", Self::for_exit_branch(ct, false), end_label - )); + ); self.emit_label(&body_label); } } @@ -2990,7 +3087,7 @@ impl CodeGen { self.emit_for_store(ct, &var_mem); } } - self.emit(&format!(" jmp {}", start_label)); + emit!(self, " jmp {}", start_label); self.emit_label(&end_label); @@ -3004,7 +3101,7 @@ impl CodeGen { for p in accumulators.iter().rev() { self.emit_promotion_store(p); if p.ty.is_integer() { - self.emit(&format!(" mov {}, QWORD PTR [rsp]", p.reg)); + emit!(self, " mov {}, QWORD PTR [rsp]", p.reg); self.emit(" add rsp, 16"); } } @@ -3012,7 +3109,7 @@ impl CodeGen { let reg = reg.clone(); self.emit_counter_writeback(ct, ®, &var_mem); if ct.is_integer() { - self.emit(&format!(" mov {}, QWORD PTR [rsp]", reg)); + emit!(self, " mov {}, QWORD PTR [rsp]", reg); self.emit(" add rsp, 16"); } } @@ -3031,7 +3128,7 @@ impl CodeGen { self.gen_stmt(s); } self.loop_stack.pop(); - self.emit(&format!(" jmp {}", start_label)); + emit!(self, " jmp {}", start_label); self.emit_label(&end_label); } @@ -3069,10 +3166,10 @@ impl CodeGen { // repeats while false. self.gen_condition(cond, &start_label, !*is_until); } else { - self.emit(&format!(" jmp {}", start_label)); + emit!(self, " jmp {}", start_label); } } else { - self.emit(&format!(" jmp {}", start_label)); + emit!(self, " jmp {}", start_label); } self.emit_label(&end_label); @@ -3083,7 +3180,7 @@ impl CodeGen { GotoTarget::Line(n) => format!("_line_{}", n), GotoTarget::Label(s) => format!("_label_{}", mangle(s)), }; - self.emit(&format!(" jmp {}", label)); + emit!(self, " jmp {}", label); } StmtKind::Gosub(target) => { @@ -3099,10 +3196,10 @@ impl CodeGen { self.emit(" cmp rcx, rax"); self.emit_check("jb", RtError::GosubOverflow); // Push return address to GOSUB stack - self.emit(&format!(" lea rax, [rip + {}]", ret_label)); + emit!(self, " lea rax, [rip + {}]", ret_label); self.emit(" mov QWORD PTR [rcx], rax"); self.emit(" mov QWORD PTR [rip + _gosub_sp], rcx"); - self.emit(&format!(" jmp {}", label)); + emit!(self, " jmp {}", label); self.emit_label(&ret_label); } @@ -3129,8 +3226,8 @@ impl CodeGen { GotoTarget::Line(n) => format!("_line_{}", n), GotoTarget::Label(s) => format!("_label_{}", mangle(s)), }; - self.emit(&format!(" cmp rax, {}", i + 1)); - self.emit(&format!(" je {}", label)); + emit!(self, " cmp rax, {}", i + 1); + emit!(self, " je {}", label); } } @@ -3158,16 +3255,16 @@ impl CodeGen { let after = self.new_label("on_gosub_ret"); self.emit(" cmp r8, 1"); - self.emit(&format!(" jl {}", after)); - self.emit(&format!(" cmp r8, {}", targets.len())); - self.emit(&format!(" jg {}", after)); + emit!(self, " jl {}", after); + emit!(self, " cmp r8, {}", targets.len()); + emit!(self, " jg {}", after); self.emit(" mov rcx, QWORD PTR [rip + _gosub_sp]"); self.emit(" sub rcx, 8"); self.emit(" lea rax, [rip + _gosub_stack]"); self.emit(" cmp rcx, rax"); self.emit_check("jb", RtError::GosubOverflow); - self.emit(&format!(" lea rax, [rip + {}]", after)); + emit!(self, " lea rax, [rip + {}]", after); self.emit(" mov QWORD PTR [rcx], rax"); self.emit(" mov QWORD PTR [rip + _gosub_sp], rcx"); @@ -3176,8 +3273,8 @@ impl CodeGen { GotoTarget::Line(n) => format!("_line_{}", n), GotoTarget::Label(s) => format!("_label_{}", mangle(s)), }; - self.emit(&format!(" cmp r8, {}", i + 1)); - self.emit(&format!(" je {}", label)); + emit!(self, " cmp r8, {}", i + 1); + emit!(self, " je {}", label); } self.emit_label(&after); } @@ -3222,7 +3319,7 @@ impl CodeGen { Some(GotoTarget::Line(n)) => self.data_line_index.get(n).copied().unwrap_or(0), Some(GotoTarget::Label(name)) => self .data_label_index - .get(&name.to_uppercase()) + .get(normalized(name)) .copied() .unwrap_or(0), None => 0, @@ -3275,13 +3372,13 @@ impl CodeGen { .find(|(f, _)| f == is_for) .map(|(_, label)| label.clone()); if let Some(label) = target { - self.emit(&format!(" jmp {}", label)); + emit!(self, " jmp {}", label); } } StmtKind::ExitProc => { if let Some(label) = self.proc_exit_label.clone() { - self.emit(&format!(" jmp {}", label)); + emit!(self, " jmp {}", label); } } @@ -3300,19 +3397,13 @@ impl CodeGen { if is_string { self.stack_offset -= 16; temp_offset = self.stack_offset; - self.emit(&format!(" mov QWORD PTR [rbp + {}], rax", temp_offset)); - self.emit(&format!( - " mov QWORD PTR [rbp + {}], rdx", - temp_offset + 8 - )); + emit!(self, " mov QWORD PTR [rbp + {}], rax", temp_offset); + emit!(self, " mov QWORD PTR [rbp + {}], rdx", temp_offset + 8); } else { self.gen_coercion(expr_type, DataType::Double); self.stack_offset -= 8; temp_offset = self.stack_offset; - self.emit(&format!( - " movsd QWORD PTR [rbp + {}], xmm0", - temp_offset - )); + emit!(self, " movsd QWORD PTR [rbp + {}], xmm0", temp_offset); } // Generate code for each case @@ -3330,7 +3421,7 @@ impl CodeGen { for clause in clauses { self.gen_case_clause(clause, temp_offset, is_string, &body_label); } - self.emit(&format!(" jmp {}", next_case_label)); + emit!(self, " jmp {}", next_case_label); self.emit_label(&body_label); } // CASE ELSE (None) falls through without comparison @@ -3342,7 +3433,7 @@ impl CodeGen { // Jump to end (skip remaining cases) if i + 1 < cases.len() { - self.emit(&format!(" jmp {}", end_label)); + emit!(self, " jmp {}", end_label); self.emit_label(&next_case_label); } } @@ -3430,7 +3521,7 @@ impl CodeGen { // field's existing pointer, so the target keeps its address and // width while the bytes underneath change. self.gen_expr(value); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); self.gen_load_lvalue_string(target); @@ -3442,9 +3533,9 @@ impl CodeGen { // length that argument 3 has yet to read. let src_ptr = Self::arg_reg(2); let src_len = Self::arg_reg(3); - self.emit(&format!(" mov {}, QWORD PTR [rsp]", src_ptr)); - self.emit(&format!(" mov {}, QWORD PTR [rsp + 8]", src_len)); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " mov {}, QWORD PTR [rsp]", src_ptr); + emit!(self, " mov {}, QWORD PTR [rsp + 8]", src_len); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); if *right { self.emit(" call _rt_rset"); } else { @@ -3526,7 +3617,7 @@ impl CodeGen { Literal::Integer(n) => match i32::try_from(*n) { Ok(v) => { // Load as integer into eax - self.emit(&format!(" mov eax, {}", v)); + emit!(self, " mov eax, {}", v); DataType::Long } // Wider than LONG: emit as a Double rather than truncating @@ -3545,17 +3636,17 @@ impl CodeGen { } Literal::String(s) => { let idx = self.add_string_literal(s); - self.emit(&format!(" lea rax, [rip + _str_{}]", idx)); + emit!(self, " lea rax, [rip + _str_{}]", idx); // A literal's length cannot approach 2^32, and writing edx // zeroes the upper half, so the narrow form is equivalent. - self.emit(&format!(" mov edx, {}", s.len())); + emit!(self, " mov edx, {}", s.len()); DataType::String } }, Expr::Variable(name) => { // A CONST is substituted with its folded value. - if let Some(lit) = self.symbols.consts.get(&name.to_uppercase()).cloned() { + if let Some(lit) = self.symbols.consts.get(normalized(name)).cloned() { return self.gen_expr(&Expr::Literal(lit)); } @@ -3602,20 +3693,20 @@ impl CodeGen { let loc = &info.loc; match info.data_type { DataType::Integer => { - self.emit(&format!(" movsx eax, {}", loc.at("WORD PTR", 0))); + emit!(self, " movsx eax, {}", loc.at("WORD PTR", 0)); } DataType::Long => { - self.emit(&format!(" mov eax, {}", loc.at("DWORD PTR", 0))); + emit!(self, " mov eax, {}", loc.at("DWORD PTR", 0)); } DataType::Single => { - self.emit(&format!(" movss xmm0, {}", loc.at("DWORD PTR", 0))); + emit!(self, " movss xmm0, {}", loc.at("DWORD PTR", 0)); } DataType::Double => { - self.emit(&format!(" movsd xmm0, {}", loc.q(0))); + emit!(self, " movsd xmm0, {}", loc.q(0)); } DataType::String => { - self.emit(&format!(" mov rax, {}", loc.q(0))); - self.emit(&format!(" mov rdx, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(0)); + emit!(self, " mov rdx, {}", loc.q(1)); } } info.data_type @@ -3649,26 +3740,34 @@ impl CodeGen { // and requires them aligned, and pool entries // are eight bytes on an eight-byte boundary. let mask = self.f64_operand(-0.0); - self.emit(&format!(" movsd xmm1, {}", mask)); + emit!(self, " movsd xmm1, {}", mask); self.emit(" xorpd xmm0, xmm1"); } operand_type } } UnaryOp::Not => { - // NOT: if 0 then -1, else 0 - result is always Long - if operand_type.is_integer() { - self.emit(" test eax, eax"); - } else if operand_type == DataType::Single { - self.emit(" xorps xmm1, xmm1"); - self.emit(" ucomiss xmm0, xmm1"); - } else { - self.emit(" xorpd xmm1, xmm1"); - self.emit(" ucomisd xmm0, xmm1"); + // NOT is a bitwise complement, like AND, OR and XOR + // beside it: `NOT 12` is -13, not 0. + // + // It used to emit `sete al / movzx / neg`, i.e. "0 gives + // -1, anything else gives 0" -- a *logical* not. That + // agrees with the bitwise answer only when the operand + // is already 0 or -1, which every test used, so the + // difference stayed invisible while `12 AND 10` was + // correctly bitwise and `NOT 12` was not. + // + // GW-BASIC complements a 16-bit two's-complement + // integer; we use 32-bit, as the other three do. + if !operand_type.is_integer() { + self.emit_typed( + operand_type, + "", + " cvttss2si eax, xmm0", + " cvttsd2si eax, xmm0", + ); } - self.emit(" sete al"); - self.emit(" movzx eax, al"); - self.emit(" neg eax"); + self.emit(" not eax"); DataType::Long } } @@ -3727,7 +3826,7 @@ impl CodeGen { // Evaluate left string (ptr in rax, len in rdx) self.gen_expr(left); // Save left string on stack using consistent sub rsp pattern (16-byte aligned) - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); // left ptr self.emit(" mov QWORD PTR [rsp + 8], rdx"); // left len @@ -3742,7 +3841,7 @@ impl CodeGen { // Restore left string from stack self.emit(" mov rax, QWORD PTR [rsp]"); // left ptr self.emit(" mov rdx, QWORD PTR [rsp + 8]"); // left len - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.emit_arg_reg(0, "rax"); // left ptr self.emit_arg_reg(1, "rdx"); // left len self.emit_arg_reg(2, "r8"); // right ptr @@ -3780,7 +3879,7 @@ impl CodeGen { BinaryOp::Ge => "setge", _ => unreachable!("guarded by is_comparison"), }; - self.emit(&format!(" {} al", setcc)); + emit!(self, " {} al", setcc); self.emit(" movzx eax, al"); self.emit(" neg eax"); // BASIC true is -1 self.expr_depth -= 1; @@ -3864,6 +3963,7 @@ impl CodeGen { BinaryOp::Pow => { self.emit_cvt_to_double(work_type); self.emit_call_libc("pow"); + self.emit_pow_domain_check(); } BinaryOp::Eq | BinaryOp::Ne @@ -3892,7 +3992,7 @@ impl CodeGen { } else { unsigned }; - self.emit(&format!(" {} al", setcc)); + emit!(self, " {} al", setcc); self.emit(" movzx eax, al"); self.emit(" neg eax"); } @@ -3904,7 +4004,7 @@ impl CodeGen { BinaryOp::Xor => "xor", _ => unreachable!(), }; - self.emit(&format!(" {} eax, ecx", instr)); + emit!(self, " {} eax, ecx", instr); } } @@ -3981,7 +4081,7 @@ impl CodeGen { if left_type == DataType::String && right_type == DataType::String { self.gen_string_compare(left, right); - self.emit(&format!(" {} {}", jcc(effective(*op), true), target)); + emit!(self, " {} {}", jcc(effective(*op), true), target); return; } @@ -3994,16 +4094,16 @@ impl CodeGen { if let Some(n) = self.const_i32(right) { let ty = self.gen_expr(left); self.gen_coercion(ty, work_type); - self.emit(&format!(" cmp eax, {}", n)); - self.emit(&format!(" {} {}", jcc(effective(*op), true), target)); + emit!(self, " cmp eax, {}", n); + emit!(self, " {} {}", jcc(effective(*op), true), target); return; } } else if work_type == DataType::Double { if let Some(value) = self.const_double(right) { self.gen_expr_to_double(left); let operand = self.f64_operand(value); - self.emit(&format!(" ucomisd xmm0, {}", operand)); - self.emit(&format!(" {} {}", jcc(effective(*op), false), target)); + emit!(self, " ucomisd xmm0, {}", operand); + emit!(self, " {} {}", jcc(effective(*op), false), target); return; } } @@ -4015,7 +4115,7 @@ impl CodeGen { " ucomiss xmm0, xmm1", " ucomisd xmm0, xmm1", ); - self.emit(&format!(" {} {}", jcc(effective(*op), signed), target)); + emit!(self, " {} {}", jcc(effective(*op), signed), target); return; } } @@ -4029,11 +4129,12 @@ impl CodeGen { self.emit(" xorpd xmm1, xmm1"); self.emit(" ucomisd xmm0, xmm1"); } - self.emit(&format!( + emit!( + self, " {} {}", if jump_if_true { "jne" } else { "je" }, target - )); + ); } /// Compare two strings, leaving memcmp-style flags set. @@ -4045,7 +4146,7 @@ impl CodeGen { fn gen_string_compare(&mut self, left: &Expr, right: &Expr) { // Evaluate left string (ptr in rax, len in rdx) self.gen_expr(left); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); // left ptr self.emit(" mov QWORD PTR [rsp + 8], rdx"); // left len @@ -4055,7 +4156,7 @@ impl CodeGen { self.emit(" mov r9, rdx"); // right len self.emit(" mov rax, QWORD PTR [rsp]"); // left ptr self.emit(" mov rdx, QWORD PTR [rsp + 8]"); // left len - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.emit_arg_reg(0, "rax"); self.emit_arg_reg(1, "rdx"); self.emit_arg_reg(2, "r8"); @@ -4075,7 +4176,7 @@ impl CodeGen { let left_type = self.gen_expr(left); self.gen_coercion(left_type, work_type); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); if work_type.is_integer() { self.emit(" mov QWORD PTR [rsp], rax"); } else if work_type == DataType::Single { @@ -4098,7 +4199,7 @@ impl CodeGen { self.emit(" movsd xmm1, xmm0"); // right in xmm1 self.emit(" movsd xmm0, QWORD PTR [rsp]"); // left in xmm0 } - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); } /// Apply `op` with a compile-time constant on the right, or report that it @@ -4166,17 +4267,17 @@ impl CodeGen { self.gen_coercion(left_type, work_type); if let Some(instr) = arith { - self.emit(&format!(" {} eax, {}", instr, n)); + emit!(self, " {} eax, {}", instr, n); } else if Self::is_comparison(op) { - self.emit(&format!(" cmp eax, {}", n)); - self.emit(&format!(" {} al", setcc(op, true))); + emit!(self, " cmp eax, {}", n); + emit!(self, " {} al", setcc(op, true)); self.emit(" movzx eax, al"); self.emit(" neg eax"); // BASIC true is -1 } else { // idiv has no immediate form, so the divisor still has to // reach ecx -- but it gets there without the spill, and // the existing checks apply to it unchanged. - self.emit(&format!(" mov ecx, {}", n)); + emit!(self, " mov ecx, {}", n); self.emit_integer_divide_checks(); self.emit(" cdq"); self.emit(" idiv ecx"); @@ -4200,9 +4301,9 @@ impl CodeGen { let operand = self.f64_operand(value); match op { - BinaryOp::Add => self.emit(&format!(" addsd xmm0, {}", operand)), - BinaryOp::Sub => self.emit(&format!(" subsd xmm0, {}", operand)), - BinaryOp::Mul => self.emit(&format!(" mulsd xmm0, {}", operand)), + BinaryOp::Add => emit!(self, " addsd xmm0, {}", operand), + BinaryOp::Sub => emit!(self, " subsd xmm0, {}", operand), + BinaryOp::Mul => emit!(self, " mulsd xmm0, {}", operand), BinaryOp::Div => { // The divisor is known here, so the check is decided // here too rather than being re-tested at run time. @@ -4211,28 +4312,29 @@ impl CodeGen { if value == 0.0 { self.emit_check("jmp", RtError::DivideByZero); } - self.emit(&format!(" divsd xmm0, {}", operand)); + emit!(self, " divsd xmm0, {}", operand); } BinaryOp::Pow => { - self.emit(&format!(" movsd xmm1, {}", operand)); + emit!(self, " movsd xmm1, {}", operand); self.emit_call_libc("pow"); + self.emit_pow_domain_check(); } BinaryOp::And | BinaryOp::Or | BinaryOp::Xor => { // Truncation to integer is left to the same conversion // the general path uses, so an out-of-range constant // behaves identically to one held in a register. - self.emit(&format!(" movsd xmm1, {}", operand)); + emit!(self, " movsd xmm1, {}", operand); self.emit_cvt_float_to_int(work_type); let instr = match op { BinaryOp::And => "and", BinaryOp::Or => "or", _ => "xor", }; - self.emit(&format!(" {} eax, ecx", instr)); + emit!(self, " {} eax, ecx", instr); } _ => { - self.emit(&format!(" ucomisd xmm0, {}", operand)); - self.emit(&format!(" {} al", setcc(op, false))); + emit!(self, " ucomisd xmm0, {}", operand); + emit!(self, " {} al", setcc(op, false)); self.emit(" movzx eax, al"); self.emit(" neg eax"); } @@ -4259,7 +4361,7 @@ impl CodeGen { self.emit_file_num_check(); self.stack_offset -= 8; let loc = Loc::Frame(self.stack_offset); - self.emit(&format!(" mov {}, eax", loc.at("DWORD PTR", 0))); + emit!(self, " mov {}, eax", loc.at("DWORD PTR", 0)); FileNum::Slot(loc) } @@ -4289,7 +4391,7 @@ impl CodeGen { self.gen_coercion(ty, DataType::Long); self.stack_offset -= 8; let loc = Loc::Frame(self.stack_offset); - self.emit(&format!(" mov {}, eax", loc.at("DWORD PTR", 0))); + emit!(self, " mov {}, eax", loc.at("DWORD PTR", 0)); FileNum::Slot(loc) } @@ -4302,21 +4404,21 @@ impl CodeGen { fn gen_bind_alias(&mut self, target: &LValue) { if let Some(indices) = &target.indices { let indices = indices.clone(); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); self.gen_array_addr(&target.name, &indices); self.emit(" mov rcx, rax"); self.emit(" mov rax, QWORD PTR [rsp]"); self.emit(" mov rdx, QWORD PTR [rsp + 8]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rcx], rax"); self.emit(" mov QWORD PTR [rcx + 8], rdx"); return; } let loc = self.get_var_loc(&target.name); - self.emit(&format!(" mov {}, rax", loc.q(0))); - self.emit(&format!(" mov {}, rdx", loc.q(1))); + emit!(self, " mov {}, rax", loc.q(0)); + emit!(self, " mov {}, rdx", loc.q(1)); } /// Load a string lvalue's current (pointer, length) into `rax`/`rdx`. @@ -4332,8 +4434,8 @@ impl CodeGen { return; } let loc = self.get_var_loc(&target.name); - self.emit(&format!(" mov rax, {}", loc.q(0))); - self.emit(&format!(" mov rdx, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(0)); + emit!(self, " mov rdx, {}", loc.q(1)); } /// Resolve where a PRINT writes: a file number, or the console. @@ -4361,7 +4463,7 @@ impl CodeGen { fn emit_file_num_check(&mut self) { self.emit(" cmp eax, 1"); self.emit_check("jl", RtError::BadFileNum); - self.emit(&format!(" cmp eax, {}", crate::sema::MAX_FILE_NUM)); + emit!(self, " cmp eax, {}", crate::sema::MAX_FILE_NUM); self.emit_check("jg", RtError::BadFileNum); } @@ -4371,7 +4473,7 @@ impl CodeGen { FileNum::Imm(n) => self.emit_arg_imm(idx, *n), FileNum::Slot(loc) => { let reg = Self::arg_reg(idx); - self.emit(&format!(" movsxd {}, {}", reg, loc.at("DWORD PTR", 0))); + emit!(self, " movsxd {}, {}", reg, loc.at("DWORD PTR", 0)); } } } @@ -4384,7 +4486,7 @@ impl CodeGen { // Everything is evaluated into a temp block first, because each // evaluation clobbers the value registers. const SLOTS: i32 = 96; // 6 values, 16-byte aligned with room to spare - self.emit(&format!(" sub rsp, {}", SLOTS)); + emit!(self, " sub rsp, {}", SLOTS); self.gen_read_lvalue(target); self.emit(" mov QWORD PTR [rsp], rax"); // target pointer @@ -4416,12 +4518,12 @@ impl CodeGen { let regs = PlatformAbi::INT_ARG_REGS; for (i, off) in [0, 8, 16, 24].iter().enumerate() { if i < regs.len() { - self.emit(&format!(" mov {}, QWORD PTR [rsp + {}]", regs[i], off)); + emit!(self, " mov {}, QWORD PTR [rsp + {}]", regs[i], off); } } if regs.len() >= 6 { - self.emit(&format!(" mov {}, QWORD PTR [rsp + 32]", regs[4])); - self.emit(&format!(" mov {}, QWORD PTR [rsp + 40]", regs[5])); + emit!(self, " mov {}, QWORD PTR [rsp + 32]", regs[4]); + emit!(self, " mov {}, QWORD PTR [rsp + 40]", regs[5]); self.emit(" call _rt_mid_assign"); } else { // Win64: the 5th and 6th arguments sit just above the 32-byte @@ -4437,7 +4539,7 @@ impl CodeGen { self.emit(" add rsp, 64"); } - self.emit(&format!(" add rsp, {}", SLOTS)); + emit!(self, " add rsp, {}", SLOTS); } /// Exchange two values, which sema has checked are the same type class. @@ -4451,7 +4553,7 @@ impl CodeGen { // Read A into a temp. self.gen_read_lvalue(a); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); if is_string { self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); @@ -4470,7 +4572,7 @@ impl CodeGen { } else { self.emit(" movsd xmm0, QWORD PTR [rsp]"); } - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.gen_store_lvalue(b); } @@ -4638,7 +4740,7 @@ impl CodeGen { let TypeRef::Record(rec) = &ty else { return DataType::Double; }; - let Some(info) = self.symbols.records.get(&rec.to_uppercase()) else { + let Some(info) = self.symbols.records.get(normalized(rec)) else { return DataType::Double; }; let Some(f) = info.field(field) else { @@ -4675,8 +4777,8 @@ impl CodeGen { } let loc = self.get_record_loc(name, &ty); match &loc { - Loc::Global(sym) => self.emit(&format!(" lea rax, [rip + {}]", sym)), - Loc::Frame(off) => self.emit(&format!(" lea rax, [rbp + {}]", off)), + Loc::Global(sym) => emit!(self, " lea rax, [rip + {}]", sym), + Loc::Frame(off) => emit!(self, " lea rax, [rbp + {}]", off), } true } @@ -4711,7 +4813,7 @@ impl CodeGen { } self.gen_array_addr(&name, &indices); if offset != 0 { - self.emit(&format!(" add rax, {}", offset)); + emit!(self, " add rax, {}", offset); } return true; } @@ -4726,8 +4828,8 @@ impl CodeGen { return false; } match &loc { - Loc::Global(sym) => self.emit(&format!(" lea rax, [rip + {}]", sym)), - Loc::Frame(off) => self.emit(&format!(" lea rax, [rbp + {}]", off)), + Loc::Global(sym) => emit!(self, " lea rax, [rip + {}]", sym), + Loc::Frame(off) => emit!(self, " lea rax, [rbp + {}]", off), } true } @@ -4744,11 +4846,7 @@ impl CodeGen { let TypeRef::Record(rec) = &ty else { return None; }; - let f = self - .symbols - .records - .get(&rec.to_uppercase())? - .field(field)?; + let f = self.symbols.records.get(normalized(rec))?.field(field)?; offset += f.word * 8; ty = f.ty.clone(); } @@ -4770,7 +4868,7 @@ impl CodeGen { self.gen_array_addr(&target.name, &indices); let loc = Loc::Frame(0); // placeholder, replaced below let _ = loc; - self.emit(&format!(" add rax, {}", byte_offset)); + emit!(self, " add rax, {}", byte_offset); self.gen_load_indirect("rax", &ty) } Some(v) => { @@ -4779,17 +4877,17 @@ impl CodeGen { let vt = self.gen_expr(v); if DataType::from_type_ref(&ty) == DataType::String { self.emit_string_copy_of(v); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); } else { self.gen_coercion(vt, DataType::Double); - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" movsd QWORD PTR [rsp], xmm0"); } self.gen_array_addr(&target.name, &indices); - self.emit(&format!(" add rax, {}", byte_offset)); + emit!(self, " add rax, {}", byte_offset); self.emit(" mov rcx, rax"); self.emit_store_parked(&ty); DataType::Double @@ -4805,13 +4903,13 @@ impl CodeGen { if DataType::from_type_ref(ty) == DataType::String { self.emit(" mov rax, QWORD PTR [rsp]"); self.emit(" mov rdx, QWORD PTR [rsp + 8]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rcx], rax"); self.emit(" mov QWORD PTR [rcx + 8], rdx"); return; } self.emit(" movsd xmm0, QWORD PTR [rsp]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.gen_coercion(DataType::Double, DataType::from_type_ref(ty)); match ty { TypeRef::Integer => self.emit(" mov WORD PTR [rcx], ax"), @@ -4825,23 +4923,23 @@ impl CodeGen { fn gen_load_indirect(&mut self, reg: &str, ty: &TypeRef) -> DataType { match ty { TypeRef::Integer => { - self.emit(&format!(" movsx eax, WORD PTR [{}]", reg)); + emit!(self, " movsx eax, WORD PTR [{}]", reg); DataType::Integer } TypeRef::Long => { - self.emit(&format!(" mov eax, DWORD PTR [{}]", reg)); + emit!(self, " mov eax, DWORD PTR [{}]", reg); DataType::Long } TypeRef::Single => { - self.emit(&format!(" movss xmm0, DWORD PTR [{}]", reg)); + emit!(self, " movss xmm0, DWORD PTR [{}]", reg); DataType::Single } TypeRef::Double => { - self.emit(&format!(" movsd xmm0, QWORD PTR [{}]", reg)); + emit!(self, " movsd xmm0, QWORD PTR [{}]", reg); DataType::Double } TypeRef::FixedString(_) => { - self.emit(&format!(" mov rcx, {}", reg)); + emit!(self, " mov rcx, {}", reg); self.emit(" mov rax, QWORD PTR [rcx]"); self.emit(" mov rdx, QWORD PTR [rcx + 8]"); DataType::String @@ -4906,7 +5004,7 @@ impl CodeGen { let TypeRef::Record(rec) = &ty else { return None; }; - let info = self.symbols.records.get(&rec.to_uppercase())?; + let info = self.symbols.records.get(normalized(rec))?; let f = info.field(field)?; loc = loc.offset_words(f.word); ty = f.ty.clone(); @@ -4977,24 +5075,24 @@ impl CodeGen { fn gen_load_typed(&mut self, loc: &Loc, ty: &TypeRef) -> DataType { match ty { TypeRef::Integer => { - self.emit(&format!(" movsx eax, {}", loc.at("WORD PTR", 0))); + emit!(self, " movsx eax, {}", loc.at("WORD PTR", 0)); DataType::Integer } TypeRef::Long => { - self.emit(&format!(" mov eax, {}", loc.at("DWORD PTR", 0))); + emit!(self, " mov eax, {}", loc.at("DWORD PTR", 0)); DataType::Long } TypeRef::Single => { - self.emit(&format!(" movss xmm0, {}", loc.at("DWORD PTR", 0))); + emit!(self, " movss xmm0, {}", loc.at("DWORD PTR", 0)); DataType::Single } TypeRef::Double => { - self.emit(&format!(" movsd xmm0, {}", loc.q(0))); + emit!(self, " movsd xmm0, {}", loc.q(0)); DataType::Double } TypeRef::FixedString(_) => { - self.emit(&format!(" mov rax, {}", loc.q(0))); - self.emit(&format!(" mov rdx, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(0)); + emit!(self, " mov rdx, {}", loc.q(1)); DataType::String } // A whole record has no scalar value; sema rejects using one here. @@ -5007,24 +5105,32 @@ impl CodeGen { match ty { TypeRef::Integer => { self.gen_coercion(value_type, DataType::Integer); - self.emit(&format!(" mov {}, ax", loc.at("WORD PTR", 0))); + emit!(self, " mov {}, ax", loc.at("WORD PTR", 0)); } TypeRef::Long => { self.gen_coercion(value_type, DataType::Long); - self.emit(&format!(" mov {}, eax", loc.at("DWORD PTR", 0))); + emit!(self, " mov {}, eax", loc.at("DWORD PTR", 0)); } TypeRef::Single => { self.gen_coercion(value_type, DataType::Single); - self.emit(&format!(" movss {}, xmm0", loc.at("DWORD PTR", 0))); + emit!(self, " movss {}, xmm0", loc.at("DWORD PTR", 0)); } TypeRef::Double => { self.gen_coercion(value_type, DataType::Double); - self.emit(&format!(" movsd {}, xmm0", loc.q(0))); - } - TypeRef::FixedString(_) => { - self.emit_string_copy(); - self.emit(&format!(" mov {}, rax", loc.q(0))); - self.emit(&format!(" mov {}, rdx", loc.q(1))); + emit!(self, " movsd {}, xmm0", loc.q(0)); + } + TypeRef::FixedString(width) => { + // A fixed-length string holds exactly its declared width: a + // short value is space-padded, a long one truncated. Storing + // the source verbatim -- as this did -- made the width mean + // nothing, so `STRING * 5` held whatever it was given and a + // record laid over a random-access file stopped lining up. + self.emit_arg_reg(0, "rax"); + self.emit_arg_reg(1, "rdx"); + self.emit_arg_imm(2, *width as i64); + self.emit(" call _rt_fixed"); + emit!(self, " mov {}, rax", loc.q(0)); + emit!(self, " mov {}, rdx", loc.q(1)); } TypeRef::Record(_) => {} } @@ -5045,7 +5151,7 @@ impl CodeGen { if let Some(indices) = target.indices.clone() { if let Some(base) = self.array_elem_type(&target.name) { if let Some((offset, ty)) = self.field_byte_offset(&base, &target.fields) { - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); if DataType::from_type_ref(&ty) == DataType::String { self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); @@ -5053,7 +5159,7 @@ impl CodeGen { self.emit(" movsd QWORD PTR [rsp], xmm0"); } self.gen_array_addr(&target.name, &indices); - self.emit(&format!(" add rax, {}", offset)); + emit!(self, " add rax, {}", offset); self.emit(" mov rcx, rax"); self.emit_store_parked(&ty); return; @@ -5072,18 +5178,43 @@ impl CodeGen { } let Some(indices) = &target.indices else { + // A variable declared with `AS` lives in typed storage and carries + // its own width; gen_store_typed is what narrows for it, and is the + // same helper an ordinary assignment to it uses. + if let Some((loc, ty)) = self.typed_storage(&target.name) { + self.gen_store_typed(&loc, &ty, DataType::Double); + return; + } + let loc = self.get_var_loc(&target.name); if is_string { - self.emit(&format!(" mov {}, rax", loc.q(0))); - self.emit(&format!(" mov {}, rdx", loc.q(1))); - } else { - self.emit(&format!(" movsd {}, xmm0", loc.q(0))); + emit!(self, " mov {}, rax", loc.q(0)); + emit!(self, " mov {}, rdx", loc.q(1)); + return; + } + // Narrow to the variable's declared type, exactly as the array + // element path below does and for the same reason. + // + // This used to store the incoming Double whole, so an INTEGER slot + // received eight bytes of a double's bit pattern and every later + // read -- `movsx eax, WORD PTR` -- took its low sixteen bits, which + // for any ordinary value are zero. READ, INPUT and SWAP all store + // through here, so `READ A%`, `INPUT A%` and `SWAP A%, B%` each + // yielded 0 while plain `A% = 7`, which stores elsewhere, was fine. + let ty = self.expr_type(&Expr::Variable(target.name.clone())); + self.gen_coercion(DataType::Double, ty); + match ty { + DataType::Integer => emit!(self, " mov {}, ax", loc.at("WORD PTR", 0)), + DataType::Long => emit!(self, " mov {}, eax", loc.at("DWORD PTR", 0)), + DataType::Single => emit!(self, " movss {}, xmm0", loc.at("DWORD PTR", 0)), + DataType::Double => emit!(self, " movsd {}, xmm0", loc.q(0)), + DataType::String => unreachable!("handled above"), } return; }; // Park the value, compute the element address, then store. - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); if is_string { self.emit(" mov QWORD PTR [rsp], rax"); self.emit(" mov QWORD PTR [rsp + 8], rdx"); @@ -5098,12 +5229,12 @@ impl CodeGen { if is_string { self.emit(" mov rax, QWORD PTR [rsp]"); self.emit(" mov rdx, QWORD PTR [rsp + 8]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rcx], rax"); self.emit(" mov QWORD PTR [rcx + 8], rdx"); } else { self.emit(" movsd xmm0, QWORD PTR [rsp]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); // Narrow to the element's declared type, as a normal store does. let elem_type = DataType::from_suffix(&target.name); self.gen_coercion(DataType::Double, elem_type); @@ -5227,27 +5358,27 @@ impl CodeGen { match clause { CaseClause::Value(e) => { self.gen_expr_to_double(e); - self.emit(&format!(" movsd xmm1, {}", sel)); + emit!(self, " movsd xmm1, {}", sel); self.emit(" ucomisd xmm1, xmm0"); - self.emit(&format!(" je {}", body_label)); + emit!(self, " je {}", body_label); } CaseClause::Range(lo, hi) => { // Inclusive at both ends. The low bound is tested first, and a // failure skips the high test. let skip = self.new_label("caseskip"); self.gen_expr_to_double(lo); - self.emit(&format!(" movsd xmm1, {}", sel)); + emit!(self, " movsd xmm1, {}", sel); self.emit(" ucomisd xmm1, xmm0"); - self.emit(&format!(" jb {}", skip)); + emit!(self, " jb {}", skip); self.gen_expr_to_double(hi); - self.emit(&format!(" movsd xmm1, {}", sel)); + emit!(self, " movsd xmm1, {}", sel); self.emit(" ucomisd xmm1, xmm0"); - self.emit(&format!(" jbe {}", body_label)); + emit!(self, " jbe {}", body_label); self.emit_label(&skip); } CaseClause::Compare(op, e) => { self.gen_expr_to_double(e); - self.emit(&format!(" movsd xmm1, {}", sel)); + emit!(self, " movsd xmm1, {}", sel); self.emit(" ucomisd xmm1, xmm0"); // Unsigned conditions, since ucomisd sets the carry flag. let cc = match op { @@ -5259,7 +5390,7 @@ impl CodeGen { BinaryOp::Ge => "jae", _ => unreachable!("the parser only builds comparisons here"), }; - self.emit(&format!(" {} {}", cc, body_label)); + emit!(self, " {} {}", cc, body_label); } } } @@ -5289,14 +5420,14 @@ impl CodeGen { match clause { CaseClause::Value(e) => { compare(self, e); - self.emit(&format!(" je {}", body_label)); + emit!(self, " je {}", body_label); } CaseClause::Range(lo, hi) => { let skip = self.new_label("caseskip"); compare(self, lo); - self.emit(&format!(" jl {}", skip)); + emit!(self, " jl {}", skip); compare(self, hi); - self.emit(&format!(" jle {}", body_label)); + emit!(self, " jle {}", body_label); self.emit_label(&skip); } CaseClause::Compare(op, e) => { @@ -5310,7 +5441,7 @@ impl CodeGen { BinaryOp::Ge => "jge", _ => unreachable!("the parser only builds comparisons here"), }; - self.emit(&format!(" {} {}", cc, body_label)); + emit!(self, " {} {}", cc, body_label); } } } @@ -5383,7 +5514,7 @@ impl CodeGen { } else { "_rt_file_print_spc" }; - self.emit(&format!(" call {}", rt)); + emit!(self, " call {}", rt); } fn gen_fn_call(&mut self, name: &str, args: &[Expr]) { @@ -5409,14 +5540,14 @@ impl CodeGen { if upper_name == "SQR" { self.emit_domain_check("jb"); } - self.emit(&format!(" {}", instr)); + emit!(self, " {}", instr); return; } // Table-driven: evaluate one argument, coerce it, call the helper. if let Some(builtin) = RT_BUILTINS.get(upper_name.as_str()) { match builtin { - Builtin::Call0(sym) => self.emit(&format!(" call {}", sym)), + Builtin::Call0(sym) => emit!(self, " call {}", sym), Builtin::CallStr(sym) => { // gen_expr leaves a string in rax/rdx. Loading the length // first keeps Win64, where the pointer's register is rdx, @@ -5424,14 +5555,17 @@ impl CodeGen { self.gen_expr(&args[0]); self.emit_arg_reg(1, "rdx"); self.emit_arg_reg(0, "rax"); - self.emit(&format!(" call {}", sym)); + emit!(self, " call {}", sym); } Builtin::CallLong(sym) => { let arg_type = self.gen_expr(&args[0]); self.gen_coercion(arg_type, DataType::Long); self.emit(" movsxd rax, eax"); + if let Some((min, max)) = Self::long_arg_range(&upper_name) { + self.emit_arg_range_check("rax", min, max); + } self.emit_arg_reg(0, "rax"); - self.emit(&format!(" call {}", sym)); + emit!(self, " call {}", sym); } Builtin::Coerce(ty) => { let arg_type = self.gen_expr(&args[0]); @@ -5450,7 +5584,7 @@ impl CodeGen { // 64-bit immediate, and via a register because the memory form // of andpd wants sixteen aligned bytes. See UnaryOp::Neg. let mask = self.f64_operand(f64::from_bits(0x7FFF_FFFF_FFFF_FFFF)); - self.emit(&format!(" movsd xmm1, {}", mask)); + emit!(self, " movsd xmm1, {}", mask); self.emit(" andpd xmm0, xmm1"); // ABS preserves its argument's type: narrow back so the value // matches what call_return_type promises. @@ -5489,10 +5623,11 @@ impl CodeGen { let count_type = self.gen_expr(&args[1]); // count - safe now let arg2 = Self::arg_reg(2); if count_type.is_integer() { - self.emit(&format!(" movsxd {}, eax", arg2)); + emit!(self, " movsxd {}, eax", arg2); } else { - self.emit(&format!(" cvttsd2si {}, xmm0", arg2)); + emit!(self, " cvttsd2si {}, xmm0", arg2); } + self.emit_arg_min_check(arg2, 0); self.emit_arg_reg(0, "r12"); // ptr self.emit_arg_reg(1, "r13"); // len self.emit(" call _rt_left"); @@ -5510,10 +5645,11 @@ impl CodeGen { let count_type = self.gen_expr(&args[1]); // count - safe now let arg2 = Self::arg_reg(2); if count_type.is_integer() { - self.emit(&format!(" movsxd {}, eax", arg2)); + emit!(self, " movsxd {}, eax", arg2); } else { - self.emit(&format!(" cvttsd2si {}, xmm0", arg2)); + emit!(self, " cvttsd2si {}, xmm0", arg2); } + self.emit_arg_min_check(arg2, 0); self.emit_arg_reg(0, "r12"); // ptr self.emit_arg_reg(1, "r13"); // len self.emit(" call _rt_right"); @@ -5526,6 +5662,11 @@ impl CodeGen { self.emit(" push r12"); self.emit(" push r13"); self.emit(" push r14"); + // Three pushes leave rsp 8 short of a 16-byte boundary, and + // every call made from here -- _rt_mid, and any error + // trampoline an argument check jumps to -- must be aligned. + // STRING$ below already pairs its odd push with this. + self.emit(" sub rsp, 8"); self.gen_expr(&args[0]); self.emit(" mov r12, rax"); // save ptr self.emit(" mov r13, rdx"); // save len @@ -5535,21 +5676,28 @@ impl CodeGen { } else { self.emit(" cvttsd2si r14, xmm0"); // save start } + // String indexing is 1-based, so a start below 1 is illegal. + self.emit_arg_min_check("r14", 1); let arg3 = Self::arg_reg(3); if args.len() > 2 { let len_type = self.gen_expr(&args[2]); // count - safe now if len_type.is_integer() { - self.emit(&format!(" movsxd {}, eax", arg3)); + emit!(self, " movsxd {}, eax", arg3); } else { - self.emit(&format!(" cvttsd2si {}, xmm0", arg3)); + emit!(self, " cvttsd2si {}, xmm0", arg3); } + // Only a count the program actually wrote is checked: the + // -1 below is this compiler's sentinel for "the rest", and + // _rt_mid reads it as such. + self.emit_arg_min_check(arg3, 0); } else { - self.emit(&format!(" mov {}, -1", arg3)); // rest of string + emit!(self, " mov {}, -1", arg3); // rest of string } self.emit_arg_reg(0, "r12"); // ptr self.emit_arg_reg(1, "r13"); // len self.emit_arg_reg(2, "r14"); // start self.emit(" call _rt_mid"); + self.emit(" add rsp, 8"); self.emit(" pop r14"); self.emit(" pop r13"); self.emit(" pop r12"); @@ -5579,6 +5727,10 @@ impl CodeGen { // Evaluate haystack and save self.emit(" push r12"); self.emit(" push r13"); + // Three pushes in all, counting rbx above: pad so that every + // call from here is 16-byte aligned, including the error + // trampoline _rt_instr's own start check may reach. + self.emit(" sub rsp, 8"); self.gen_expr(hay_arg); self.emit(" mov r12, rax"); // haystack ptr self.emit(" mov r13, rdx"); // haystack len @@ -5594,6 +5746,7 @@ impl CodeGen { // V's third argument register. self.emit_call_with_args("_rt_instr", &["r12", "r13", "rax", "rdx", "rbx"]); + self.emit(" add rsp, 8"); self.emit(" pop r13"); self.emit(" pop r12"); self.emit(" pop rbx"); @@ -5602,6 +5755,10 @@ impl CodeGen { } "ASC" => { self.gen_expr(&args[0]); + // The empty string has no first character. This read one + // anyway -- whatever byte the pointer happened at -- and + // answered 0 for a string with no bytes at all. + self.emit_arg_min_check("rdx", 1); self.emit(" movzx eax, BYTE PTR [rax]"); // ASC returns integer in eax (Long type) } @@ -5624,6 +5781,8 @@ impl CodeGen { let t = self.gen_expr(&args[0]); self.gen_coercion(t, DataType::Long); self.emit(" movsxd rax, eax"); + // A negative count used to clamp to the empty string. + self.emit_arg_min_check("rax", 0); self.emit(" push rax"); self.emit(" sub rsp, 8"); // keep rsp 16-byte aligned let ct = self.gen_expr(&args[1]); @@ -5662,7 +5821,7 @@ impl CodeGen { self.gen_dim_index(dim, rank); } } - self.emit(&format!(" mov eax, {}", self.symbols.option_base)); + emit!(self, " mov eax, {}", self.symbols.option_base); return; } @@ -5677,14 +5836,14 @@ impl CodeGen { // to a fixed descriptor slot. None | Some((_, Some(_))) => { let dim = args.get(1).and_then(|d| self.const_dim(d)).unwrap_or(1); - self.emit(&format!(" mov rax, {}", loc.q(dim))); + emit!(self, " mov rax, {}", loc.q(dim)); } // A computed dimension indexes the descriptor at run time. Some((dim, None)) => { self.gen_dim_index(dim, rank); match &loc { - Loc::Global(sym) => self.emit(&format!(" lea rcx, [rip + {}]", sym)), - Loc::Frame(off) => self.emit(&format!(" lea rcx, [rbp + {}]", off)), + Loc::Global(sym) => emit!(self, " lea rcx, [rip + {}]", sym), + Loc::Frame(off) => emit!(self, " lea rcx, [rbp + {}]", off), } self.emit(" mov rax, QWORD PTR [rcx + rax*8]"); } @@ -5702,7 +5861,7 @@ impl CodeGen { "LOC" => "_rt_file_loc", _ => "_rt_file_lof", }; - self.emit(&format!(" call {}", rt)); + emit!(self, " call {}", rt); } // STR$ renders what PRINT would, which means picking the same // table PRINT would: a SINGLE carries ~7 digits, and rendering it @@ -5762,7 +5921,7 @@ impl CodeGen { "CVS" => "_rt_cvs", _ => "_rt_cvd", }; - self.emit(&format!(" call {}", rt)); + emit!(self, " call {}", rt); } // Print positioning. These emit output rather than yielding a // value, so they are only meaningful inside PRINT. @@ -5803,7 +5962,7 @@ impl CodeGen { let mangled = mangle(&upper); if args.is_empty() { - self.emit(&format!(" call _proc_{}", mangled)); + emit!(self, " call _proc_{}", mangled); return; } @@ -5816,7 +5975,7 @@ impl CodeGen { .map(|t| Self::words_for(*t) as usize) .sum(); let temp_bytes = ((words * 8 + 15) & !15) as i32; - self.emit(&format!(" sub rsp, {}", temp_bytes)); + emit!(self, " sub rsp, {}", temp_bytes); let param_decls: Vec = self .symbols @@ -5838,7 +5997,7 @@ impl CodeGen { }) = param_decls.get(i) { if self.gen_record_addr(arg) { - self.emit(&format!(" mov QWORD PTR [rsp + {}], rax", w * 8)); + emit!(self, " mov QWORD PTR [rsp + {}], rax", w * 8); temp_of.push(w * 8); w += 1; continue; @@ -5846,15 +6005,15 @@ impl CodeGen { } if *ty == DataType::String { self.gen_expr(arg); - self.emit(&format!(" mov QWORD PTR [rsp + {}], rax", w * 8)); - self.emit(&format!(" mov QWORD PTR [rsp + {}], rdx", w * 8 + 8)); + emit!(self, " mov QWORD PTR [rsp + {}], rax", w * 8); + emit!(self, " mov QWORD PTR [rsp + {}], rdx", w * 8 + 8); temp_of.push(w * 8); w += 2; } else { // Numeric arguments travel as f64 bit patterns in integer // slots; the callee narrows to the declared type. self.gen_expr_to_double(arg); - self.emit(&format!(" movsd QWORD PTR [rsp + {}], xmm0", w * 8)); + emit!(self, " movsd QWORD PTR [rsp + {}], xmm0", w * 8); temp_of.push(w * 8); w += 1; } @@ -5863,23 +6022,21 @@ impl CodeGen { // Phase 2: copy the stack-passed slots into place. let stack_bytes = ((stack_slots * 8 + 15) & !15) as i32; if stack_slots > 0 { - self.emit(&format!(" sub rsp, {}", stack_bytes)); + emit!(self, " sub rsp, {}", stack_bytes); } for (place, off) in places.iter().zip(&temp_of) { // r11 is caller-saved and an argument register on neither ABI. if let Slot::Stk(i) = place.ptr { - self.emit(&format!( - " mov r11, QWORD PTR [rsp + {}]", - stack_bytes + off - )); - self.emit(&format!(" mov QWORD PTR [rsp + {}], r11", i as i32 * 8)); + emit!(self, " mov r11, QWORD PTR [rsp + {}]", stack_bytes + off); + emit!(self, " mov QWORD PTR [rsp + {}], r11", i as i32 * 8); } if let Some(Slot::Stk(i)) = place.len { - self.emit(&format!( + emit!( + self, " mov r11, QWORD PTR [rsp + {}]", stack_bytes + off + 8 - )); - self.emit(&format!(" mov QWORD PTR [rsp + {}], r11", i as i32 * 8)); + ); + emit!(self, " mov QWORD PTR [rsp + {}], r11", i as i32 * 8); } } @@ -5887,23 +6044,25 @@ impl CodeGen { let regs = PlatformAbi::INT_ARG_REGS; for (place, off) in places.iter().zip(&temp_of) { if let Slot::Reg(i) = place.ptr { - self.emit(&format!( + emit!( + self, " mov {}, QWORD PTR [rsp + {}]", regs[i], stack_bytes + off - )); + ); } if let Some(Slot::Reg(i)) = place.len { - self.emit(&format!( + emit!( + self, " mov {}, QWORD PTR [rsp + {}]", regs[i], stack_bytes + off + 8 - )); + ); } } - self.emit(&format!(" call _proc_{}", mangled)); - self.emit(&format!(" add rsp, {}", stack_bytes + temp_bytes)); + emit!(self, " call _proc_{}", mangled); + emit!(self, " add rsp, {}", stack_bytes + temp_bytes); } /// Emit storage for one DIM/REDIM declarator. @@ -5980,11 +6139,11 @@ impl CodeGen { // With PRESERVE, the old element count is needed to know where the // newly added tail begins. if preserve { - self.emit(&format!(" mov rax, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(1)); for i in 1..ndims { - self.emit(&format!(" imul rax, {}", loc.q(1 + i as i32))); + emit!(self, " imul rax, {}", loc.q(1 + i as i32)); } - self.emit(&format!(" imul rax, {}", elem_size)); + emit!(self, " imul rax, {}", elem_size); self.emit(" push rax"); // old size in bytes self.emit(" sub rsp, 8"); // keep rsp 16-byte aligned } @@ -6000,22 +6159,22 @@ impl CodeGen { self.emit(" cvttsd2si rax, xmm0"); } self.emit(" inc rax"); // DIM A(N) has N+1 elements (0 to N) - self.emit(&format!(" mov {}, rax", loc.q(1 + i as i32))); + emit!(self, " mov {}, rax", loc.q(1 + i as i32)); } // Calculate total elements: dim0 * dim1 * dim2 * ... - self.emit(&format!(" mov rax, {}", loc.q(1))); + emit!(self, " mov rax, {}", loc.q(1)); for i in 1..ndims { - self.emit(&format!(" imul rax, {}", loc.q(1 + i as i32))); + emit!(self, " imul rax, {}", loc.q(1 + i as i32)); } - self.emit(&format!(" imul rax, {}", elem_size)); + emit!(self, " imul rax, {}", elem_size); if preserve { // realloc(old_ptr, new_size) self.emit(" mov r10, rax"); // new size in bytes self.emit(" push r10"); self.emit(" sub rsp, 8"); // keep rsp 16-byte aligned across the call - self.emit(&format!(" mov {}, {}", Self::arg_reg(0), loc.q(0))); + emit!(self, " mov {}, {}", Self::arg_reg(0), loc.q(0)); self.emit_arg_reg(1, "r10"); self.emit_call_libc("realloc"); self.emit(" add rsp, 8"); @@ -6023,7 +6182,7 @@ impl CodeGen { } else { // calloc(1, size): BASIC guarantees a fresh array reads as 0 / "", // which malloc alone does not. - self.emit(&format!(" mov {}, 1", Self::arg_reg(0))); + emit!(self, " mov {}, 1", Self::arg_reg(0)); self.emit_arg_reg(1, "rax"); self.emit_call_libc("calloc"); } @@ -6036,7 +6195,7 @@ impl CodeGen { } // Store array pointer - self.emit(&format!(" mov {}, rax", loc.q(0))); + emit!(self, " mov {}, rax", loc.q(0)); if preserve { // Zero the newly added tail, from the old size up to the new one. @@ -6047,10 +6206,10 @@ impl CodeGen { let done_label = self.new_label("preserve_done"); self.emit_label(&loop_label); self.emit(" cmp rcx, r10"); - self.emit(&format!(" jae {}", done_label)); + emit!(self, " jae {}", done_label); self.emit(" mov BYTE PTR [rax + rcx], 0"); self.emit(" inc rcx"); - self.emit(&format!(" jmp {}", loop_label)); + emit!(self, " jmp {}", loop_label); self.emit_label(&done_label); } @@ -6102,7 +6261,7 @@ impl CodeGen { // body never runs must not report an array it never touched. if self.opts.checks { let base = self.array_base_operand(name, &loc); - self.emit(&format!(" cmp {}, 0", base)); + emit!(self, " cmp {}, 0", base); self.emit_check("je", RtError::Undim); } @@ -6121,7 +6280,7 @@ impl CodeGen { // bound is already the element count (declared bound + 1). if self.opts.checks { let bound = self.array_bound_operand(name, &loc, 0); - self.emit(&format!(" cmp rax, {}", bound)); + emit!(self, " cmp rax, {}", bound); self.emit_check("jae", RtError::Subscript); self.emit_lower_bound_check("rax"); } @@ -6129,7 +6288,7 @@ impl CodeGen { // For each subsequent index, multiply by dimension bound and add for (i, idx_expr) in indices.iter().enumerate().skip(1) { // Save current accumulated index - use 16 bytes for alignment - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); // Evaluate next index let idx_type = self.gen_expr(idx_expr); @@ -6140,14 +6299,14 @@ impl CodeGen { } let bound = self.array_bound_operand(name, &loc, i); if self.opts.checks { - self.emit(&format!(" cmp rcx, {}", bound)); + emit!(self, " cmp rcx, {}", bound); self.emit_check("jae", RtError::Subscript); self.emit_lower_bound_check("rcx"); } self.emit(" mov rax, QWORD PTR [rsp]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); // rax = rax * dim[i] + indices[i] - self.emit(&format!(" imul rax, {}", bound)); + emit!(self, " imul rax, {}", bound); self.emit(" add rax, rcx"); } @@ -6164,11 +6323,11 @@ impl CodeGen { // index runs arbitrary code, which is free to clobber r10. if Self::is_sib_scale(elem_size) { let base = self.array_base_into_register(name, &loc); - self.emit(&format!(" lea rax, [{} + rax*{}]", base, elem_size)); + emit!(self, " lea rax, [{} + rax*{}]", base, elem_size); } else { - self.emit(&format!(" imul rax, {}", elem_size)); + emit!(self, " imul rax, {}", elem_size); let base = self.array_base_operand(name, &loc); - self.emit(&format!(" add rax, {}", base)); + emit!(self, " add rax, {}", base); } } @@ -6183,19 +6342,19 @@ impl CodeGen { let base = self.array_base_into_register(name, &loc); let at = format!("[{} + rax*{}]", base, elem_size); match elem_type { - DataType::Integer => self.emit(&format!(" movsx eax, WORD PTR {}", at)), - DataType::Long => self.emit(&format!(" mov eax, DWORD PTR {}", at)), - DataType::Single => self.emit(&format!(" movss xmm0, DWORD PTR {}", at)), - DataType::Double => self.emit(&format!(" movsd xmm0, QWORD PTR {}", at)), + DataType::Integer => emit!(self, " movsx eax, WORD PTR {}", at), + DataType::Long => emit!(self, " mov eax, DWORD PTR {}", at), + DataType::Single => emit!(self, " movss xmm0, DWORD PTR {}", at), + DataType::Double => emit!(self, " movsd xmm0, QWORD PTR {}", at), DataType::String => unreachable!("excluded above"), } return; } // Otherwise build the address and read through it. - self.emit(&format!(" imul rax, {}", elem_size)); + emit!(self, " imul rax, {}", elem_size); let base = self.array_base_operand(name, &loc); - self.emit(&format!(" add rax, {}", base)); + emit!(self, " add rax, {}", base); match elem_type { DataType::String => { self.emit(" mov rcx, rax"); @@ -6213,7 +6372,7 @@ impl CodeGen { self.gen_array_addr(name, indices); // Save the address while the value is evaluated - 16 bytes for alignment - self.emit(&format!(" sub rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " sub rsp, {}", STACK_TEMP_SPACE); self.emit(" mov QWORD PTR [rsp], rax"); let val_type = self.gen_expr(value); @@ -6222,7 +6381,7 @@ impl CodeGen { } self.emit(" mov rcx, QWORD PTR [rsp]"); - self.emit(&format!(" add rsp, {}", STACK_TEMP_SPACE)); + emit!(self, " add rsp, {}", STACK_TEMP_SPACE); let elem_type = DataType::from_suffix(name); if elem_type == DataType::String { @@ -6248,8 +6407,8 @@ impl CodeGen { // Both words were reserved when the variable was first seen, so this no // longer has to scavenge a slot per assignment. let loc = self.get_var_loc(name); - self.emit(&format!(" mov {}, rax", loc.q(0))); - self.emit(&format!(" mov {}, rdx", loc.q(1))); + emit!(self, " mov {}, rax", loc.q(0)); + emit!(self, " mov {}, rdx", loc.q(1)); } fn emit_data_section(&mut self) { @@ -6372,10 +6531,11 @@ impl CodeGen { // GOSUB stack (if needed) if self.gosub_used { - self.emit(&format!( + emit!( + self, "_gosub_stack: .skip {} # GOSUB return stack (64K entries)", GOSUB_STACK_SIZE - )); + ); } } } diff --git a/src/lexer.rs b/src/lexer.rs index a1606c2..9a13da2 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -3,75 +3,103 @@ // Copyright (c) 2025-2026 Jeff Garzik // SPDX-License-Identifier: MIT -use std::collections::HashMap; use std::iter::Peekable; use std::str::Chars; -use std::sync::LazyLock; - -/// Keyword lookup table (initialized once on first use) -static KEYWORDS: LazyLock> = LazyLock::new(|| { - HashMap::from([ - ("PRINT", Token::Print), - ("INPUT", Token::Input), - ("LINE", Token::Line), - ("LET", Token::Let), - ("DIM", Token::Dim), - ("IF", Token::If), - ("THEN", Token::Then), - ("ELSE", Token::Else), - ("ELSEIF", Token::ElseIf), - ("ENDIF", Token::EndIf), - ("FOR", Token::For), - ("TO", Token::To), - ("STEP", Token::Step), - ("NEXT", Token::Next), - ("WHILE", Token::While), - ("WEND", Token::Wend), - ("DO", Token::Do), - ("LOOP", Token::Loop), - ("UNTIL", Token::Until), - ("GOTO", Token::Goto), - ("GOSUB", Token::Gosub), - ("RETURN", Token::Return), - ("ON", Token::On), - ("SUB", Token::Sub), - ("ENDSUB", Token::EndSub), - ("FUNCTION", Token::Function), - ("ENDFUNCTION", Token::EndFunction), - ("SELECT", Token::Select), - ("CASE", Token::Case), - ("ENDSELECT", Token::EndSelect), - ("END", Token::End), - ("STOP", Token::Stop), - ("REM", Token::Rem), - ("DATA", Token::Data), - ("READ", Token::Read), - ("RESTORE", Token::Restore), - ("CLS", Token::Cls), - ("OPEN", Token::Open), - ("CLOSE", Token::Close), - ("AS", Token::As), - ("OUTPUT", Token::Output), - ("APPEND", Token::Append), - ("AND", Token::And), - ("OR", Token::Or), - ("NOT", Token::Not), - ("XOR", Token::Xor), - ("MOD", Token::Mod), - ("USING", Token::Using), - ("SWAP", Token::Swap), - ("CONST", Token::Const), - ("WRITE", Token::Write), - ("EXIT", Token::Exit), - ("DEF", Token::Def), - ("OPTION", Token::Option), - ("BASE", Token::Base), - ("REDIM", Token::Redim), - ("PRESERVE", Token::Preserve), - ("TYPE", Token::Type), - ("ENDTYPE", Token::EndType), - ]) -}); + +/// An identifier, as the lexer already normalized it. +/// +/// [`Lexer::read_identifier`] uppercases every character of every name, so by +/// the time one reaches the parser, sema or codegen it is *already* upper case. +/// Sixty-odd call sites used to write `name.to_uppercase()` to say so, each +/// allocating a fresh `String` to produce the string it was handed. +/// +/// This states the invariant in one place and checks it, rather than having +/// every consumer defensively re-establish it -- and re-establish it slightly +/// differently, which is how `Symbols::lookup_array` came to uppercase the name +/// for its module-level lookup but not for its procedure-local one. +/// +/// The check is `debug_assert`, so it costs nothing in the shipped compiler and +/// fires during `cargo test` in a debug profile if a name ever arrives raw. +pub fn normalized(name: &str) -> &str { + debug_assert!( + !name.chars().any(char::is_lowercase), + "identifier '{}' was not uppercased by the lexer", + name + ); + name +} + +/// Recognise a keyword. +/// +/// A `match` on the string rather than a `HashMap`: rustc lowers this to a +/// switch on length followed by a memcmp chain, so there is no lazy-init check, +/// no hashing and no clone of the matched token on every identifier scanned. +/// +/// REM and DATA are absent deliberately -- `next_token` intercepts both before +/// this is reached. REM introduces a comment; DATA's operand is raw text rather +/// than a token sequence, so it arrives as a single `Token::DataText`. +fn keyword(s: &str) -> Option { + match s { + "PRINT" => Some(Token::Print), + "INPUT" => Some(Token::Input), + "LINE" => Some(Token::Line), + "LET" => Some(Token::Let), + "DIM" => Some(Token::Dim), + "IF" => Some(Token::If), + "THEN" => Some(Token::Then), + "ELSE" => Some(Token::Else), + "ELSEIF" => Some(Token::ElseIf), + "ENDIF" => Some(Token::EndIf), + "FOR" => Some(Token::For), + "TO" => Some(Token::To), + "STEP" => Some(Token::Step), + "NEXT" => Some(Token::Next), + "WHILE" => Some(Token::While), + "WEND" => Some(Token::Wend), + "DO" => Some(Token::Do), + "LOOP" => Some(Token::Loop), + "UNTIL" => Some(Token::Until), + "GOTO" => Some(Token::Goto), + "GOSUB" => Some(Token::Gosub), + "RETURN" => Some(Token::Return), + "ON" => Some(Token::On), + "SUB" => Some(Token::Sub), + "ENDSUB" => Some(Token::EndSub), + "FUNCTION" => Some(Token::Function), + "ENDFUNCTION" => Some(Token::EndFunction), + "SELECT" => Some(Token::Select), + "CASE" => Some(Token::Case), + "ENDSELECT" => Some(Token::EndSelect), + "END" => Some(Token::End), + "STOP" => Some(Token::Stop), + "READ" => Some(Token::Read), + "RESTORE" => Some(Token::Restore), + "CLS" => Some(Token::Cls), + "OPEN" => Some(Token::Open), + "CLOSE" => Some(Token::Close), + "AS" => Some(Token::As), + "OUTPUT" => Some(Token::Output), + "APPEND" => Some(Token::Append), + "AND" => Some(Token::And), + "OR" => Some(Token::Or), + "NOT" => Some(Token::Not), + "XOR" => Some(Token::Xor), + "MOD" => Some(Token::Mod), + "USING" => Some(Token::Using), + "SWAP" => Some(Token::Swap), + "CONST" => Some(Token::Const), + "WRITE" => Some(Token::Write), + "EXIT" => Some(Token::Exit), + "DEF" => Some(Token::Def), + "OPTION" => Some(Token::Option), + "BASE" => Some(Token::Base), + "REDIM" => Some(Token::Redim), + "PRESERVE" => Some(Token::Preserve), + "TYPE" => Some(Token::Type), + "ENDTYPE" => Some(Token::EndType), + _ => None, + } +} #[derive(Debug, Clone, PartialEq)] pub enum Token { @@ -116,8 +144,9 @@ pub enum Token { EndSelect, End, Stop, - Rem, - Data, + /// `DATA` together with its operand, captured as raw source text. + /// See `Lexer::read_data_text` for why it is not tokenized. + DataText(String), Read, Restore, Cls, @@ -174,9 +203,7 @@ pub enum Token { } pub struct Lexer<'a> { - input: &'a str, chars: Peekable>, - pos: usize, line: u32, at_line_start: bool, /// Source line of each token produced by `tokenize`, parallel to its output. @@ -186,9 +213,7 @@ pub struct Lexer<'a> { impl<'a> Lexer<'a> { pub fn new(input: &'a str) -> Self { Lexer { - input, chars: input.chars().peekable(), - pos: 0, line: 1, at_line_start: true, lines: Vec::new(), @@ -196,11 +221,7 @@ impl<'a> Lexer<'a> { } fn advance(&mut self) -> Option { - let c = self.chars.next(); - if let Some(ch) = c { - self.pos += ch.len_utf8(); - } - c + self.chars.next() } fn peek(&mut self) -> Option { @@ -227,9 +248,9 @@ impl<'a> Lexer<'a> { } } + /// Scan a string literal. The opening quote is already consumed. fn read_string(&mut self) -> Result { let mut s = String::new(); - self.advance(); // consume opening " loop { match self.advance() { Some('"') => { @@ -250,6 +271,36 @@ impl<'a> Lexer<'a> { Ok(s) } + /// Scan the operand of a `DATA` statement as raw source text. + /// + /// DATA items are not expressions and cannot be reassembled from tokens: + /// [`Self::read_identifier`] uppercases, so `DATA hello` would come back as + /// `HELLO`, and `DATA 007` and `DATA 1.50` would lose their spelling. So the + /// text is taken verbatim and split by the parser, which is also how the + /// interpreters this follows did it. + /// + /// Scanning stops at end of line, or at a colon *outside* quotes so that a + /// statement may follow on the same line -- which is what this compiler has + /// always allowed. GW-BASIC runs DATA to the end of the line and treats a + /// colon as data; nothing in the wild depends on that, and stopping keeps + /// `DATA 1,2 : PRINT 3` working. + /// + /// The newline itself is left for `next_token`, which owns `at_line_start`. + fn read_data_text(&mut self) -> String { + let mut s = String::new(); + let mut in_quotes = false; + while let Some(c) = self.peek() { + match c { + '\n' => break, + ':' if !in_quotes => break, + '"' => in_quotes = !in_quotes, + _ => {} + } + s.push(self.advance().unwrap()); + } + s + } + /// Scan a decimal number. /// /// An integer too large for LONG becomes a Double rather than silently @@ -287,21 +338,32 @@ impl<'a> Lexer<'a> { // Replace D with E for parsing let s = s.replace(['d', 'D'], "e"); + // `parse::()` answers `inf` for a literal too large to represent + // rather than failing, so an overflow used to slip through as a silent + // infinity -- the one numeric form in this lexer that guessed. + let finite = |v: f64, text: &str| { + if v.is_finite() { + Ok(Token::Float(v)) + } else { + Err(format!("number '{}' is too large", text)) + } + }; + if is_float { - return s - .parse::() - .map(Token::Float) - .map_err(|_| format!("malformed number '{}'", s)); + return match s.parse::() { + Ok(v) => finite(v, &s), + Err(_) => Err(format!("malformed number '{}'", s)), + }; } match s.parse::() { Ok(n) => Ok(Token::Integer(n as i64)), // Outside LONG range: widen to Double, as MS BASIC does, rather // than truncating to 32 bits. - Err(_) => s - .parse::() - .map(Token::Float) - .map_err(|_| format!("number '{}' is too large", s)), + Err(_) => match s.parse::() { + Ok(v) => finite(v, &s), + Err(_) => Err(format!("number '{}' is too large", s)), + }, } } @@ -334,6 +396,13 @@ impl<'a> Lexer<'a> { } } + /// Scan an identifier, uppercasing it. + /// + /// This is where BASIC's case-insensitivity is implemented, and it is the + /// only place: every name that reaches the AST has been through here, and + /// the entry gate in `next_token` is `is_ascii_alphabetic`, so there is no + /// Unicode folding to worry about and the type suffixes (`% & ! # $`) are + /// case-invariant. See [`normalized`] for the invariant this establishes. fn read_identifier(&mut self, first: char) -> String { let mut s = String::new(); s.push(first.to_ascii_uppercase()); @@ -367,15 +436,27 @@ impl<'a> Lexer<'a> { if s.ends_with(['%', '&', '!', '#', '$']) { return Token::Ident(s.to_string()); } - KEYWORDS - .get(s) - .cloned() - .unwrap_or_else(|| Token::Ident(s.to_string())) + keyword(s).unwrap_or_else(|| Token::Ident(s.to_string())) } pub fn next_token(&mut self) -> Result { self.skip_whitespace(); + // A trailing `_` joins this line to the next one. The newline is + // swallowed so no Newline token is produced and the statement carries + // on, but the line counter still advances so diagnostics keep pointing + // at the line the text was written on. + while self.peek() == Some('_') { + self.advance(); + self.skip_whitespace(); + if self.peek() != Some('\n') { + return Err("'_' continues a line, so nothing may follow it".to_string()); + } + self.advance(); + self.line += 1; + self.skip_whitespace(); + } + // Check for line number at start of line if self.at_line_start { if let Some(c) = self.peek() { @@ -390,7 +471,15 @@ impl<'a> Lexer<'a> { } self.at_line_start = false; self.skip_whitespace(); - return Ok(Token::LineNumber(num.parse().unwrap_or(0))); + // A number too large to represent used to become label 0, + // so the program silently defined a label nobody wrote and + // every GOTO to it failed separately -- past LONG range the + // same digits lex as a Double. Every other numeric form in + // this lexer refuses to guess; so does this one. + return match num.parse::() { + Ok(n) => Ok(Token::LineNumber(n)), + Err(_) => Err(format!("line number '{}' is too large", num)), + }; } } } @@ -408,12 +497,7 @@ impl<'a> Lexer<'a> { Ok(Token::Newline) } - '"' => { - self.pos -= 1; // back up to re-read the quote - self.chars = self.input[self.pos..].chars().peekable(); - let s = self.read_string()?; - Ok(Token::String(s)) - } + '"' => Ok(Token::String(self.read_string()?)), '\'' => { self.skip_comment(); @@ -496,6 +580,13 @@ impl<'a> Lexer<'a> { return Ok(Token::Newline); } + // DATA's operand is raw text, not a sequence of tokens; see + // `read_data_text`. The comparison is against the bare word, so + // a suffixed `Data$` is still an ordinary variable. + if ident == "DATA" { + return Ok(Token::DataText(self.read_data_text())); + } + Ok(self.keyword_or_ident(&ident)) } @@ -733,11 +824,41 @@ mod tests { #[test] fn test_keywords_data() { - let mut lexer = Lexer::new("DATA READ RESTORE"); + let mut lexer = Lexer::new("READ RESTORE"); + let tokens = lexer.tokenize().unwrap(); + assert_eq!(tokens[0], Token::Read); + assert_eq!(tokens[1], Token::Restore); + } + + /// DATA takes the rest of its line as raw text, keeping case and spacing. + /// Reassembling from tokens would uppercase the words and renormalize the + /// numbers, so it is captured verbatim and split by the parser. + #[test] + fn test_data_operand_is_raw_text() { + let mut lexer = Lexer::new("DATA hello, 007, 1.50\nPRINT 1"); + let tokens = lexer.tokenize().unwrap(); + assert_eq!(tokens[0], Token::DataText(" hello, 007, 1.50".to_string())); + assert_eq!(tokens[1], Token::Newline); + assert_eq!(tokens[2], Token::Print); + } + + /// A colon outside quotes ends the statement; one inside is data. + #[test] + fn test_data_stops_at_an_unquoted_colon() { + let mut lexer = Lexer::new("DATA 1, \"a:b\" : PRINT 2"); + let tokens = lexer.tokenize().unwrap(); + assert_eq!(tokens[0], Token::DataText(" 1, \"a:b\" ".to_string())); + assert_eq!(tokens[1], Token::Colon); + assert_eq!(tokens[2], Token::Print); + } + + /// A suffixed `Data$` is an ordinary variable, not the keyword. + #[test] + fn test_data_with_a_suffix_is_an_identifier() { + let mut lexer = Lexer::new("Data$ = \"c\""); let tokens = lexer.tokenize().unwrap(); - assert_eq!(tokens[0], Token::Data); - assert_eq!(tokens[1], Token::Read); - assert_eq!(tokens[2], Token::Restore); + assert_eq!(tokens[0], Token::Ident("DATA$".to_string())); + assert_eq!(tokens[1], Token::Eq); } #[test] @@ -959,4 +1080,30 @@ mod tests { assert!(result.is_err()); assert!(result.unwrap_err().contains("Unexpected character")); } + + // The uppercase invariant + + /// Every identifier the lexer produces satisfies `normalized`, whatever + /// case it was written in and whatever suffix it carries. + #[test] + fn test_every_identifier_is_normalized() { + let mut lexer = Lexer::new("MyVar counter FOO123 a$ b% c& d! e# under_score"); + for tok in lexer.tokenize().unwrap() { + if let Token::Ident(name) = &tok { + assert_eq!(normalized(name), name); + } + } + } + + /// And the guard is real: it fires on a name that skipped the lexer. + /// + /// Without this the invariant would be documented and unenforced, which is + /// how sixty-odd call sites came to re-assert it defensively in the first + /// place. + #[test] + #[should_panic(expected = "was not uppercased")] + #[cfg(debug_assertions)] + fn test_normalized_rejects_a_raw_name() { + normalized("lowercase"); + } } diff --git a/src/main.rs b/src/main.rs index 59b8fda..225c0d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,6 +53,23 @@ fn locate(file: &str, line: u32) -> String { } } +/// Print a batch of diagnostics and exit. +/// +/// Shared by the parser and by sema so the two stages report identically: both +/// now produce a list rather than a single error, and a program with several +/// mistakes should not look different depending on which pass found them. +fn report(file: &str, diagnostics: &[(u32, String, Option)]) -> ! { + for (line, message, note) in diagnostics { + eprintln!("{}: error: {}", locate(file, *line), message); + if let Some(note) = note { + eprintln!("{}: note: {}", locate(file, *line), note); + } + } + let n = diagnostics.len(); + eprintln!("xbasic64: {} error{}", n, if n == 1 { "" } else { "s" }); + std::process::exit(1); +} + /// Whether a failed Windows link looks like GNU coreutils' `link` rather than /// the MSVC linker. /// @@ -91,27 +108,28 @@ fn main() { // Parse let mut parser = parser::Parser::new(tokens, line_map); - let program = match parser.parse() { + let mut program = match parser.parse() { Ok(p) => p, - Err(e) => { - eprintln!("{}: error: {}", locate(input_file, e.line), e); - std::process::exit(1); - } + Err(errors) => report( + input_file, + &errors + .iter() + .map(|e| (e.line, e.to_string(), None)) + .collect::>(), + ), }; // Semantic analysis: reject bad programs here, with a source line, rather // than letting them reach codegen and become a panic or a linker error. - let (symbols, diagnostics) = sema::analyze(&program); + let (symbols, diagnostics) = sema::analyze(&mut program); if !diagnostics.is_empty() { - for d in &diagnostics { - eprintln!("{}: error: {}", locate(input_file, d.line), d.message); - if let Some(note) = &d.note { - eprintln!("{}: note: {}", locate(input_file, d.line), note); - } - } - let n = diagnostics.len(); - eprintln!("xbasic64: {} error{}", n, if n == 1 { "" } else { "s" }); - std::process::exit(1); + report( + input_file, + &diagnostics + .iter() + .map(|d| (d.line, d.message.clone(), d.note.clone())) + .collect::>(), + ); } // Generate code @@ -119,12 +137,12 @@ fn main() { let opts = codegen::Options { checks: !args.no_checks, }; - let asm = codegen.generate(&program, symbols, opts); - - // Add runtime - let runtime_asm = runtime::generate_runtime(); + let mut full_asm = codegen.generate(&program, symbols, opts); - let full_asm = format!("{}\n{}", asm, runtime_asm); + // Append the runtime rather than `format!("{}\n{}", ..)`, which would build + // a third copy of an assembly text that reaches 145 MB on a large program. + full_asm.push('\n'); + full_asm.push_str(&runtime::generate_runtime()); // Determine output file names - put temp files next to output let input_path = Path::new(&input_file); diff --git a/src/parser.rs b/src/parser.rs index bc823aa..eeec215 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -4,25 +4,31 @@ // SPDX-License-Identifier: MIT use crate::lexer::Token; -use std::collections::HashSet; +use std::collections::{HashSet, VecDeque}; /// Binary operator precedence levels (higher = tighter binding) +/// +/// These are LANGREF's table read upside down -- it numbers 1 as tightest, this +/// numbers 1 as loosest. `XOR` used to sit alone at 3, binding tighter than +/// `AND`, which contradicted both LANGREF and GW-BASIC and made +/// `-1 OR 0 XOR -1` give -1 instead of 0. +/// /// Returns (precedence, BinaryOp) or None if not a binary operator fn binary_op_info(token: &Token) -> Option<(u8, BinaryOp)> { match token { - // Precedence 1: logical OR (lowest) + // Precedence 1: logical OR and XOR (lowest), which share a level Token::Or => Some((1, BinaryOp::Or)), + Token::Xor => Some((1, BinaryOp::Xor)), // Precedence 2: logical AND Token::And => Some((2, BinaryOp::And)), - // Precedence 3: logical XOR - Token::Xor => Some((3, BinaryOp::Xor)), + // Precedence 3 is NOT, a prefix operator; see `parse_prec_inner`. // Precedence 4: comparison - Token::Eq => Some((4, BinaryOp::Eq)), - Token::Ne => Some((4, BinaryOp::Ne)), - Token::Lt => Some((4, BinaryOp::Lt)), - Token::Gt => Some((4, BinaryOp::Gt)), - Token::Le => Some((4, BinaryOp::Le)), - Token::Ge => Some((4, BinaryOp::Ge)), + Token::Eq => Some((CMP_PREC, BinaryOp::Eq)), + Token::Ne => Some((CMP_PREC, BinaryOp::Ne)), + Token::Lt => Some((CMP_PREC, BinaryOp::Lt)), + Token::Gt => Some((CMP_PREC, BinaryOp::Gt)), + Token::Le => Some((CMP_PREC, BinaryOp::Le)), + Token::Ge => Some((CMP_PREC, BinaryOp::Ge)), // Precedence 5: additive Token::Plus => Some((5, BinaryOp::Add)), Token::Minus => Some((5, BinaryOp::Sub)), @@ -31,15 +37,34 @@ fn binary_op_info(token: &Token) -> Option<(u8, BinaryOp)> { Token::Slash => Some((6, BinaryOp::Div)), Token::Backslash => Some((6, BinaryOp::IntDiv)), Token::Mod => Some((6, BinaryOp::Mod)), - // Precedence 7: power (handled specially for right-associativity) + // Precedence 7: power Token::Caret => Some((POWER_PREC, BinaryOp::Pow)), _ => None, } } +/// Precedence of the comparison operators. +/// +/// Named because `NOT` sits directly below it: `NOT` takes an operand at this +/// level, which is what makes `NOT A = B` group as `NOT (A = B)` while +/// `NOT A AND B` groups as `(NOT A) AND B`. +const CMP_PREC: u8 = 4; + /// Precedence of `^`, the tightest-binding binary operator. const POWER_PREC: u8 = 7; +/// How deeply expressions and blocks may nest before the parser gives up. +/// +/// This is a recursive-descent parser, so nesting costs stack. Without a bound +/// it did not refuse deep input, it *died* on it: 50,000 nested parentheses -- +/// or the same number of unary minuses or `NOT`s, which descend through the +/// same path -- aborted the process with "fatal runtime error: stack overflow" +/// and exit code 134, which is outside anything a caller can interpret. +/// +/// The limit is far above what a person writes and far below what the stack +/// can take, so the only programs it rejects are ones that were going to crash. +const MAX_DEPTH: u32 = 256; + // AST Definitions #[derive(Debug, Clone)] @@ -81,6 +106,13 @@ pub enum StmtKind { }, Input { prompt: Option, + /// Whether to print `? ` after the prompt. + /// + /// GW-BASIC decides this by the separator: `INPUT "p"; A` prints `p? ` + /// and `INPUT "p", A` prints `p` alone, while a promptless `INPUT A` + /// prints just `? `. The parser used to accept either separator and + /// discard which, so no form ever printed a question mark. + query: bool, vars: Vec, /// `INPUT #n` reads from a file; `None` is the console. file_num: Option, @@ -490,12 +522,17 @@ pub fn child_bodies(stmt: &Stmt) -> Vec<&[Stmt]> { /// A block-closing keyword, consumed by `parse_statement` on behalf of the /// enclosing block parser. /// -/// These are not errors. BASIC's block terminators are statements -/// syntactically, but they belong to the construct that opened the block, so -/// `parse_statement` reports them through the error channel and the enclosing -/// parser (`parse_if_body`, `parse_for`, ...) treats the matching one as a +/// BASIC's block terminators are statements syntactically, but they belong to +/// the construct that opened the block, so `parse_statement` hands the matching +/// one back to the enclosing parser (`parse_if_body`, `parse_for`, ...) as a /// normal end-of-body. Any terminator that reaches the top level without a -/// matching opener is rendered as a real diagnostic. +/// matching opener becomes a diagnostic there. +/// +/// This used to travel through the error channel as `ParseError::Block`, which +/// worked but meant `?` could not be trusted: every `?` in the parser might be +/// propagating an ordinary end-of-block rather than a failure, and a stray +/// terminator would be caught by whichever enclosing block parser matched it +/// first. It is now part of [`Parsed`], so `?` carries only real errors. /// /// Conditions travel as payloads rather than through parser fields, so a /// terminator cannot be separated from its expression. @@ -505,7 +542,10 @@ pub enum BlockEnd { EndSub, EndFunction, EndSelect, - Next, + /// `NEXT [var [, var]...]`. The names are carried rather than discarded so + /// that `FOR I ... NEXT J` can be refused and `NEXT J, I` can close both + /// loops. Empty means a bare `NEXT`, which closes the innermost loop. + Next(Vec), Wend, Loop, LoopWhile(Expr), @@ -522,7 +562,7 @@ impl BlockEnd { BlockEnd::EndSub => "END SUB", BlockEnd::EndFunction => "END FUNCTION", BlockEnd::EndSelect => "END SELECT", - BlockEnd::Next => "NEXT", + BlockEnd::Next(_) => "NEXT", BlockEnd::Wend => "WEND", BlockEnd::Loop | BlockEnd::LoopWhile(_) | BlockEnd::LoopUntil(_) => "LOOP", BlockEnd::Else => "ELSE", @@ -537,27 +577,50 @@ impl BlockEnd { BlockEnd::EndSub => "SUB", BlockEnd::EndFunction => "FUNCTION", BlockEnd::EndSelect => "SELECT CASE", - BlockEnd::Next => "FOR", + BlockEnd::Next(_) => "FOR", BlockEnd::Wend => "WHILE", BlockEnd::Loop | BlockEnd::LoopWhile(_) | BlockEnd::LoopUntil(_) => "DO", } } } +/// The result of parsing one statement: the statement, or the terminator that +/// closed the block it was in. +/// +/// Generic over the item so that `parse_statement_kind` (which yields a +/// `StmtKind`) and `parse_statement` (which tags it with a line to make a +/// `Stmt`) can share one type. +#[derive(Debug, Clone)] +pub enum Parsed { + Item(T), + End(BlockEnd), +} + /// Why parsing of a statement stopped. #[derive(Debug, Clone)] pub enum ParseError { - /// A block terminator was consumed; the enclosing block parser handles it. - Block(BlockEnd), - /// A genuine syntax error. + /// A syntax error at the parser's current position. Error(String), + /// A syntax error belonging to an earlier line -- the opener of a block + /// that was never closed. Without this the diagnostic lands on end of file, + /// which is where the parser noticed rather than where the mistake is. + ErrorAt(u32, String), +} + +impl ParseError { + /// The line this error belongs to, if it names one of its own. + fn line(&self) -> Option { + match self { + ParseError::Error(_) => None, + ParseError::ErrorAt(line, _) => Some(*line), + } + } } impl std::fmt::Display for ParseError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - ParseError::Error(msg) => write!(f, "{}", msg), - ParseError::Block(b) => write!(f, "{} without matching {}", b.keyword(), b.opener()), + ParseError::Error(msg) | ParseError::ErrorAt(_, msg) => write!(f, "{}", msg), } } } @@ -576,6 +639,96 @@ impl std::fmt::Display for LocatedParseError { } } +/// How a token is written in BASIC source, for diagnostics. +/// +/// Every fixed token has a spelling, so a diagnostic can quote what the +/// programmer would have typed. Errors used to fall back to `{:?}` and show +/// Rust variant names instead -- "Expected To, got Integer(2)" rather than +/// "expected TO, got 2", and `EndSelect`, `LParen` and `Ne` at people who had +/// written `END SELECT`, `(` and `<>`. +fn token_spelling(tok: &Token) -> Option<&'static str> { + Some(match tok { + Token::Print => "PRINT", + Token::Input => "INPUT", + Token::Line => "LINE", + Token::Let => "LET", + Token::Dim => "DIM", + Token::If => "IF", + Token::Then => "THEN", + Token::Else => "ELSE", + Token::ElseIf => "ELSEIF", + Token::EndIf => "ENDIF", + Token::For => "FOR", + Token::To => "TO", + Token::Step => "STEP", + Token::Next => "NEXT", + Token::While => "WHILE", + Token::Wend => "WEND", + Token::Do => "DO", + Token::Loop => "LOOP", + Token::Until => "UNTIL", + Token::Goto => "GOTO", + Token::Gosub => "GOSUB", + Token::Return => "RETURN", + Token::On => "ON", + Token::Sub => "SUB", + Token::EndSub => "ENDSUB", + Token::Function => "FUNCTION", + Token::EndFunction => "ENDFUNCTION", + Token::Select => "SELECT", + Token::Case => "CASE", + Token::EndSelect => "ENDSELECT", + Token::End => "END", + Token::Stop => "STOP", + Token::DataText(_) => "DATA", + Token::Read => "READ", + Token::Restore => "RESTORE", + Token::Cls => "CLS", + Token::Open => "OPEN", + Token::Close => "CLOSE", + Token::As => "AS", + Token::Output => "OUTPUT", + Token::Append => "APPEND", + Token::And => "AND", + Token::Or => "OR", + Token::Not => "NOT", + Token::Xor => "XOR", + Token::Mod => "MOD", + Token::Using => "USING", + Token::Swap => "SWAP", + Token::Const => "CONST", + Token::Write => "WRITE", + Token::Exit => "EXIT", + Token::Def => "DEF", + Token::Option => "OPTION", + Token::Base => "BASE", + Token::Redim => "REDIM", + Token::Preserve => "PRESERVE", + Token::Type => "TYPE", + Token::EndType => "ENDTYPE", + Token::Plus => "+", + Token::Minus => "-", + Token::Star => "*", + Token::Slash => "/", + Token::Backslash => "\\", + Token::Caret => "^", + Token::Eq => "=", + Token::Ne => "<>", + Token::Lt => "<", + Token::Gt => ">", + Token::Le => "<=", + Token::Ge => ">=", + Token::LParen => "(", + Token::RParen => ")", + Token::Comma => ",", + Token::Semicolon => ";", + Token::Colon => ":", + Token::Hash => "#", + Token::Dot => ".", + _ => return None, + }) +} + /// Human-readable name for a token, for diagnostics. fn describe_token(tok: &Token) -> String { match tok { @@ -585,8 +738,110 @@ fn describe_token(tok: &Token) -> String { Token::String(s) => format!("string \"{}\"", s), Token::Newline => "end of line".to_string(), Token::Eof => "end of file".to_string(), - other => format!("{:?}", other), + Token::LineNumber(n) => format!("line number {}", n), + other => token_spelling(other) + .map(str::to_string) + .unwrap_or_else(|| format!("{:?}", other)), + } +} + +/// One item of a `DATA` statement, as written. +struct DataItem { + text: String, + /// Whether it was written in quotes, which decides both whether the + /// surrounding spaces were significant and whether it is a string. + quoted: bool, +} + +impl DataItem { + /// The value this item denotes. + /// + /// A quoted item is always a string. An unquoted one is whatever it looks + /// like: GW-BASIC decides the type when the item is READ, but the table this + /// compiles to is tagged per item, and the runtime already converts a string + /// entry to a number with `strtod` -- so classifying here loses nothing and + /// keeps numeric DATA on the fast path. + /// + /// An omitted item is the empty string, which reads as 0 or as "". + fn literal(&self) -> Literal { + if self.quoted { + return Literal::String(self.text.clone()); + } + if let Ok(n) = self.text.parse::() { + return Literal::Integer(n as i64); + } + if let Ok(f) = self.text.parse::() { + if f.is_finite() { + return Literal::Float(f); + } + } + Literal::String(self.text.clone()) + } +} + +/// Split a `DATA` operand into its items. +/// +/// Items are separated by commas outside quotes. An unquoted item has its +/// surrounding spaces trimmed; a quoted one keeps everything between the quotes, +/// which is why quotes are needed for an item containing a comma, a colon, or +/// spaces that matter. A doubled `""` inside quotes is one quote character, as +/// everywhere else in the language. +fn split_data_items(text: &str) -> Vec { + // `DATA` with nothing after it declares no items at all, as against + // `DATA ,` which declares two empty ones. + if text.trim().is_empty() { + return Vec::new(); + } + + let mut items = Vec::new(); + let mut chars = text.chars().peekable(); + loop { + while chars.peek() == Some(&' ') || chars.peek() == Some(&'\t') { + chars.next(); + } + + let item = if chars.peek() == Some(&'"') { + chars.next(); + let mut body = String::new(); + while let Some(c) = chars.next() { + if c == '"' { + // A doubled quote is one quote character, as everywhere + // else in the language. + if chars.peek() == Some(&'"') { + chars.next(); + body.push('"'); + continue; + } + break; + } + body.push(c); + } + // Whatever separates the closing quote from the comma is not data. + while chars.peek().is_some_and(|c| *c != ',') { + chars.next(); + } + DataItem { + text: body, + quoted: true, + } + } else { + let mut body = String::new(); + while chars.peek().is_some_and(|c| *c != ',') { + body.push(chars.next().unwrap()); + } + DataItem { + text: body.trim().to_string(), + quoted: false, + } + }; + items.push(item); + + match chars.next() { + Some(',') => continue, + _ => break, + } } + items } /// Shorthand for the parser's result type. @@ -605,11 +860,22 @@ pub struct Parser { /// Source line of each token, parallel to `tokens`. Empty when unknown. lines: Vec, pos: usize, - /// Tracks declared array names for distinguishing array access from function calls - declared_arrays: HashSet, /// SUB/FUNCTION names, collected before parsing so that `Name:` at the /// start of a line is not mistaken for a label definition. declared_procs: HashSet, + /// How deep the recursive descent currently is, so that pathological input + /// is refused rather than overflowing the stack. See [`MAX_DEPTH`]. + depth: u32, + /// Errors found so far. Parsing continues past each one, so a program with + /// several mistakes reports them all rather than one per compile. + errors: Vec, + /// Loop names from a `NEXT I, J` still waiting to close their loops. + /// + /// One NEXT may close several nested loops. The innermost `parse_for` takes + /// the first name and leaves the rest here; each enclosing block body picks + /// the next one up before reading another token, so the terminator reaches + /// every loop it names as the recursion unwinds outward. + pending_next: VecDeque, } impl Parser { @@ -712,12 +978,21 @@ impl Parser { tok } + /// Consume the next token, which must be `expected`. + /// + /// Matching is by variant, so a payload would be ignored -- every caller + /// passes a payload-free token, and `token_spelling` returning `Some` for + /// exactly those is what keeps that honest: a payload-carrying token has no + /// fixed spelling to name in the diagnostic, and is refused here. fn expect(&mut self, expected: Token) -> PResult<()> { + let Some(wanted) = token_spelling(&expected) else { + unreachable!("expect takes a token with a fixed spelling") + }; let tok = self.advance(); if std::mem::discriminant(&tok) == std::mem::discriminant(&expected) { Ok(()) } else { - err(format!("Expected {:?}, got {:?}", expected, tok)) + err(format!("expected {}, got {}", wanted, describe_token(&tok))) } } @@ -727,42 +1002,234 @@ impl Parser { } } - pub fn parse(&mut self) -> Result { - self.parse_program().map_err(|error| LocatedParseError { - // `pos` is left at the token that stopped the parse. - line: self.cur_line(), - error, - }) + /// Parse the whole program, reporting every syntax error it contains. + /// + /// Sema has always returned a `Vec`, so a program with five + /// undefined names is told about all five. The parser stopped at the first + /// error, so five typos meant five compiles. It now recovers in the same + /// way and reports the same way. + pub fn parse(&mut self) -> Result> { + let program = match self.parse_program() { + Ok(p) => p, + // A hard error -- one recovery could not get past, such as an + // unterminated block -- ends the parse, but whatever was already + // collected is still worth reporting alongside it. + Err(error) => { + self.record(error); + return Err(std::mem::take(&mut self.errors)); + } + }; + if self.errors.is_empty() { + Ok(program) + } else { + Err(std::mem::take(&mut self.errors)) + } + } + + /// Note an error, giving it a line if it does not name one of its own. + fn record(&mut self, error: ParseError) { + let line = error.line().unwrap_or_else(|| self.cur_line()); + self.errors.push(LocatedParseError { line, error }); + } + + /// Skip to the next place a statement could begin. + /// + /// Panic-mode recovery. BASIC is line-oriented, which makes this unusually + /// reliable: a newline or a colon ends a statement no matter what went + /// wrong before it, so the parser can pick up with the next one instead of + /// abandoning the file. Stopping *before* the boundary rather than past it + /// leaves any block terminator on that line to be read normally, so one bad + /// statement inside a SUB does not also cost the END SUB. + fn synchronize(&mut self) { + // Whatever a half-parsed statement left queued is meaningless now, and + // a stale name would surface as a terminator somewhere unrelated. + self.pending_next.clear(); + + let start = self.pos; + while !matches!(self.peek(), Token::Newline | Token::Colon | Token::Eof) { + self.advance(); + } + // If the error was already at a boundary, step over it: the caller's + // loop would otherwise see the same token again and spin. + if self.pos == start && !matches!(self.peek(), Token::Eof) { + self.advance(); + } } fn parse_program(&mut self) -> PResult { let mut statements = Vec::new(); self.skip_newlines(); - while !matches!(self.peek(), Token::Eof) { - let stmt = self.parse_statement()?; - statements.push(stmt); + while !matches!(self.peek(), Token::Eof) || !self.pending_next.is_empty() { + // A name left over from `NEXT I, J` has no loop to close: more + // loops were named than were open. Draining it here rather than + // only inside the loop condition matters, because the commonest + // shape -- `FOR I .. NEXT I, J` as the last statement -- leaves the + // token stream at Eof with the name still queued. + if let Some(name) = self.pending_next.pop_front() { + let e = ParseError::Error(format!("NEXT {} without matching FOR", name)); + self.record(e); + continue; + } + match self.parse_statement() { + Ok(Parsed::Item(stmt)) => statements.push(stmt), + // A terminator here closed nothing: there is no enclosing block + // for it to belong to. + Ok(Parsed::End(end)) => { + // Once anything has gone wrong, a stray terminator says + // nothing useful: it is usually the perfectly good closer + // of a block whose *header* failed, so reporting it blames + // a FOR that is sitting right there. Suppress after the + // first error, report it in a clean program. + if self.errors.is_empty() { + let e = ParseError::Error(format!( + "{} without matching {}", + end.keyword(), + end.opener() + )); + self.record(e); + } + self.synchronize(); + } + Err(e) => { + self.record(e); + self.synchronize(); + } + } self.skip_newlines(); } Ok(Program { statements }) } - /// Parse one statement, tagging it with the line it started on. + /// Parse the statements of a block, up to and including its terminator. /// - /// `?` propagates `ParseError::Block` unchanged, so the block-terminator - /// protocol is unaffected by the wrapping. - fn parse_statement(&mut self) -> PResult { + /// Every block-bearing construct shares this. `opener` names the construct + /// and `closer` the keyword it needs, for the diagnostic when end of file + /// arrives first -- none of the seven hand-written loops this replaces + /// checked for EOF at all. They terminated only because an unrecognised + /// token became "Unexpected token: Eof", so an unterminated SUB blamed the + /// last line of the file and named nothing. + fn parse_block_body( + &mut self, + opener: &str, + closer: &str, + opener_line: u32, + ) -> PResult<(Vec, BlockEnd)> { + let mut body = Vec::new(); + loop { + // A `NEXT I, J` left names here for the enclosing loops. Take one + // before looking at the token stream at all -- the NEXT has already + // been consumed, so the next real token is whatever followed it, + // and at the end of a program that is Eof. Checking after the guard + // below would report "FOR is missing its NEXT" with the terminator + // sitting right there. + if let Some(name) = self.pending_next.pop_front() { + return Ok((body, BlockEnd::Next(vec![name]))); + } + if matches!(self.peek(), Token::Eof) { + return Err(ParseError::ErrorAt( + opener_line, + format!("{} is missing its {}", opener, closer), + )); + } + match self.parse_statement() { + Ok(Parsed::Item(stmt)) => body.push(stmt), + Ok(Parsed::End(end)) => return Ok((body, end)), + // Recover here as well as at the top level, so a mistake inside + // a block costs that statement rather than the whole block -- + // otherwise the error would propagate out, the block's own + // terminator would be left stranded, and the reader would get a + // cascade of complaints about a SUB that was perfectly closed. + Err(e) => { + self.record(e); + self.synchronize(); + } + } + self.skip_newlines(); + } + } + + /// A block body that must end with exactly one terminator, named by `want`. + /// + /// `want` is matched by variant, so a payload-free value stands in for the + /// whole family; it also supplies the keyword for both diagnostics. + fn parse_block( + &mut self, + want: BlockEnd, + opener: &str, + opener_line: u32, + ) -> PResult> { + let (body, end) = self.parse_block_body(opener, want.keyword(), opener_line)?; + if std::mem::discriminant(&end) != std::mem::discriminant(&want) { + return Err(ParseError::ErrorAt( + opener_line, + format!( + "{} needs {} to close it, but {} came first", + opener, + want.keyword(), + end.keyword() + ), + )); + } + Ok(body) + } + + /// Parse one statement, tagging it with the line it started on. + fn parse_statement(&mut self) -> PResult> { let line = self.cur_line(); - let kind = self.parse_statement_kind()?; - Ok(Stmt { line, kind }) + Ok(match self.parse_statement_kind()? { + Parsed::Item(kind) => Parsed::Item(Stmt { line, kind }), + Parsed::End(end) => Parsed::End(end), + }) + } + + /// Parse one statement that is known not to close a block. + /// + /// Used where a terminator would be meaningless -- the branches of a + /// single-line IF -- so the caller does not have to invent a diagnostic. + fn parse_inner_statement(&mut self) -> PResult { + match self.parse_statement()? { + Parsed::Item(stmt) => Ok(stmt), + Parsed::End(end) => err(format!( + "{} without matching {}", + end.keyword(), + end.opener() + )), + } + } + + fn parse_statement_kind(&mut self) -> PResult> { + self.depth += 1; + let r = self.parse_statement_kind_inner(); + self.depth -= 1; + r } - fn parse_statement_kind(&mut self) -> PResult { + fn parse_statement_kind_inner(&mut self) -> PResult> { + if self.depth > MAX_DEPTH { + return err(format!("nesting is too deep (limit {} levels)", MAX_DEPTH)); + } + + // Skip any run of separators and blank lines before the statement + // proper. Each of these used to recurse. That is a tail call, so a + // release build optimized it away and only a debug build overflowed on + // a long run of colons -- the worst way to hold a bug, since CI runs + // `cargo test --release`. A loop costs no stack in any profile. + while matches!(self.peek(), Token::Colon | Token::Newline) { + self.advance(); + } + + // A block-closing keyword belongs to the construct that opened the + // block, so hand it back rather than parsing it as a statement. + if let Some(end) = self.try_block_end()? { + return Ok(Parsed::End(end)); + } + // Handle line numbers as labels if let Token::LineNumber(n) = self.peek().clone() { self.advance(); - return Ok(StmtKind::Label(n)); + return Ok(Parsed::Item(StmtKind::Label(n))); } // A named label definition: `Retry:` at the start of a line. @@ -771,16 +1238,10 @@ impl Parser { unreachable!("at_label_definition checked for an identifier") }; self.advance(); // consume ':' - return Ok(StmtKind::LabelName(name)); - } - - // Handle colon as statement separator - if matches!(self.peek(), Token::Colon) { - self.advance(); - return self.parse_statement_kind(); + return Ok(Parsed::Item(StmtKind::LabelName(name))); } - match self.peek().clone() { + let kind = match self.peek().clone() { Token::Print => self.parse_print(false), Token::Write => self.parse_print(true), Token::Swap => self.parse_swap(), @@ -807,7 +1268,7 @@ impl Parser { Token::Type => self.parse_type_def(), Token::Sub => self.parse_sub(), Token::Function => self.parse_function(), - Token::Data => self.parse_data(), + Token::DataText(text) => self.parse_data(&text), Token::Read => self.parse_read(), Token::Restore => self.parse_restore(), Token::Cls => { @@ -816,116 +1277,134 @@ impl Parser { } Token::Open => self.parse_open(), Token::Close => self.parse_close(), + // `try_block_end` has already taken END followed by IF, SUB, + // FUNCTION or SELECT, so a bare END is the statement. Token::End => { self.advance(); - // Check for END IF, END SUB, END FUNCTION, END SELECT - match self.peek() { - Token::If => { - self.advance(); - // Return to caller - this is a terminator, not a statement - Err(ParseError::Block(BlockEnd::EndIf)) - } - Token::Sub => { - self.advance(); - Err(ParseError::Block(BlockEnd::EndSub)) - } - Token::Function => { - self.advance(); - Err(ParseError::Block(BlockEnd::EndFunction)) - } - Token::Select => { - self.advance(); - Err(ParseError::Block(BlockEnd::EndSelect)) - } - _ => Ok(StmtKind::End), + Ok(StmtKind::End) + } + Token::Stop => { + self.advance(); + Ok(StmtKind::Stop) + } + Token::Select => self.parse_select_case(), + // `parse_select_case` consumes CASE itself, so a CASE reaching here + // is always outside any SELECT CASE. + Token::Case => err("CASE without matching SELECT CASE"), + Token::Ident(ref n) => { + // The random-access statements lead with a name rather than a + // reserved word. Each is recognised only in a shape that an + // assignment or a call could not take -- `FIELD #`, `LSET v =` + // -- so programs may still use these names for their own + // variables and procedures. + match n.to_uppercase().as_str() { + "FIELD" if self.next_is(Token::Hash) => self.parse_field(), + "GET" if self.next_is(Token::Hash) => self.parse_get_put(false), + "PUT" if self.next_is(Token::Hash) => self.parse_get_put(true), + "LOCK" if self.next_is(Token::Hash) => self.parse_lock(false), + "UNLOCK" if self.next_is(Token::Hash) => self.parse_lock(true), + "CALL" if self.next_is_ident() => self.parse_call(), + "LSET" if self.next_is_ident() => self.parse_set_field(false), + "RSET" if self.next_is_ident() => self.parse_set_field(true), + _ => self.parse_assignment_or_call(), } } + // Newline and Colon are consumed by the skip loop above, so + // reaching here means the statement itself is unrecognised. + _ => err(format!( + "unexpected {} at the start of a statement", + describe_token(self.peek()) + )), + }?; + Ok(Parsed::Item(kind)) + } + + /// Consume a block-closing keyword if one is next, leaving the position + /// untouched otherwise. + /// + /// `END` is the awkward one: alone it terminates the program, and only the + /// token after it decides. `NEXT` swallows its optional control variable, + /// and `LOOP`/`ELSEIF` carry the condition they were written with, so that + /// a terminator can never be separated from its expression. + fn try_block_end(&mut self) -> PResult> { + let end = match self.peek().clone() { + Token::End => { + let closes = match self.peek_at(1) { + Token::If => BlockEnd::EndIf, + Token::Sub => BlockEnd::EndSub, + Token::Function => BlockEnd::EndFunction, + Token::Select => BlockEnd::EndSelect, + // A bare END is the program-termination statement. + _ => return Ok(None), + }; + self.advance(); + self.advance(); + closes + } Token::EndIf => { self.advance(); - Err(ParseError::Block(BlockEnd::EndIf)) + BlockEnd::EndIf } Token::EndSub => { self.advance(); - Err(ParseError::Block(BlockEnd::EndSub)) + BlockEnd::EndSub } Token::EndFunction => { self.advance(); - Err(ParseError::Block(BlockEnd::EndFunction)) + BlockEnd::EndFunction } Token::EndSelect => { self.advance(); - Err(ParseError::Block(BlockEnd::EndSelect)) - } - Token::Stop => { - self.advance(); - Ok(StmtKind::Stop) + BlockEnd::EndSelect } Token::Next => { self.advance(); - // Skip optional variable name - if let Token::Ident(_) = self.peek() { + // `NEXT`, `NEXT I` or `NEXT I, J, ...`. The names used to be + // swallowed and discarded, so a NEXT could close a loop it did + // not name and a list was a syntax error. + let mut names = Vec::new(); + while let Token::Ident(name) = self.peek().clone() { self.advance(); + names.push(name); + if matches!(self.peek(), Token::Comma) { + self.advance(); + } else { + break; + } } - Err(ParseError::Block(BlockEnd::Next)) + BlockEnd::Next(names) } Token::Wend => { self.advance(); - Err(ParseError::Block(BlockEnd::Wend)) + BlockEnd::Wend } Token::Loop => { self.advance(); - // Check for WHILE/UNTIL condition match self.peek() { Token::While => { self.advance(); - let cond = self.parse_expression()?; - Err(ParseError::Block(BlockEnd::LoopWhile(cond))) + BlockEnd::LoopWhile(self.parse_expression()?) } Token::Until => { self.advance(); - let cond = self.parse_expression()?; - Err(ParseError::Block(BlockEnd::LoopUntil(cond))) + BlockEnd::LoopUntil(self.parse_expression()?) } - _ => Err(ParseError::Block(BlockEnd::Loop)), + _ => BlockEnd::Loop, } } Token::Else => { self.advance(); - Err(ParseError::Block(BlockEnd::Else)) + BlockEnd::Else } Token::ElseIf => { self.advance(); let cond = self.parse_expression()?; self.expect(Token::Then)?; - Err(ParseError::Block(BlockEnd::ElseIf(cond))) + BlockEnd::ElseIf(cond) } - Token::Select => self.parse_select_case(), - // `parse_select_case` consumes CASE itself, so a CASE reaching here - // is always outside any SELECT CASE. - Token::Case => err("CASE without matching SELECT CASE"), - Token::Ident(ref n) => { - // The random-access statements lead with a name rather than a - // reserved word. Each is recognised only in a shape that an - // assignment or a call could not take -- `FIELD #`, `LSET v =` - // -- so programs may still use these names for their own - // variables and procedures. - match n.to_uppercase().as_str() { - "FIELD" if self.next_is(Token::Hash) => self.parse_field(), - "GET" if self.next_is(Token::Hash) => self.parse_get_put(false), - "PUT" if self.next_is(Token::Hash) => self.parse_get_put(true), - "LOCK" if self.next_is(Token::Hash) => self.parse_lock(false), - "UNLOCK" if self.next_is(Token::Hash) => self.parse_lock(true), - "LSET" if self.next_is_ident() => self.parse_set_field(false), - "RSET" if self.next_is_ident() => self.parse_set_field(true), - _ => self.parse_assignment_or_call(), - } - } - Token::Newline => { - self.advance(); - self.parse_statement_kind() - } - _ => err(format!("Unexpected token: {:?}", self.peek())), - } + _ => return Ok(None), + }; + Ok(Some(end)) } fn parse_print(&mut self, write: bool) -> PResult { @@ -1086,6 +1565,7 @@ impl Parser { fn parse_input(&mut self) -> PResult { self.advance(); // consume INPUT + self.skip_input_suppressor(); // Check for INPUT #n (file input) if matches!(self.peek(), Token::Hash) { @@ -1106,6 +1586,8 @@ impl Parser { return Ok(StmtKind::Input { prompt: None, + // A file read prompts for nothing. + query: false, vars, file_num: Some(file_num), }); @@ -1113,14 +1595,25 @@ impl Parser { let mut prompt = None; let mut vars = Vec::new(); + // A promptless INPUT still asks: GW-BASIC prints a bare `? `. + let mut query = true; // Check for prompt string if let Token::String(s) = self.peek().clone() { self.advance(); prompt = Some(s); - // Expect comma or semicolon after prompt - if matches!(self.peek(), Token::Comma | Token::Semicolon) { - self.advance(); + // The separator decides whether a question mark follows the + // prompt: `;` adds one, `,` suppresses it. Both were accepted and + // the choice thrown away, so neither form ever printed one. + match self.peek() { + Token::Semicolon => { + self.advance(); + } + Token::Comma => { + self.advance(); + query = false; + } + _ => {} } } @@ -1136,14 +1629,30 @@ impl Parser { Ok(StmtKind::Input { prompt, + query, vars, file_num: None, }) } + /// Consume the optional `;` that may precede an INPUT prompt. + /// + /// In GW-BASIC this suppresses the newline echoed when the operator presses + /// Return, so the next PRINT continues the same line. That newline is the + /// terminal's echo here -- neither runtime prints one -- so there is nothing + /// on our side to suppress and the form is accepted and ignored. Rejecting + /// it outright would refuse a program for asking about a difference it + /// cannot observe. LANGREF says so. + fn skip_input_suppressor(&mut self) { + if matches!(self.peek(), Token::Semicolon) { + self.advance(); + } + } + fn parse_line_input(&mut self) -> PResult { self.advance(); // consume LINE self.expect(Token::Input)?; + self.skip_input_suppressor(); // LINE INPUT #n, Var$ -- documented in LANGREF but never parsed. let file_num = if matches!(self.peek(), Token::Hash) { @@ -1158,7 +1667,8 @@ impl Parser { let mut prompt = None; - // Check for prompt string + // Check for prompt string. Unlike INPUT, LINE INPUT never adds a + // question mark, so the separator carries no meaning here. if let Token::String(s) = self.peek().clone() { self.advance(); prompt = Some(s); @@ -1195,19 +1705,7 @@ impl Parser { None }; // A record field path may follow: v.field.sub - let mut fields = Vec::new(); - while matches!(self.peek(), Token::Dot) { - self.advance(); - match self.advance() { - Token::Ident(f) => fields.push(f), - tok => { - return err(format!( - "Expected a field name after '.', got {}", - describe_token(&tok) - )); - } - } - } + let fields = self.parse_field_path()?; Ok(LValue { name, indices, @@ -1238,19 +1736,7 @@ impl Parser { // A record field assignment: v.field... = value if matches!(self.peek(), Token::Dot) { - let mut fields = Vec::new(); - while matches!(self.peek(), Token::Dot) { - self.advance(); - match self.advance() { - Token::Ident(f) => fields.push(f), - tok => { - return err(format!( - "Expected a field name after '.', got {}", - describe_token(&tok) - )); - } - } - } + let fields = self.parse_field_path()?; self.expect(Token::Eq)?; let value = self.parse_expression()?; return Ok(StmtKind::FieldAssign { @@ -1274,19 +1760,7 @@ impl Parser { // arr(i).field... = value if matches!(self.peek(), Token::Dot) { - let mut fields = Vec::new(); - while matches!(self.peek(), Token::Dot) { - self.advance(); - match self.advance() { - Token::Ident(f) => fields.push(f), - tok => { - return err(format!( - "Expected a field name after '.', got {}", - describe_token(&tok) - )); - } - } - } + let fields = self.parse_field_path()?; self.expect(Token::Eq)?; let value = self.parse_expression()?; return Ok(StmtKind::FieldAssign { @@ -1358,7 +1832,14 @@ impl Parser { indices: None, fields: Vec::new(), }, - Expr::ArrayAccess { name, indices } => LValue { + // A subscripted target arrives as FnCall now that the parser no + // longer guesses which one it is; sema turns the surviving calls + // into array accesses, but MID$ needs the LValue here. + Expr::ArrayAccess { name, indices } + | Expr::FnCall { + name, + args: indices, + } => LValue { name, indices: Some(indices), fields: Vec::new(), @@ -1374,18 +1855,33 @@ impl Parser { } fn parse_if(&mut self) -> PResult { + let if_line = self.cur_line(); self.advance(); // consume IF let condition = self.parse_expression()?; self.expect(Token::Then)?; + // A run of colons after THEN separates no statements, so the line ends + // there and this is a block IF. Treating them as the start of a + // statement -- as any non-newline token used to be -- made + // `IF X = 1 THEN :` a one-line IF whose END IF was then unmatched. + let mut ahead = 0; + while matches!(self.peek_at(ahead), Token::Colon) { + ahead += 1; + } + if matches!(self.peek_at(ahead), Token::Newline | Token::Eof) { + for _ in 0..ahead { + self.advance(); + } + } + // Check for single-line IF if !matches!(self.peek(), Token::Newline | Token::Eof) { // Single-line IF - let then_branch = vec![self.parse_statement()?]; + let then_branch = self.parse_single_line_branch()?; let else_branch = if matches!(self.peek(), Token::Else) { self.advance(); - Some(vec![self.parse_statement()?]) + Some(self.parse_single_line_branch()?) } else { None }; @@ -1399,7 +1895,7 @@ impl Parser { // Block IF - parse body, handling ELSEIF as nested IF self.skip_newlines(); - let (then_branch, else_branch) = self.parse_if_body()?; + let (then_branch, else_branch) = self.parse_if_body(if_line)?; Ok(StmtKind::If { condition, @@ -1408,59 +1904,95 @@ impl Parser { }) } - /// Parse the body of an IF block, returning (then_branch, else_branch) - /// Handles ELSEIF by constructing nested IF statements in else_branch - fn parse_if_body(&mut self) -> PResult<(Vec, Option>)> { - let mut body = Vec::new(); + /// Parse one branch of a single-line IF: a colon-separated statement list. + /// + /// Everything after `THEN` up to `ELSE` or the end of the line is the THEN + /// clause, and everything after `ELSE` is the ELSE clause. Taking only the + /// first statement -- as this used to -- let the rest of the line escape + /// the conditional and run unconditionally, so `IF X = 1 THEN PRINT "A" : + /// PRINT "B"` printed `B` when X was 0. It also broke the ELSE form + /// outright: the second statement became a sibling of the IF, leaving the + /// `ELSE` to reach the top level as "ELSE without matching IF". + /// + /// The check for a terminator after the separator is what keeps a trailing + /// colon from pulling in the next line: `parse_statement` skips a leading + /// newline, so without it `IF C THEN PRINT "x" :` would swallow the + /// statement below it. + fn parse_single_line_branch(&mut self) -> PResult> { + let mut body = vec![self.parse_branch_statement()?]; + while matches!(self.peek(), Token::Colon) { + self.advance(); + if matches!(self.peek(), Token::Else | Token::Newline | Token::Eof) { + break; + } + body.push(self.parse_branch_statement()?); + } + Ok(body) + } - loop { - // Captured before parsing so an ELSEIF's synthesized nested IF can - // be attributed to the ELSEIF line rather than to END IF. - let stmt_line = self.cur_line(); - match self.parse_statement() { - Ok(stmt) => { - body.push(stmt); - } - Err(ParseError::Block(BlockEnd::EndIf)) => { - return Ok((body, None)); - } - Err(ParseError::Block(BlockEnd::Else)) => { - // Parse ELSE body until END IF - self.skip_newlines(); - let mut else_body = Vec::new(); - loop { - match self.parse_statement() { - Ok(stmt) => else_body.push(stmt), - Err(ParseError::Block(BlockEnd::EndIf)) => break, - Err(e) => return Err(e), - } - self.skip_newlines(); - } - return Ok((body, Some(else_body))); - } - Err(ParseError::Block(BlockEnd::ElseIf(elseif_condition))) => { - // Recursively parse the rest as a nested IF - self.skip_newlines(); - let (nested_then, nested_else) = self.parse_if_body()?; - - let nested_if = Stmt { - line: stmt_line, - kind: StmtKind::If { - condition: elseif_condition, - then_branch: nested_then, - else_branch: nested_else, - }, - }; - - return Ok((body, Some(vec![nested_if]))); - } - Err(e) => return Err(e), + /// One statement of a single-line IF branch, where a bare line number is + /// an implied GOTO. + /// + /// `IF X < 0 THEN 900` is how GW-BASIC spells its commonest branch, and it + /// is unambiguous: no other statement may begin with a number, so this used + /// to be rejected as "Unexpected token: Integer(900)". + fn parse_branch_statement(&mut self) -> PResult { + let line = self.cur_line(); + if let Token::Integer(_) | Token::LineNumber(_) = self.peek() { + let target = self.parse_goto_target()?; + return Ok(Stmt { + line, + kind: StmtKind::Goto(target), + }); + } + self.parse_inner_statement() + } + + /// Parse the body of an IF block, returning (then_branch, else_branch). + /// Handles ELSEIF by constructing nested IF statements in else_branch. + /// + /// `if_line` is the line of the IF or ELSEIF that opened this body, used to + /// blame the right line when END IF never arrives. + fn parse_if_body(&mut self, if_line: u32) -> PResult<(Vec, Option>)> { + let (body, end) = self.parse_block_body("IF", "END IF", if_line)?; + // Captured before the newline is skipped, so an ELSEIF's synthesized + // nested IF is attributed to the ELSEIF's own line: a terminator is + // followed by the newline that ends the line it was written on. + let elseif_line = self.cur_line(); + + match end { + BlockEnd::EndIf => Ok((body, None)), + BlockEnd::Else => { + self.skip_newlines(); + let else_body = self.parse_block(BlockEnd::EndIf, "IF", if_line)?; + Ok((body, Some(else_body))) } - self.skip_newlines(); + BlockEnd::ElseIf(condition) => { + // The rest of the chain is an IF nested in this one's ELSE. + self.skip_newlines(); + let (nested_then, nested_else) = self.parse_if_body(if_line)?; + let nested_if = Stmt { + line: elseif_line, + kind: StmtKind::If { + condition, + then_branch: nested_then, + else_branch: nested_else, + }, + }; + Ok((body, Some(vec![nested_if]))) + } + other => Err(ParseError::ErrorAt( + if_line, + format!( + "IF needs END IF to close it, but {} came first", + other.keyword() + ), + )), } } fn parse_for(&mut self) -> PResult { + let for_line = self.cur_line(); self.advance(); // consume FOR let var = if let Token::Ident(n) = self.advance() { n @@ -1482,14 +2014,35 @@ impl Parser { self.skip_newlines(); - let mut body = Vec::new(); - loop { - match self.parse_statement() { - Ok(stmt) => body.push(stmt), - Err(ParseError::Block(BlockEnd::Next)) => break, - Err(e) => return Err(e), + // Inspect the terminator rather than letting `parse_block` check only + // its variant, so the control variable can be matched against this + // loop's. `parse_do_loop` and `parse_if_body` take the same route. + let (body, terminator) = self.parse_block_body("FOR", "NEXT", for_line)?; + let BlockEnd::Next(mut names) = terminator else { + return Err(ParseError::ErrorAt( + for_line, + format!( + "FOR needs NEXT to close it, but {} came first", + terminator.keyword() + ), + )); + }; + + // A bare NEXT closes the innermost loop, whichever it is. A named one + // must name *this* loop: `FOR I ... NEXT J` used to compile, and with + // two loops open it silently produced a nesting nobody wrote. + if !names.is_empty() { + let closes = names.remove(0); + if closes != var { + return Err(ParseError::ErrorAt( + for_line, + format!("FOR {} is closed by NEXT {}", var, closes), + )); + } + // The rest belong to the loops enclosing this one. + for name in names.into_iter().rev() { + self.pending_next.push_front(name); } - self.skip_newlines(); } Ok(StmtKind::For { @@ -1502,24 +2055,18 @@ impl Parser { } fn parse_while(&mut self) -> PResult { + let while_line = self.cur_line(); self.advance(); // consume WHILE let condition = self.parse_expression()?; self.skip_newlines(); - let mut body = Vec::new(); - loop { - match self.parse_statement() { - Ok(stmt) => body.push(stmt), - Err(ParseError::Block(BlockEnd::Wend)) => break, - Err(e) => return Err(e), - } - self.skip_newlines(); - } + let body = self.parse_block(BlockEnd::Wend, "WHILE", while_line)?; Ok(StmtKind::While { condition, body }) } fn parse_do_loop(&mut self) -> PResult { + let do_line = self.cur_line(); self.advance(); // consume DO // Check for DO WHILE/UNTIL at start @@ -1537,34 +2084,35 @@ impl Parser { self.skip_newlines(); - let mut body = Vec::new(); - let mut end_condition: Option = None; - let mut end_is_until = false; - - loop { - match self.parse_statement() { - Ok(stmt) => body.push(stmt), - Err(ParseError::Block(BlockEnd::Loop)) => break, - Err(ParseError::Block(BlockEnd::LoopWhile(cond))) => { - end_condition = Some(cond); - end_is_until = false; - break; - } - Err(ParseError::Block(BlockEnd::LoopUntil(cond))) => { - end_condition = Some(cond); - end_is_until = true; - break; - } - Err(e) => return Err(e), + let (body, end) = self.parse_block_body("DO", "LOOP", do_line)?; + let (end_condition, end_is_until) = match end { + BlockEnd::Loop => (None, false), + BlockEnd::LoopWhile(cond) => (Some(cond), false), + BlockEnd::LoopUntil(cond) => (Some(cond), true), + other => { + return Err(ParseError::ErrorAt( + do_line, + format!( + "DO needs LOOP to close it, but {} came first", + other.keyword() + ), + )); } - self.skip_newlines(); - } + }; - // Use end condition if no start condition, or start condition takes precedence - let final_condition = condition.or(end_condition); + // A loop tests at one end or the other. These used to be merged with + // `condition.or(end_condition)`, which silently discarded the one on + // the LOOP: `DO WHILE I < 3 ... LOOP UNTIL I > 100` ran on the WHILE + // alone, with the UNTIL having no effect at all. Writing both is a + // mistake about which test applies, so say so rather than pick one. + if condition.is_some() && end_condition.is_some() { + return err( + "a DO loop may test its condition at only one end, not on both DO and LOOP", + ); + } Ok(StmtKind::DoLoop { - condition: final_condition, + condition: condition.or(end_condition), cond_at_start, is_until: if cond_at_start { is_until @@ -1576,6 +2124,7 @@ impl Parser { } fn parse_select_case(&mut self) -> PResult { + let select_line = self.cur_line(); self.advance(); // consume SELECT self.expect(Token::Case)?; let expr = self.parse_expression()?; @@ -1585,6 +2134,17 @@ impl Parser { // Parse CASE blocks until END SELECT loop { + // The body loop below stops at end of file rather than spinning, so + // this is where an unterminated SELECT CASE is caught. It used to + // fall through to `expect(Token::Case)` and report "Expected Case, + // got Eof" against the last line of the file. + if matches!(self.peek(), Token::Eof) { + return Err(ParseError::ErrorAt( + select_line, + "SELECT CASE is missing its END SELECT".to_string(), + )); + } + // Check for END SELECT if self.at_end_select() { // Consume END SELECT @@ -1619,7 +2179,7 @@ impl Parser { break; } - body.push(self.parse_statement()?); + body.push(self.parse_inner_statement()?); self.skip_newlines(); } @@ -1691,7 +2251,10 @@ impl Parser { Token::Integer(n) => Ok(GotoTarget::Line(n as u32)), Token::LineNumber(n) => Ok(GotoTarget::Line(n)), Token::Ident(name) => Ok(GotoTarget::Label(name)), - tok => err(format!("Expected line number or label, got {:?}", tok)), + tok => err(format!( + "expected a line number or label, got {}", + describe_token(&tok) + )), } } @@ -1754,9 +2317,6 @@ impl Parser { self.advance(); let dims = self.parse_expr_list()?; self.expect(Token::RParen)?; - // Track the name so that `name(i)` parses as an array access - // rather than a function call. - self.declared_arrays.insert(name.to_uppercase()); Some(dims) } else { None @@ -1879,6 +2439,7 @@ impl Parser { } fn parse_sub(&mut self) -> PResult { + let sub_line = self.cur_line(); self.advance(); // consume SUB let name = if let Token::Ident(n) = self.advance() { n @@ -1905,21 +2466,14 @@ impl Parser { self.skip_newlines(); - let mut body = Vec::new(); - loop { - match self.parse_statement() { - Ok(stmt) => body.push(stmt), - Err(ParseError::Block(BlockEnd::EndSub)) => break, - Err(e) => return Err(e), - } - self.skip_newlines(); - } + let body = self.parse_block(BlockEnd::EndSub, &format!("SUB '{}'", name), sub_line)?; Ok(StmtKind::Sub { name, params, body }) } /// `FUNCTION name(params)` ... `END FUNCTION` fn parse_function(&mut self) -> PResult { + let fn_line = self.cur_line(); self.advance(); // consume FUNCTION let name = if let Token::Ident(n) = self.advance() { n @@ -1954,15 +2508,11 @@ impl Parser { self.skip_newlines(); - let mut body = Vec::new(); - loop { - match self.parse_statement() { - Ok(stmt) => body.push(stmt), - Err(ParseError::Block(BlockEnd::EndFunction)) => break, - Err(e) => return Err(e), - } - self.skip_newlines(); - } + let body = self.parse_block( + BlockEnd::EndFunction, + &format!("FUNCTION '{}'", name), + fn_line, + )?; Ok(StmtKind::Function { name, @@ -1993,42 +2543,23 @@ impl Parser { Ok(params) } - fn parse_data(&mut self) -> PResult { - self.advance(); // consume DATA - let mut values = Vec::new(); - - loop { - match self.peek().clone() { - Token::Integer(n) => { - self.advance(); - values.push(Literal::Integer(n)); - } - Token::Float(f) => { - self.advance(); - values.push(Literal::Float(f)); - } - Token::String(s) => { - self.advance(); - values.push(Literal::String(s)); - } - Token::Minus => { - self.advance(); - match self.advance() { - Token::Integer(n) => values.push(Literal::Integer(-n)), - Token::Float(f) => values.push(Literal::Float(-f)), - _ => return err("Expected number after minus in DATA"), - } - } - _ => break, - } - if matches!(self.peek(), Token::Comma) { - self.advance(); - } else { - break; - } - } - - Ok(StmtKind::Data(values)) + /// `DATA item, item, ...`, where the items arrived as raw source text. + /// + /// The lexer hands over the whole operand verbatim (see + /// `Lexer::read_data_text`), because a DATA item is not an expression: it is + /// a literal run of characters, and tokenizing it would uppercase words and + /// renormalize numbers. This used to accept only Integer, Float, String and + /// a leading minus, so `DATA hello, world` ended the list at `hello` -- and + /// silently, since the loop simply broke, leaving the word to be parsed as a + /// fresh statement and produce an error about the word rather than the DATA. + fn parse_data(&mut self, text: &str) -> PResult { + self.advance(); // consume the DATA token + Ok(StmtKind::Data( + split_data_items(text) + .iter() + .map(|it| it.literal()) + .collect(), + )) } fn parse_read(&mut self) -> PResult { @@ -2096,9 +2627,10 @@ impl Parser { FileMode::Random } tok => { + let tok = tok.clone(); return err(format!( - "Expected INPUT, OUTPUT, APPEND or RANDOM, got {:?}", - tok + "expected INPUT, OUTPUT, APPEND or RANDOM, got {}", + describe_token(&tok) )); } }; @@ -2154,6 +2686,30 @@ impl Parser { Ok(StmtKind::Field { file_num, fields }) } + /// `CALL Name(args)` or `CALL Name` -- the explicit form of a procedure + /// call, which GW-BASIC and QuickBASIC both accept. + /// + /// Recognised only in statement position before a name, like the + /// random-access statement names, so a program may still use CALL for a + /// variable of its own. Without this the word parsed as a paren-less call + /// to a subroutine named CALL, and the diagnostic complained about the + /// callee rather than about CALL. + fn parse_call(&mut self) -> PResult { + self.advance(); // consume CALL + let Token::Ident(name) = self.advance() else { + return err("Expected a procedure name after CALL"); + }; + let args = if matches!(self.peek(), Token::LParen) { + self.advance(); + let args = self.parse_expr_list()?; + self.expect(Token::RParen)?; + args + } else { + Vec::new() + }; + Ok(StmtKind::Call { name, args }) + } + /// `LSET v$ = expr` / `RSET v$ = expr` fn parse_set_field(&mut self, right: bool) -> PResult { self.advance(); // consume LSET or RSET @@ -2234,6 +2790,29 @@ impl Parser { /// Precedence-climbing parser for binary expressions /// min_prec: minimum precedence level to parse at this level + /// Consume a `.field.sub` chain, returning the names. + /// + /// The statement-level twin of [`Self::parse_field_chain`], which builds + /// `Expr::Field` nodes instead. Assignment targets keep their path as a + /// plain list of names inside an `LValue`, and three statement parsers + /// spelled this loop out identically before it was hoisted here. + fn parse_field_path(&mut self) -> PResult> { + let mut fields = Vec::new(); + while matches!(self.peek(), Token::Dot) { + self.advance(); + match self.advance() { + Token::Ident(f) => fields.push(f), + tok => { + return err(format!( + "Expected a field name after '.', got {}", + describe_token(&tok) + )); + } + } + } + Ok(fields) + } + /// Consume any `.field` chain following an expression. fn parse_field_chain(&mut self, mut base: Expr) -> PResult { while matches!(self.peek(), Token::Dot) { @@ -2256,11 +2835,34 @@ impl Parser { Ok(base) } + /// Every descent into an expression passes through here, so this is the one + /// place the depth has to be counted: parentheses re-enter via + /// `parse_primary`, and unary `-`, `+` and `NOT` re-enter directly. fn parse_prec(&mut self, min_prec: u8) -> PResult { - // Handle NOT prefix operator (binds tighter than binary ops) + self.depth += 1; + let r = self.parse_prec_inner(min_prec); + self.depth -= 1; + r + } + + fn parse_prec_inner(&mut self, min_prec: u8) -> PResult { + if self.depth > MAX_DEPTH { + return err(format!("nesting is too deep (limit {} levels)", MAX_DEPTH)); + } + + // `NOT` is a prefix operator sitting between the comparisons and `AND`, + // so its operand is everything that binds at least as tightly as a + // comparison -- and no more. + // + // It used to take its operand at the *caller's* `min_prec`, which at + // statement level is the lowest of all, so `NOT` swallowed whatever + // followed it: `NOT A AND B` parsed as `NOT (A AND B)`. Taking the + // operand at CMP_PREC keeps `NOT A = B` grouping as `NOT (A = B)` -- + // the form that actually matters -- while leaving `AND` to the loop + // below, which is where it belongs. let mut left = if matches!(self.peek(), Token::Not) { self.advance(); - let operand = self.parse_prec(min_prec)?; // NOT is right-associative + let operand = self.parse_prec(CMP_PREC)?; Expr::Unary { op: UnaryOp::Not, operand: Box::new(operand), @@ -2275,8 +2877,10 @@ impl Parser { break; } self.advance(); - // Power is right-associative; others are left-associative - let next_min = if op == BinaryOp::Pow { prec } else { prec + 1 }; + // Every operator associates left to right, `^` included: GW-BASIC + // and QuickBASIC evaluate `2 ^ 3 ^ 2` as `(2^3)^2` = 64. This used + // to special-case `Pow` to bind right, giving 512. + let next_min = prec + 1; let right = self.parse_prec(next_min)?; left = Expr::Binary { op, @@ -2330,16 +2934,14 @@ impl Parser { let args = self.parse_expr_list()?; self.expect(Token::RParen)?; - // Distinguish array access from function call based on DIM declarations - let base = if self.declared_arrays.contains(&name.to_uppercase()) { - Expr::ArrayAccess { - name, - indices: args, - } - } else { - Expr::FnCall { name, args } - }; - self.parse_field_chain(base) + // `A(1)` is an array element or a call; the parser cannot + // tell, and used to guess from the DIM statements it had + // read so far. That made the AST depend on where the DIM + // was written -- the same source became FnCall before it + // and ArrayAccess after -- and ignored scope entirely, so a + // DIM inside a SUB changed how module-level code parsed. + // Sema resolves it against the finished symbol table. + self.parse_field_chain(Expr::FnCall { name, args }) } else { self.parse_field_chain(Expr::Variable(name)) } @@ -2350,7 +2952,10 @@ impl Parser { self.expect(Token::RParen)?; Ok(expr) } - tok => err(format!("Unexpected token in expression: {:?}", tok)), + tok => err(format!( + "unexpected {} in an expression", + describe_token(&tok) + )), } } @@ -2373,12 +2978,20 @@ mod tests { use super::*; use crate::lexer::Lexer; + /// Parse, joining any errors so these tests keep their `Result<_, String>` + /// shape now that the parser reports every error it finds rather than one. fn parse(input: &str) -> Result { let mut lexer = Lexer::new(input); let tokens = lexer.tokenize()?; let lines = lexer.line_map().to_vec(); let mut parser = Parser::new(tokens, lines); - parser.parse().map_err(|e| e.to_string()) + parser.parse().map_err(|errors| { + errors + .iter() + .map(|e| e.to_string()) + .collect::>() + .join("; ") + }) } // Label Tests @@ -3235,14 +3848,16 @@ mod tests { } #[test] - fn test_expr_power_right_associative() { - // 2 ^ 3 ^ 2 should be 2 ^ (3 ^ 2) = 512, not (2 ^ 3) ^ 2 = 64 + fn test_expr_power_left_associative() { + // 2 ^ 3 ^ 2 is (2 ^ 3) ^ 2 = 64, as GW-BASIC and QuickBASIC evaluate + // it. This asserted the opposite nesting until associativity was made + // uniform: every operator here associates left to right. let prog = parse("X = 2 ^ 3 ^ 2").unwrap(); if let StmtKind::Let { value, .. } = &prog.statements[0].kind { - if let Expr::Binary { op, right, .. } = value { + if let Expr::Binary { op, left, .. } = value { assert_eq!(*op, BinaryOp::Pow); assert!(matches!( - right.as_ref(), + left.as_ref(), Expr::Binary { op: BinaryOp::Pow, .. @@ -3311,19 +3926,43 @@ mod tests { #[test] fn test_expr_logical_operators() { + // OR and XOR share the lowest level and associate left to right, and + // AND binds tighter than both, so this groups as + // ((A AND B) OR C) XOR D and the outermost operator is XOR. + // + // This asserted OR at the top, which held only because XOR used to have + // a level of its own that bound tighter than AND -- contradicting + // LANGREF's table, which puts OR and XOR together. let prog = parse("X = A AND B OR C XOR D").unwrap(); - if let StmtKind::Let { value, .. } = &prog.statements[0].kind { - // OR has lowest precedence, then XOR, then AND - assert!(matches!( - value, + let StmtKind::Let { value, .. } = &prog.statements[0].kind else { + panic!("Expected Let"); + }; + let Expr::Binary { + op: BinaryOp::Xor, + left, + .. + } = value + else { + panic!("Expected XOR at the top, got {:?}", value); + }; + let Expr::Binary { + op: BinaryOp::Or, + left: or_left, + .. + } = left.as_ref() + else { + panic!("Expected OR beneath the XOR, got {:?}", left); + }; + assert!( + matches!( + or_left.as_ref(), Expr::Binary { - op: BinaryOp::Or, + op: BinaryOp::And, .. } - )); - } else { - panic!("Expected Let"); - } + ), + "AND binds tighter than both" + ); } #[test] diff --git a/src/runtime/sysv/data.s b/src/runtime/sysv/data.s index 55f4f67..4b53636 100644 --- a/src/runtime/sysv/data.s +++ b/src/runtime/sysv/data.s @@ -80,8 +80,15 @@ _rt_read_number: # _rt_read_string - Read next DATA value as a string # Reads the next value from the DATA table and returns it as a string. -# Currently only handles string-typed DATA entries; numeric entries -# should not be READ into string variables (BASIC would convert, we don't). +# Type conversion is performed automatically, mirroring _rt_read_number: +# - String (type 2): return the literal +# - Integer (type 0) / Float (type 1): render with _rt_str, as PRINT would +# +# The numeric cases used to be absent: the entry's second word was passed to +# strlen whatever its type, so `DATA 42` followed by `READ A$` called strlen on +# address 42 and the program died with SIGSEGV. GW-BASIC converts, and so does +# _rt_read_number in the other direction (it strtod's a string entry), so the +# table's tag never has to match the variable the program reads it into. # # Arguments: none # @@ -99,7 +106,11 @@ _rt_read_string: shl rax, 4 # offset = index * 16 lea rcx, [rip + _data_table] add rcx, rax # rcx = entry address - # Load string pointer (assumes type is string) + # Load type tag + mov rax, QWORD PTR [rcx] # rax = type (0=int, 1=float, 2=string) + cmp rax, 2 + jne .Lread_num_as_str + # Load string pointer mov rax, QWORD PTR [rcx + 8] # rax = string pointer # Calculate length using strlen (DATA strings are null-terminated) mov rdi, rax # string pointer for strlen @@ -115,6 +126,17 @@ _rt_read_string: inc QWORD PTR [rip + _data_ptr] leave ret +.Lread_num_as_str: + # Numeric entry: widen to double and render it the way PRINT would. + movsd xmm0, QWORD PTR [rcx + 8] # float bits, if type 1 + cmp rax, 0 + jne .Lread_num_as_str_go + mov rax, QWORD PTR [rcx + 8] # integer: load and convert + cvtsi2sd xmm0, rax +.Lread_num_as_str_go: + inc QWORD PTR [rip + _data_ptr] # advance before the tail call + leave + jmp _rt_str # returns (rax, rdx) already # _rt_restore - Reset DATA pointer (RESTORE statement) # Resets the DATA read position, allowing DATA to be re-read from the beginning diff --git a/src/runtime/sysv/error.s b/src/runtime/sysv/error.s index 25ed06f..884338d 100644 --- a/src/runtime/sysv/error.s +++ b/src/runtime/sysv/error.s @@ -29,6 +29,9 @@ _err_badfile: .asciz "Bad file number" _err_badmode: .asciz "Bad file mode" _err_fieldovf: .asciz "FIELD overflow" _err_permission: .asciz "Permission denied" +_err_notfound: .asciz "File not found" +_err_alreadyopen: .asciz "File already open" +_err_pastend: .asciz "Input past end of file" .text diff --git a/src/runtime/sysv/file.s b/src/runtime/sysv/file.s index 9edc224..5380224 100644 --- a/src/runtime/sysv/file.s +++ b/src/runtime/sysv/file.s @@ -100,6 +100,14 @@ _rt_file_open: mov r14d, edx # mode (0/1/2) mov ebx, ecx # file number + # A number that is still open must be CLOSEd first. Rebinding it used to + # succeed silently, leaking the old FILE* and losing whatever was buffered + # in it. + lea rax, [rip + _file_handles] + mov rax, [rax + rbx*8] + test rax, rax + jnz .Lopen_already + # Copy filename to buffer and null-terminate # memcpy(_file_name_buf, filename_ptr, filename_len) lea rdi, [rip + _file_name_buf] @@ -129,6 +137,12 @@ _rt_file_open: lea rdi, [rip + _file_name_buf] call fopen # returns FILE* in rax (or NULL on error) + # A failed open used to be stored anyway, so the program carried on with a + # NULL handle: OPEN of a missing file "succeeded", EOF() answered -1, and + # every later use was a crash or silence. + test rax, rax + jz .Lopen_failed + # Store FILE* in handle table: _file_handles[file_number] = rax lea rcx, [rip + _file_handles] mov [rcx + rbx*8], rax @@ -212,6 +226,8 @@ _rt_file_print_string: # Get FILE* from handle table lea rax, [rip + _file_handles] mov rdi, [rax + rbx*8] # FILE* → 1st arg + test rdi, rdi + jz .Lfile_not_open # a number nobody OPENed is NULL here # fprintf(file, "%.*s", len, ptr) lea rsi, [rip + _file_fmt_str] # format → 2nd arg @@ -398,6 +414,8 @@ _rt_file_print_char: lea rax, [rip + _file_handles] mov rdi, [rax + rbx*8] # FILE* + test rdi, rdi + jz .Lfile_not_open # a number nobody OPENed is NULL here lea rsi, [rip + _file_fmt_char] mov rdx, r12 # char xor eax, eax @@ -430,6 +448,8 @@ _rt_file_print_newline: # Use fputc('\n', file) - simpler than fprintf lea rax, [rip + _file_handles] mov rsi, [rax + rbx*8] # FILE* → rsi (2nd arg) + test rsi, rsi + jz .Lfile_not_open # a number nobody OPENed is NULL here mov edi, 10 # '\n' → edi (1st arg) call fputc @@ -638,11 +658,15 @@ _rt_file_line_input: mov rsi, 1023 # max chars (leave room for null) lea rax, [rip + _file_handles] mov rdx, [rax + rbx*8] # FILE* + test rdx, rdx + jz .Lfile_not_open # a number nobody OPENed is NULL here call fgets - # Check for EOF/error (fgets returns NULL) + # A read past the end used to answer with an empty string, so a loop + # missing its EOF() test ran on quietly forever instead of saying what was + # wrong. GW-BASIC calls this "Input past end of file". test rax, rax - jz .Lfile_input_string_empty + jz .Lfile_past_end # Calculate length using strlen lea rdi, [rip + _file_input_buf] @@ -1546,3 +1570,32 @@ _rt_file_lof: pop rbx leave ret + +# Shared tail for a file number that was never OPENed. +# +# PRINT # and LINE INPUT # used to load the NULL handle and hand it straight to +# fprintf/fgets, so `PRINT #3, "x"` on an unopened number died with SIGSEGV -- +# no message, exit 139. _rt_file_getc and _rt_file_eof already guarded; these +# did not. The line number is unknown here (these helpers are not given one), +# and _rt_error prints a bare message for 0. +.Lfile_not_open: + lea rdi, [rip + _err_badfile] + xor esi, esi + call _rt_error + +# OPEN failed. The line number is not passed to _rt_file_open, so these report +# without one; _rt_error prints a bare message for 0. +.Lopen_failed: + lea rdi, [rip + _err_notfound] + xor esi, esi + call _rt_error + +.Lopen_already: + lea rdi, [rip + _err_alreadyopen] + xor esi, esi + call _rt_error + +.Lfile_past_end: + lea rdi, [rip + _err_pastend] + xor esi, esi + call _rt_error diff --git a/src/runtime/sysv/string.s b/src/runtime/sysv/string.s index 2af0bf7..a00a220 100644 --- a/src/runtime/sysv/string.s +++ b/src/runtime/sysv/string.s @@ -240,8 +240,17 @@ _rt_instr: mov r14, rdx # needle ptr mov r15, rcx # needle len mov rbx, r8 # start position (1-based) + # A start below 1 would move the search pointer backwards out of the + # buffer; GW-BASIC calls that an illegal argument. + cmp rbx, 1 + jl .Linstr_badstart # Adjust for start position dec rbx # convert to 0-based + # A start past the end finds nothing. Without this the `sub` below drove + # the remaining length negative -- enormous, unsigned -- so the "room for + # the needle" test passed and memcmp read past the end of the string. + cmp rbx, r13 + jae .Linstr_not_found add r12, rbx # advance haystack ptr sub r13, rbx # reduce remaining length # Special case: empty needle matches at current position @@ -274,6 +283,11 @@ _rt_instr: jmp .Linstr_done .Linstr_not_found: xor rax, rax # return 0 + jmp .Linstr_done +.Linstr_badstart: + lea rdi, [rip + _err_domain] + xor esi, esi + call _rt_error .Linstr_done: # Restore stack and callee-saved registers add rsp, 8 # Restore stack alignment @@ -455,6 +469,68 @@ _rt_space: leave ret +# _rt_fixed - Fit a string to a declared width, for STRING * n +# +# A fixed-length string always holds exactly its declared number of characters: +# a shorter value is padded with spaces on the right, a longer one is truncated. +# Assignment used to store the source verbatim, so `STRING * 5` held whatever it +# was given and the declared width meant nothing. +# +# A fresh buffer rather than an interior pointer, because the result is stored +# and the source may be a temporary. +# +# Arguments: rdi = source pointer, rsi = source length, rdx = width +# Returns: rax = pointer, rdx = width +.globl _rt_fixed +_rt_fixed: + push rbp + mov rbp, rsp + push rbx + push r12 + push r13 + push r14 + + mov r12, rdi # source pointer + mov r13, rsi # source length + mov rbx, rdx # width + cmp rbx, 0 + jge .Lfixed_ok + xor rbx, rbx # a non-positive width yields the empty string +.Lfixed_ok: + # Copy at most `width` bytes. + mov r14, r13 + cmp r14, rbx + jbe .Lfixed_have_n + mov r14, rbx +.Lfixed_have_n: + + lea rdi, [rbx + 1] # room for the NUL the string helpers expect + call malloc + + # Space-fill the whole width first, then overwrite the prefix; memset + # returns its destination, so the pointer survives without a save. + mov rdi, rax + mov esi, ' ' + mov rdx, rbx + call memset + + # memcpy(dest, src, min(len, width)). rax still holds the buffer. + mov rdi, rax + mov rsi, r12 + mov rdx, r14 + mov r13, rax # keep the buffer across the call; a push here + call memcpy # would leave rsp misaligned + mov rax, r13 + + mov rdx, rbx # the result is always `width` long + + pop r14 + pop r13 + pop r12 + pop rbx + leave + ret + # _rt_string_n - STRING$(n, ch): a string of n copies of one character # Arguments: rdi = count, rsi = character code # Returns: rax = pointer, rdx = length diff --git a/src/runtime/win64-native/data.s b/src/runtime/win64-native/data.s index 1a2cd76..536c92f 100644 --- a/src/runtime/win64-native/data.s +++ b/src/runtime/win64-native/data.s @@ -73,6 +73,15 @@ _rt_read_number: # _rt_read_string - Read next DATA value as a string # Reads the next value from the DATA table and returns it as a string. +# Type conversion is performed automatically, mirroring _rt_read_number: +# - String (type 2): return the literal +# - Integer (type 0) / Float (type 1): render with _rt_str, as PRINT would +# +# The numeric cases used to be absent: the entry's second word was passed to +# lstrlenA whatever its type, so `DATA 42` followed by `READ A$` measured a +# string at address 42. GW-BASIC converts, and _rt_read_number already converts +# in the other direction, so the table's tag need not match the variable the +# program reads it into. # # Arguments: none # @@ -92,6 +101,11 @@ _rt_read_string: lea rcx, [rip + _data_table] add rcx, rax # rcx = entry address + # Load type tag + mov rax, QWORD PTR [rcx] # rax = type (0=int, 1=float, 2=string) + cmp rax, 2 + jne .Lread_num_as_str + # Load string pointer and save for later mov rdi, QWORD PTR [rcx + 8] # rdi = string pointer @@ -110,6 +124,20 @@ _rt_read_string: leave ret +.Lread_num_as_str: + # Numeric entry: widen to double and render it the way PRINT would. + movsd xmm0, QWORD PTR [rcx + 8] # float bits, if type 1 + cmp rax, 0 + jne .Lread_num_as_str_go + mov rax, QWORD PTR [rcx + 8] # integer: load and convert + cvtsi2sd xmm0, rax +.Lread_num_as_str_go: + inc QWORD PTR [rip + _data_ptr] # advance before the tail call + add rsp, 40 + pop rdi + leave + jmp _rt_str # returns (rax, rdx) already + # _rt_restore - Reset DATA pointer (RESTORE statement) # Resets the DATA read position. # diff --git a/src/runtime/win64-native/error.s b/src/runtime/win64-native/error.s index f34d5a1..c47ce5b 100644 --- a/src/runtime/win64-native/error.s +++ b/src/runtime/win64-native/error.s @@ -31,6 +31,9 @@ _err_badfile: .asciz "Bad file number" _err_badmode: .asciz "Bad file mode" _err_fieldovf: .asciz "FIELD overflow" _err_permission: .asciz "Permission denied" +_err_notfound: .asciz "File not found" +_err_alreadyopen: .asciz "File already open" +_err_pastend: .asciz "Input past end of file" # Zero-filled scratch, so .bss rather than .data -- see data_defs.s. The # .text below restores the section for the code that follows. diff --git a/src/runtime/win64-native/file.s b/src/runtime/win64-native/file.s index 93ed76f..cfd4cf6 100644 --- a/src/runtime/win64-native/file.s +++ b/src/runtime/win64-native/file.s @@ -95,6 +95,13 @@ _rt_file_open: mov r14d, r8d # mode (0/1/2) mov ebx, r9d # file number + # A number that is still open must be CLOSEd first. Rebinding it used to + # succeed silently, leaking the old handle. + lea rax, [rip + _file_handles] + mov rax, [rax + rbx*8] + test rax, rax + jnz .Lopen_already + # Copy filename and null-terminate lea rcx, [rip + _file_name_buf] mov rdx, rdi # src @@ -136,6 +143,12 @@ _rt_file_open: mov QWORD PTR [rsp + 48], 0 # hTemplateFile = NULL call CreateFileA + # A failed open used to be stored anyway, so the program carried on with + # INVALID_HANDLE_VALUE. Note this is -1 rather than NULL, so the NULL + # guards elsewhere would not have caught it. + cmp rax, INVALID_HANDLE_VALUE + je .Lopen_failed + # Store HANDLE in handle table lea rcx, [rip + _file_handles] mov [rcx + rbx*8], rax @@ -232,6 +245,8 @@ _rt_file_print_string: # Get HANDLE from table lea rax, [rip + _file_handles] mov rcx, [rax + rbx*8] # hFile + test rcx, rcx + jz .Lfile_not_open # a number nobody OPENed is NULL here # WriteFile(hFile, lpBuffer, nNumberOfBytesToWrite, lpNumberOfBytesWritten, lpOverlapped) mov rdx, rdi # lpBuffer = string ptr @@ -429,6 +444,8 @@ _rt_file_print_char: # Get HANDLE lea rax, [rip + _file_handles] mov rcx, [rax + rbx*8] # hFile + test rcx, rcx + jz .Lfile_not_open # a number nobody OPENed is NULL here # WriteFile(hFile, buffer, 1, &bytesWritten, NULL) lea rdx, [rip + _file_output_buf] @@ -462,6 +479,8 @@ _rt_file_print_newline: # Get HANDLE lea rax, [rip + _file_handles] mov rcx, [rax + rbx*8] # hFile + test rcx, rcx + jz .Lfile_not_open # a number nobody OPENed is NULL here # WriteFile(hFile, "\r\n", CRLF_LEN, &bytesWritten, NULL) lea rdx, [rip + _file_newline] @@ -691,6 +710,8 @@ _rt_file_line_input: # ReadFile(hFile, &buffer[pos], 1, &bytesRead, NULL) lea rax, [rip + _file_handles] mov rcx, [rax + rbx*8] # hFile + test rcx, rcx + jz .Lfile_not_open # a number nobody OPENed is NULL here lea rdx, [rip + _file_input_buf] add rdx, r12 # &buffer[pos] mov r8, SINGLE_BYTE @@ -702,7 +723,15 @@ _rt_file_line_input: lea rax, [rip + _file_bytes_read] mov rax, [rax] test rax, rax - jz .Lfile_input_str_done # EOF + jnz .Lfile_input_str_have + # Nothing read. If nothing was read on this call at all, the file was + # already at its end and this read is one too many -- GW-BASIC calls that + # "Input past end of file". A partial last line without a trailing newline + # is not: that has characters in the buffer already. + test r12d, r12d + jz .Lfile_past_end + jmp .Lfile_input_str_done +.Lfile_input_str_have: # Check if it's a newline lea rax, [rip + _file_input_buf] @@ -1606,3 +1635,32 @@ _rt_file_lof: pop rbx leave ret + +# Shared tail for a file number that was never OPENed. +# +# PRINT # and LINE INPUT # used to hand the NULL handle straight to WriteFile +# and ReadFile. The System V build died with SIGSEGV on the same programs; +# _rt_file_getc and _rt_file_eof already guarded and these did not. The line +# number is unknown here (these helpers are not given one), and _rt_error +# prints a bare message for 0. +.Lfile_not_open: + lea rcx, [rip + _err_badfile] + xor edx, edx + call _rt_error + +# OPEN failed. The line number is not passed to _rt_file_open, so these report +# without one; _rt_error prints a bare message for 0. +.Lopen_failed: + lea rcx, [rip + _err_notfound] + xor edx, edx + call _rt_error + +.Lopen_already: + lea rcx, [rip + _err_alreadyopen] + xor edx, edx + call _rt_error + +.Lfile_past_end: + lea rcx, [rip + _err_pastend] + xor edx, edx + call _rt_error diff --git a/src/runtime/win64-native/string.s b/src/runtime/win64-native/string.s index f036e09..4fe2420 100644 --- a/src/runtime/win64-native/string.s +++ b/src/runtime/win64-native/string.s @@ -221,8 +221,18 @@ _rt_instr: mov r15, r9 # needle len mov rbx, rdi # start position (1-based) + # A start below 1 would move the search pointer backwards out of the + # buffer; GW-BASIC calls that an illegal argument. + cmp rbx, 1 + jl .Linstr_badstart + # Adjust for start position dec rbx # convert to 0-based + # A start past the end finds nothing. Without this the `sub` below drove + # the remaining length negative -- enormous, unsigned -- so the "room for + # the needle" test passed and memcmp read past the end of the string. + cmp rbx, r13 + jae .Linstr_not_found add r12, rbx # advance haystack ptr sub r13, rbx # reduce remaining length @@ -259,6 +269,11 @@ _rt_instr: .Linstr_not_found: xor rax, rax + jmp .Linstr_done +.Linstr_badstart: + lea rcx, [rip + _err_domain] + xor edx, edx + call _rt_error .Linstr_done: add rsp, 40 @@ -441,6 +456,68 @@ _rt_space: leave ret +# _rt_fixed - Fit a string to a declared width, for STRING * n +# +# A fixed-length string always holds exactly its declared number of characters: +# a shorter value is padded with spaces on the right, a longer one is truncated. +# Assignment used to store the source verbatim, so `STRING * 5` held whatever it +# was given and the declared width meant nothing. +# +# Arguments: rcx = source pointer, rdx = source length, r8 = width +# Returns: rax = pointer, rdx = width +.globl _rt_fixed +_rt_fixed: + push rbp + mov rbp, rsp + push rbx + push r12 + push r13 + push r14 + sub rsp, 32 # shadow space only: the four pushes above already + # leave rsp aligned, unlike the two in _rt_space + + mov r12, rcx # source pointer + mov r13, rdx # source length + mov rbx, r8 # width + cmp rbx, 0 + jge .Lfixed_ok + xor rbx, rbx # a non-positive width yields the empty string +.Lfixed_ok: + # Copy at most `width` bytes. + mov r14, r13 + cmp r14, rbx + jbe .Lfixed_have_n + mov r14, rbx +.Lfixed_have_n: + + lea rcx, [rbx + 1] # room for the NUL the string helpers expect + call malloc + + # Space-fill the whole width first, then overwrite the prefix; memset + # returns its destination, so the pointer survives without a save. + mov rcx, rax + mov rdx, ' ' + mov r8, rbx + call memset + + # memcpy(dest, src, min(len, width)). rax still holds the buffer. + mov rcx, rax + mov rdx, r12 + mov r8, r14 + mov r13, rax # keep the buffer across the call + call memcpy + mov rax, r13 + + mov rdx, rbx # the result is always `width` long + + add rsp, 32 + pop r14 + pop r13 + pop r12 + pop rbx + leave + ret + # _rt_string_n - STRING$(n, ch): a string of n copies of one character # Arguments: rcx = count, rdx = character code # Returns: rax = pointer, rdx = length diff --git a/src/sema.rs b/src/sema.rs index 1ad4672..c2e295e 100644 --- a/src/sema.rs +++ b/src/sema.rs @@ -24,6 +24,7 @@ // Copyright (c) 2025-2026 Jeff Garzik // SPDX-License-Identifier: MIT +use crate::lexer::normalized; use crate::parser::*; use std::collections::{HashMap, HashSet}; @@ -243,7 +244,7 @@ pub fn unsupported_reason(name: &str) -> Option<&'static str> { /// mention of either is a call. Without this they parsed as ordinary variable /// reads and quietly returned the zero of a slot nobody ever wrote. pub fn is_zero_arg_builtin(name: &str) -> bool { - builtin(&name.to_uppercase()).is_some_and(|(_, min, _)| *min == 0) + builtin(normalized(name)).is_some_and(|(_, min, _)| *min == 0) } fn builtin(name: &str) -> Option<&'static (&'static str, usize, usize)> { @@ -338,7 +339,7 @@ impl Symbols { TypeRef::FixedString(_) => 2, TypeRef::Record(name) => self .records - .get(&name.to_uppercase()) + .get(normalized(name)) .map(|r| r.words) .unwrap_or(1), } @@ -385,14 +386,190 @@ pub struct Diagnostic { pub note: Option, } -/// Analyze a program: collect its symbols and report any problems. -pub fn analyze(program: &Program) -> (Symbols, Vec) { +/// Analyze a program: collect its symbols, resolve `A(1)`, and report problems. +/// +/// The program is taken by `&mut` because of the middle step. `A(1)` is either +/// an array element or a call, and only the finished symbol table can say +/// which. The parser used to guess from the DIM statements it had read so far, +/// which meant the same source produced a different AST depending on whether +/// its DIM came earlier or later in the file, and ignored scope entirely. +pub fn analyze(program: &mut Program) -> (Symbols, Vec) { let mut a = Analyzer::default(); a.collect(&program.statements, &Scope::Module); + a.check_name_collisions(); + a.check_return_has_a_gosub(&program.statements); + a.resolve_array_accesses(&mut program.statements, &Scope::Module); a.check(&program.statements, &Scope::Module); (a.symbols, a.diagnostics) } +/// Call `f` for every statement in `stmts`, nested bodies included. +fn walk_stmts(stmts: &[Stmt], f: &mut impl FnMut(&Stmt)) { + for stmt in stmts { + f(stmt); + for body in child_bodies(stmt) { + walk_stmts(body, f); + } + } +} + +/// Apply `f` to every expression directly held by `stmt`, in place. +/// +/// Deliberately exhaustive, with no wildcard arm: this is the single place that +/// knows where a statement keeps its expressions, so a new `StmtKind` that +/// carries one must fail to compile here rather than be silently skipped. +/// Nested statement bodies are *not* visited -- the caller walks those itself, +/// because it has to change scope on the way in. +fn for_each_expr_mut(stmt: &mut Stmt, f: &mut impl FnMut(&mut Expr)) { + /// Every expression an assignment target can hold: its subscripts. + fn lvalue(lv: &mut LValue, f: &mut impl FnMut(&mut Expr)) { + if let Some(indices) = &mut lv.indices { + indices.iter_mut().for_each(&mut *f); + } + } + + match &mut stmt.kind { + StmtKind::Let { indices, value, .. } => { + if let Some(indices) = indices { + indices.iter_mut().for_each(&mut *f); + } + f(value); + } + StmtKind::Print { + file_num, + items, + using, + .. + } => { + file_num.iter_mut().for_each(&mut *f); + using.iter_mut().for_each(&mut *f); + for item in items { + if let PrintItem::Expr(e) = item { + f(e); + } + } + } + StmtKind::Input { vars, file_num, .. } => { + file_num.iter_mut().for_each(&mut *f); + vars.iter_mut().for_each(|v| lvalue(v, f)); + } + StmtKind::LineInput { var, file_num, .. } => { + file_num.iter_mut().for_each(&mut *f); + lvalue(var, f); + } + StmtKind::If { condition, .. } => f(condition), + StmtKind::For { + start, end, step, .. + } => { + f(start); + f(end); + step.iter_mut().for_each(&mut *f); + } + StmtKind::While { condition, .. } => f(condition), + StmtKind::DoLoop { condition, .. } => condition.iter_mut().for_each(&mut *f), + StmtKind::OnGoto { expr, .. } | StmtKind::OnGosub { expr, .. } => f(expr), + StmtKind::Dim { decls } | StmtKind::Redim { decls, .. } => { + for d in decls { + if let Some(dims) = &mut d.dimensions { + dims.iter_mut().for_each(&mut *f); + } + } + } + StmtKind::Call { args, .. } => args.iter_mut().for_each(&mut *f), + StmtKind::MidAssign { + target, + start, + len, + value, + } => { + lvalue(target, f); + f(start); + len.iter_mut().for_each(&mut *f); + f(value); + } + StmtKind::Swap(a, b) => { + lvalue(a, f); + lvalue(b, f); + } + StmtKind::Const { value, .. } => f(value), + StmtKind::FieldAssign { target, value } => { + lvalue(target, f); + f(value); + } + StmtKind::Read(vars) => vars.iter_mut().for_each(|v| lvalue(v, f)), + StmtKind::SelectCase { expr, cases } => { + f(expr); + for (clauses, _) in cases { + for clause in clauses.iter_mut().flatten() { + match clause { + CaseClause::Value(e) | CaseClause::Compare(_, e) => f(e), + CaseClause::Range(lo, hi) => { + f(lo); + f(hi); + } + } + } + } + } + StmtKind::Open { + filename, + file_num, + reclen, + .. + } => { + f(filename); + f(file_num); + reclen.iter_mut().for_each(&mut *f); + } + StmtKind::Close { file_num } => file_num.iter_mut().for_each(&mut *f), + StmtKind::Field { file_num, fields } => { + f(file_num); + for slice in fields { + f(&mut slice.width); + lvalue(&mut slice.target, f); + } + } + StmtKind::SetField { target, value, .. } => { + lvalue(target, f); + f(value); + } + StmtKind::GetPut { + file_num, record, .. + } => { + f(file_num); + record.iter_mut().for_each(&mut *f); + } + StmtKind::Lock { + file_num, range, .. + } => { + f(file_num); + if let Some((start, end)) = range { + f(start); + end.iter_mut().for_each(&mut *f); + } + } + // Statements that hold no expression of their own. Listed rather than + // matched with a wildcard so that a new variant carrying one is a + // compile error here. + StmtKind::Label(_) + | StmtKind::LabelName(_) + | StmtKind::Goto(_) + | StmtKind::Gosub(_) + | StmtKind::Return + | StmtKind::Sub { .. } + | StmtKind::Function { .. } + | StmtKind::ExitLoop { .. } + | StmtKind::ExitProc + | StmtKind::OptionBase(_) + | StmtKind::TypeDef { .. } + | StmtKind::Data(_) + | StmtKind::Restore(_) + | StmtKind::Cls + | StmtKind::End + | StmtKind::Stop => {} + } +} + #[derive(Default)] struct Analyzer { symbols: Symbols, @@ -532,8 +709,24 @@ impl Analyzer { let key = (scope.clone(), decl.name.to_uppercase()); if let Some(ty) = &decl.ty { + // A declared type and a type suffix must agree, or + // there is no telling which the program meant. The + // same check has always guarded a FUNCTION's result + // type; DIM accepted the contradiction silently. + if decl.name.ends_with(|c| "%&!#$".contains(c)) + && !matches!(ty, TypeRef::Record(_)) + && DataType::from_type_ref(ty) != DataType::from_suffix(&decl.name) + { + self.error( + stmt.line, + format!( + "'{}' is declared AS a different type than its name's suffix", + decl.name + ), + ); + } if let TypeRef::Record(r) = ty { - if !self.symbols.records.contains_key(&r.to_uppercase()) { + if !self.symbols.records.contains_key(normalized(r)) { self.error(stmt.line, format!("undefined TYPE '{}'", r)); } } @@ -635,7 +828,7 @@ impl Analyzer { for p in params { if let Some(ty) = &p.ty { if let TypeRef::Record(r) = ty { - if !self.symbols.records.contains_key(&r.to_uppercase()) { + if !self.symbols.records.contains_key(normalized(r)) { self.error( stmt.line, format!( @@ -662,7 +855,169 @@ impl Analyzer { } } - // Pass 2: check uses + /// Refuse a `RETURN` in a program that contains no `GOSUB`. + /// + /// codegen defines the GOSUB return stack only when it has seen a GOSUB, so + /// a lone RETURN emitted a reference to `_gosub_sp` that nothing defined and + /// the user was handed `ld: undefined reference to _gosub_sp`. Reaching the + /// linker with a name the compiler invented is exactly what this pass exists + /// to prevent. + /// + /// The check is deliberately whole-program rather than per-path: deciding + /// whether a particular RETURN can be reached from a particular GOSUB needs + /// the control flow, and GOTO makes that undecidable. "No GOSUB at all" is + /// sound, and it is the shape a mistake actually takes. + fn check_return_has_a_gosub(&mut self, stmts: &[Stmt]) { + let mut has_gosub = false; + let mut first_return = None; + walk_stmts(stmts, &mut |stmt| match &stmt.kind { + StmtKind::Gosub(_) | StmtKind::OnGosub { .. } => has_gosub = true, + StmtKind::Return if first_return.is_none() => first_return = Some(stmt.line), + _ => {} + }); + if let (false, Some(line)) = (has_gosub, first_return) { + self.error_with_note( + line, + "RETURN without GOSUB".to_string(), + "RETURN ends a GOSUB; use EXIT SUB or EXIT FUNCTION to leave a procedure" + .to_string(), + ); + } + } + + /// Refuse a name declared as an array and also as a procedure or builtin. + /// + /// `A(1)` reaches the compiler as one shape and has to become one thing. + /// Sema resolved such a clash in favour of the array and codegen in favour + /// of the procedure, and the parser's DIM-order heuristic hid the + /// disagreement for as long as it lasted. `DIM F(5)` alongside + /// `FUNCTION F(X)` compiled silently; so did `DIM LEN(5)`, where codegen's + /// builtin table would answer first and the array would never be read. + /// + /// Rather than pick an order and document it, refuse the program: nobody + /// writes this on purpose, and either resolution surprises somebody. + fn check_name_collisions(&mut self) { + let arrays: Vec<(Scope, String, u32)> = self + .symbols + .arrays + .iter() + .map(|((scope, name), info)| (scope.clone(), name.clone(), info.line)) + .collect(); + + for (_, name, line) in arrays { + if let Some(proc_info) = self.symbols.procs.get(&name) { + let kind = if proc_info.is_function { + "FUNCTION" + } else { + "SUB" + }; + let proc_line = proc_info.line; + self.error_with_note( + line, + format!("'{}' is declared both as an array and as a {}", name, kind), + format!("the {} is on line {}", kind, proc_line), + ); + } else if builtin(&name).is_some() { + self.error( + line, + format!( + "'{}' is the name of a built-in function and cannot also be an array", + name + ), + ); + } + } + } + + // Pass 2: resolve `A(1)` against the symbol table + + /// Turn every `FnCall` naming an array into an `ArrayAccess`. + /// + /// The parser emits `FnCall` for all of `name(args)`, because at that point + /// nothing knows whether `name` is an array, a procedure or a builtin. Here + /// the symbol table is complete and scoped, so the question has one answer + /// regardless of where the `DIM` was written. + /// + /// Doing it as a rewrite keeps `ArrayAccess` in the AST that codegen sees, + /// which matters for more than tidiness: `expr_is_call_free` treats every + /// `FnCall` as opaque, so leaving array reads as calls would silently + /// disable FOR-counter promotion, and `walk_array_uses` collects names only + /// from `ArrayAccess`, so loop-invariant descriptor hoisting would stop + /// firing. Both would have been invisible except as lost performance. + fn resolve_array_accesses(&mut self, stmts: &mut [Stmt], scope: &Scope) { + for stmt in stmts { + for_each_expr_mut(stmt, &mut |e| Self::resolve_expr(&self.symbols, e, scope)); + + // Nested bodies, entering procedure scope where there is one. + match &mut stmt.kind { + StmtKind::Sub { name, body, .. } | StmtKind::Function { name, body, .. } => { + // `clone`, not `to_uppercase`: the lexer already uppercased + // it, and `collect` and `check` build this scope the same + // way -- a scope key that normalized differently from + // theirs would simply fail to match. + let inner = Scope::Proc(name.clone()); + self.resolve_array_accesses(body, &inner); + } + StmtKind::If { + then_branch, + else_branch, + .. + } => { + self.resolve_array_accesses(then_branch, scope); + if let Some(eb) = else_branch { + self.resolve_array_accesses(eb, scope); + } + } + StmtKind::SelectCase { cases, .. } => { + for (_, body) in cases { + self.resolve_array_accesses(body, scope); + } + } + StmtKind::For { body, .. } + | StmtKind::While { body, .. } + | StmtKind::DoLoop { body, .. } => self.resolve_array_accesses(body, scope), + _ => {} + } + } + } + + /// Rewrite one expression tree, innermost first. + fn resolve_expr(symbols: &Symbols, expr: &mut Expr, scope: &Scope) { + match expr { + Expr::Literal(_) | Expr::Variable(_) => return, + Expr::Unary { operand, .. } => Self::resolve_expr(symbols, operand, scope), + Expr::Binary { left, right, .. } => { + Self::resolve_expr(symbols, left, scope); + Self::resolve_expr(symbols, right, scope); + } + Expr::Field { base, .. } => Self::resolve_expr(symbols, base, scope), + Expr::ArrayAccess { indices, .. } => { + for i in indices { + Self::resolve_expr(symbols, i, scope); + } + } + Expr::FnCall { args, .. } => { + for a in args.iter_mut() { + Self::resolve_expr(symbols, a, scope); + } + } + } + + // A procedure wins over an array of the same name, matching what + // codegen has always done; `check_name_collisions` refuses the program + // that makes the two disagree, so the order is unobservable. + if let Expr::FnCall { name, args } = expr { + let upper = normalized(name); + if !symbols.procs.contains_key(upper) && symbols.lookup_array(scope, upper).is_some() { + *expr = Expr::ArrayAccess { + name: std::mem::take(name), + indices: std::mem::take(args), + }; + } + } + } + + // Pass 3: check uses fn check(&mut self, stmts: &[Stmt], scope: &Scope) { for stmt in stmts { @@ -942,7 +1297,14 @@ impl Analyzer { } } } - if a.name.ends_with('$') != b.name.ends_with('$') { + // Compare what the operands *are*, not the suffix of the + // variable they hang off: for `P.N` the base is a record and + // carries no suffix, so a string field and a numeric one both + // looked non-string and the mismatch reached codegen, which + // stored a string pointer into an INTEGER slot and crashed. + let a_str = self.expr_is_string(&lvalue_as_expr(a), scope); + let b_str = self.expr_is_string(&lvalue_as_expr(b), scope); + if a_str.is_some() && b_str.is_some() && a_str != b_str { self.error(line, "SWAP requires both values to be the same type"); } } @@ -1033,7 +1395,7 @@ impl Analyzer { fn const_eval(&self, e: &Expr) -> Option { match e { Expr::Literal(l) => Some(l.clone()), - Expr::Variable(n) => self.symbols.consts.get(&n.to_uppercase()).cloned(), + Expr::Variable(n) => self.symbols.consts.get(normalized(n)).cloned(), Expr::Unary { op, operand } => { let v = self.const_eval(operand)?; match (op, v) { @@ -1086,7 +1448,7 @@ impl Analyzer { let Expr::Variable(arr) = first else { return Some(format!("argument 1 of '{}' must be an array name", name)); }; - let Some(info) = self.symbols.lookup_array(scope, &arr.to_uppercase()) else { + let Some(info) = self.symbols.lookup_array(scope, normalized(arr)) else { return Some(format!("'{}' is not a declared array", arr)); }; let rank = info.rank; @@ -1183,7 +1545,7 @@ impl Analyzer { }; for f in &fields { let TypeRef::Record(rec) = &ty else { return }; - let Some(info) = self.symbols.records.get(&rec.to_uppercase()) else { + let Some(info) = self.symbols.records.get(normalized(rec)) else { return; }; match info.field(f) { @@ -1597,7 +1959,7 @@ impl Analyzer { ty = self .symbols .records - .get(&rec.to_uppercase())? + .get(normalized(rec))? .field(field)? .ty .clone(); @@ -1626,12 +1988,12 @@ impl Analyzer { ); return None; }; - let info = self.symbols.records.get(&rec.to_uppercase())?; + let info = self.symbols.records.get(normalized(rec))?; match info.field(field) { Some(f) => ty = f.ty.clone(), None => { let known: Vec<&str> = info.fields.iter().map(|(n, _)| n.as_str()).collect(); - match closest(&field.to_uppercase(), &known) { + match closest(normalized(field), &known) { Some(sug) => self.error_with_note( line, format!("TYPE '{}' has no field '{}'", rec, field), @@ -1691,8 +2053,16 @@ impl Analyzer { match expr { Expr::Literal(Literal::String(_)) => Some(true), Expr::Literal(_) => Some(false), - Expr::Variable(name) => Some(name.ends_with('$')), - Expr::ArrayAccess { name, .. } => Some(name.ends_with('$')), + // A `$` suffix is the usual way to be a string, but not the only + // one: `DIM S AS STRING * 4` says so with no suffix at all, and + // without consulting that, `LEN(S)` and `S = "xy"` were both + // rejected as numeric. + Expr::Variable(name) | Expr::ArrayAccess { name, .. } => { + Some(match self.symbols.typed_var(scope, name) { + Some(t) => matches!(t, TypeRef::FixedString(_)), + None => name.ends_with('$'), + }) + } Expr::Unary { .. } => Some(false), Expr::Binary { op, left, .. } => { if matches!( @@ -1716,7 +2086,7 @@ impl Analyzer { let TypeRef::Record(rec) = &ty else { return None; }; - let info = self.symbols.records.get(&rec.to_uppercase())?; + let info = self.symbols.records.get(normalized(rec))?; ty = info.field(field)?.ty.clone(); } Some(matches!(ty, TypeRef::FixedString(_))) diff --git a/tests/arithmetic/mod.rs b/tests/arithmetic/mod.rs index 47f2629..3ea72a2 100644 --- a/tests/arithmetic/mod.rs +++ b/tests/arithmetic/mod.rs @@ -47,6 +47,14 @@ PRINT -5 + 10 assert_eq!(lines[2], "5", "negative"); } +/// The logical operators used as conditions. +/// +/// `IF NOT 1` is the interesting line. `NOT` is a *bitwise* complement, so +/// `NOT 1` is -2 -- non-zero, therefore true. This test used to assert that it +/// was false, which is what a logical not would give; the two agree only when +/// the operand is already 0 or -1. Comparisons yield exactly those values, +/// which is why `IF NOT (A > 0)` reads the way anyone would expect while +/// `IF NOT 1` does not. #[test] fn test_logical_operators() { // Tests: AND, OR, NOT, XOR @@ -57,7 +65,9 @@ IF 1 AND 0 THEN PRINT "and-no" IF 0 OR 1 THEN PRINT "or-yes" IF 0 OR 0 THEN PRINT "or-no" IF NOT 0 THEN PRINT "not-yes" -IF NOT 1 THEN PRINT "not-no" +IF NOT 1 THEN PRINT "not-minus-two-is-true" +IF NOT (1 > 0) THEN PRINT "not-comparison-no" +IF NOT (1 < 0) THEN PRINT "not-comparison-yes" IF 1 XOR 0 THEN PRINT "xor-a" IF 0 XOR 1 THEN PRINT "xor-b" IF 1 XOR 1 THEN PRINT "xor-c" @@ -68,7 +78,46 @@ IF 0 XOR 0 THEN PRINT "xor-d" let lines: Vec<&str> = output.trim().lines().collect(); assert_eq!( lines, - vec!["and-yes", "or-yes", "not-yes", "xor-a", "xor-b"] + vec![ + "and-yes", + "or-yes", + "not-yes", + "not-minus-two-is-true", + "not-comparison-yes", + "xor-a", + "xor-b" + ] + ); +} + +/// `NOT` complements every bit; it is not a boolean negation. +/// +/// It used to emit `sete al / movzx / neg` -- "0 gives -1, anything else gives +/// 0" -- so `NOT 12` was 0 rather than -13, while `12 AND 10` beside it was +/// correctly bitwise. LANGREF says these "operate bitwise on integers". +#[test] +fn test_not_is_a_bitwise_complement() { + let output = compile_and_run( + r#" +PRINT NOT 12 +A = 12 +PRINT NOT A +B% = 12 +PRINT NOT B% +PRINT NOT 0 +PRINT NOT -1 +C = 1.9 +PRINT NOT C +D% = &H00FF +PRINT NOT D% +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + vec!["-13", "-13", "-13", "-1", "0", "-2", "-256"], + "literal, Double, Integer, the boolean values, a truncated Double, and a bit mask" ); } @@ -201,7 +250,6 @@ PRINT (-2) ^ 2 PRINT 0 - 2 ^ 2 A = 3 PRINT -A ^ 2 -PRINT 2 ^ 3 ^ 2 PRINT -2 * 3 PRINT 1 - -2 "#, @@ -210,8 +258,8 @@ PRINT 1 - -2 let lines: Vec<&str> = output.trim().lines().collect(); assert_eq!( lines, - vec!["-4", "-8", "4", "-4", "-9", "512", "-6", "3"], - "^ binds tighter than unary minus; ^ stays right-associative" + vec!["-4", "-8", "4", "-4", "-9", "-6", "3"], + "^ binds tighter than unary minus" ); } @@ -314,3 +362,155 @@ PRINT CSNG(1 / 3) let lines: Vec<&str> = output.trim().lines().collect(); assert_eq!(lines, vec!["3.14159", "0.1", "3.14", "0.33333334"]); } + +/// The bitwise operators must work on Double operands, which is what an +/// unsuffixed variable is. +/// +/// `promote_types` had no case for AND/OR/XOR, so their result type came out +/// Double while codegen emitted the answer into EAX. PRINT then called +/// `_rt_file_print_float`, read xmm0 -- still holding the left operand -- and +/// the result was silently discarded: +/// +/// A = 12 : B = 10 : PRINT A AND B printed 12, not 8 +/// +/// Every existing test used literal operands, which are constant-folded before +/// this path is reached, so 470 tests coexisted with it. +#[test] +fn test_bitwise_operators_on_double_variables() { + let output = compile_and_run( + r#" +A = 12 : B = 10 +PRINT A AND B +PRINT A OR B +PRINT A XOR B +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["8", "14", "6"], "AND/OR/XOR over Double"); +} + +/// The same through every type, and through assignment as well as PRINT -- +/// the result was discarded identically in both. +#[test] +fn test_bitwise_operators_across_types() { + let output = compile_and_run( + r#" +A% = 12 : B% = 10 +PRINT A% AND B% +C& = 12 : D& = 10 +PRINT C& AND D& +E = 12 : F = 10 +G = E AND F +PRINT G +H% = E AND F +PRINT H% +IF (E AND F) = 8 THEN PRINT "cond-ok" ELSE PRINT "cond-bad" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + vec!["8", "8", "8", "8", "cond-ok"], + "INTEGER, LONG, Double assignment, narrowing, and IF condition" + ); +} + +/// The neighbouring operators in `promote_types` must not shift: `\`, MOD and +/// the comparisons already returned Long and were correct. +#[test] +fn test_non_bitwise_operators_on_doubles_are_unchanged() { + let output = compile_and_run( + r#" +A = 12 : B = 10 +PRINT A \ B +PRINT A MOD B +PRINT A + B +PRINT A / B +PRINT A = B +PRINT A > B +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["1", "2", "22", "1.2", "0", "-1"]); +} + +/// Operator precedence must match LANGREF's own table. +/// +/// It listed `NOT` (6) as binding tighter than `AND` (7), and `OR` and `XOR` +/// together at 8 -- but the parser gave `XOR` its own level tighter than `AND`, +/// and parsed `NOT`'s operand at the caller's precedence so that `NOT` swallowed +/// whatever followed it. Both are silent wrong answers, not rejections. +#[test] +fn test_logical_operator_precedence() { + let output = compile_and_run( + r#" +A% = 0 : B% = 0 +PRINT NOT A% AND B% +PRINT (NOT A%) AND B% +PRINT -1 OR 0 XOR -1 +PRINT (-1 OR 0) XOR -1 +PRINT 1 AND 0 OR 1 +PRINT 12 XOR 10 AND 6 +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + // NOT binds tighter than AND, so the first two agree. + assert_eq!(&lines[0..2], &["0", "0"], "NOT binds tighter than AND"); + // OR and XOR share a level and associate left to right, so these agree too. + assert_eq!(&lines[2..4], &["0", "0"], "OR and XOR share a level"); + // AND binds tighter than OR: (1 AND 0) OR 1 = 1. + assert_eq!(lines[4], "1", "AND binds tighter than OR"); + // AND binds tighter than XOR: 12 XOR (10 AND 6) = 12 XOR 2 = 14. + assert_eq!(lines[5], "14", "AND binds tighter than XOR"); +} + +/// `NOT` still binds looser than a comparison, which is the form that matters. +#[test] +fn test_not_binds_looser_than_comparison() { + let output = compile_and_run( + r#" +A = 1 : B = 2 +IF NOT A = B THEN PRINT "not-equal" ELSE PRINT "equal" +IF NOT A < B THEN PRINT "not-less" ELSE PRINT "less" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + // NOT (A = B): A <> B, so NOT 0 = -1, true. + assert_eq!( + lines[0], "not-equal", + "NOT groups over the whole comparison" + ); + // NOT (A < B): A < B, so NOT -1 = 0, false. + assert_eq!(lines[1], "less"); +} + +/// Equal-precedence operators associate left to right, `^` included. +/// +/// `^` was right-associative, so `2 ^ 3 ^ 2` gave 512 where GW-BASIC and +/// QuickBASIC give 64. Unary minus still binds looser than `^`. +#[test] +fn test_operator_associativity() { + let output = compile_and_run( + r#" +PRINT 2 ^ 3 ^ 2 +PRINT -2 ^ 2 +PRINT 2 ^ -2 +PRINT 100 - 10 - 5 +PRINT 100 / 10 / 5 +PRINT 2 ^ 3 ^ 2 ^ 1 +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines[0], "64", "(2^3)^2, not 2^(3^2)"); + assert_eq!(lines[1], "-4", "^ binds tighter than unary minus"); + assert_eq!(lines[2], "0.25", "a negative exponent still parses"); + assert_eq!(lines[3], "85", "subtraction is left to right"); + assert_eq!(lines[4], "2", "division is left to right"); + assert_eq!(lines[5], "64", "((2^3)^2)^1"); +} diff --git a/tests/arrays/mod.rs b/tests/arrays/mod.rs index 91fd74c..b906473 100644 --- a/tests/arrays/mod.rs +++ b/tests/arrays/mod.rs @@ -189,3 +189,48 @@ fn test_bounds_with_computed_dimension() { .unwrap(); assert_eq!(output.trim(), "552"); } + +/// An array behaves the same wherever its DIM is written. +/// +/// The parser used to decide between array access and function call from the +/// DIM statements it had read so far, so the same source produced a different +/// AST depending on whether the DIM came earlier or later -- and it ignored +/// scope, so a DIM inside a SUB changed how module-level code parsed. Sema now +/// resolves it against the finished symbol table. +#[test] +fn test_array_use_before_its_dim() { + let output = compile_and_run( + r#" +SUB Show(N) +PRINT A(N) +END SUB +DIM A(3) +A(1) = 7 +Show 1 +PRINT A(1) +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["7", "7"], "read before and after the DIM agree"); +} + +/// A DIM inside a procedure must not make module-level code resolve to it. +#[test] +fn test_procedure_local_dim_does_not_escape() { + let output = compile_and_run( + r#" +SUB Local +DIM Q(3) +Q(1) = 5 +PRINT Q(1) +END SUB +Local +PRINT Q +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines[0], "5", "the local array works"); + assert_eq!(lines[1], "0", "Q at module level is an untouched scalar"); +} diff --git a/tests/codegen/mod.rs b/tests/codegen/mod.rs index a107bec..b2bbb99 100644 --- a/tests/codegen/mod.rs +++ b/tests/codegen/mod.rs @@ -1044,3 +1044,100 @@ PRINT T# let got: Vec<&str> = out.trim().lines().collect(); assert_eq!(got, vec!["9", "23", "90"]); } + +/// Every call in generated code must be made with the stack 16-byte aligned. +/// +/// `tests/runtime` already checks this for the hand-written runtime; nothing +/// checked the code the compiler emits. MID$ and INSTR each pushed three +/// callee-saved registers and never padded, so every call in those sequences +/// was 8 bytes out. The leaf helpers tolerated it, which is why it went +/// unnoticed -- until an argument check jumped to an error trampoline, whose +/// `_rt_error` calls into the C library. On Linux that survived; on Windows it +/// was an access violation, and the Linux CI could not see it. +/// +/// The model is a linear scan of `main`'s body, which is sound here because +/// each program below is straight-line: no loops, no branches of its own. A +/// conditional jump to an error trampoline is checked too, since the trampoline +/// touches no stack before calling and so inherits the jump site's alignment. +#[test] +fn test_generated_calls_are_stack_aligned() { + let programs = [ + ("MID$ three args", "PRINT MID$(\"abcdef\", 2, 3)\n"), + ("MID$ two args", "PRINT MID$(\"abcdef\", 2)\n"), + ("INSTR two args", "PRINT INSTR(\"abc\", \"b\")\n"), + ("INSTR three args", "PRINT INSTR(2, \"abc\", \"b\")\n"), + ("LEFT$", "PRINT LEFT$(\"abc\", 2)\n"), + ("RIGHT$", "PRINT RIGHT$(\"abc\", 2)\n"), + ("STRING$", "PRINT STRING$(3, \"x\")\n"), + ("SPACE$", "PRINT SPACE$(3)\n"), + ("CHR$", "PRINT CHR$(65)\n"), + ("ASC", "PRINT ASC(\"A\")\n"), + ( + "nested", + "PRINT MID$(LEFT$(\"abcdef\", 5), INSTR(\"abc\", \"b\"), 2)\n", + ), + ("concat", "PRINT MID$(\"abc\", 1, 2) + RIGHT$(\"xyz\", 1)\n"), + ( + "array subscript", + "DIM A(3)\nA(1) = 2\nPRINT MID$(\"abcdef\", A(1), 2)\n", + ), + ("REDIM PRESERVE", "DIM A(3)\nREDIM PRESERVE A(5)\n"), + ("REDIM plain", "DIM A(3)\nREDIM A(5)\n"), + ("string concat", "A$ = \"x\" + \"y\"\n"), + ("PRINT USING", "PRINT USING \"##.##\"; 1.5\n"), + ("SWAP", "A% = 1 : B% = 2\nSWAP A%, B%\n"), + ("fixed string", "DIM S AS STRING * 4\nS = \"xy\"\n"), + ]; + + let mut problems = Vec::new(); + for (what, source) in programs { + let asm = compile_to_asm(source).expect("must compile"); + // `offset` is rsp modulo 16 measured from the ABI's state on entry. + // After `push rbp` it is 0, and every call must see it at 0. + let mut offset: i64 = 8; + let mut in_main = false; + + for raw in asm.lines() { + let line = raw.split('#').next().unwrap_or("").trim(); + if line.is_empty() { + continue; + } + if line == "main:" { + in_main = true; + offset = 8; + continue; + } + if !in_main { + continue; + } + // The body ends at the first `ret`; the trampolines past it are + // reached by jumps, so a linear scan cannot model them. + if line == "ret" { + break; + } + + if line.starts_with("push ") || line.starts_with("pop ") { + offset = (offset + 8) % 16; + } else if let Some(n) = line.strip_prefix("sub rsp, ") { + offset = (offset - n.trim().parse::().expect("literal")).rem_euclid(16); + } else if let Some(n) = line.strip_prefix("add rsp, ") { + offset = (offset + n.trim().parse::().expect("literal")) % 16; + } else if line == "leave" { + offset = 8; + } else if let Some(target) = line.strip_prefix("call ") { + if offset != 0 { + problems.push(format!("{what}: call {target} with rsp % 16 == {offset}")); + } + } else if line.starts_with('j') && line.contains(".Lerr_") && offset != 0 { + problems.push(format!("{what}: {line} with rsp % 16 == {offset}")); + } + } + } + + assert!( + problems.is_empty(), + "{} misaligned site(s):\n{}", + problems.len(), + problems.join("\n") + ); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9834455..13c0c1f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -125,6 +125,12 @@ pub fn compile_and_run_flags( } let mut child = Command::new(&exe_file) + // Run inside the temp dir, so a program that opens a file by a bare + // name writes it there and it dies with the TempDir. Without this the + // program inherited the test runner's directory -- the repository root + // -- and `OPEN "a.txt" FOR OUTPUT` left a stray file behind on every + // run, two of which were committed by accident. + .current_dir(tmp.path()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) diff --git a/tests/control/mod.rs b/tests/control/mod.rs index f942180..1b2ba8a 100644 --- a/tests/control/mod.rs +++ b/tests/control/mod.rs @@ -543,7 +543,9 @@ fn test_swap_record_string_fields() { "TYPE P\nN AS STRING * 8\nEND TYPE\nDIM A AS P\nDIM B AS P\nA.N = \"aa\"\nB.N = \"bb\"\nSWAP A.N, B.N\nPRINT A.N; \" \"; B.N\n", ) .unwrap(); - assert_eq!(output.trim(), "bb aa"); + // Both fields are `STRING * 8`, so each is padded to eight characters; + // `output.trim()` removes only the trailing pad of the second. + assert_eq!(output.trim(), "bb aa"); } /// And for a field of an array element, whose address is only known at run @@ -873,3 +875,214 @@ fn test_on_without_goto_or_gosub_is_diagnosed() { err.stderr ); } + +/// Every colon-separated statement after `THEN` belongs to the THEN branch. +/// +/// The parser used to take exactly one statement, so the rest of the line +/// escaped the conditional and ran unconditionally: with `X = 0`, the program +/// below printed `B`. Nothing in the suite used the form, so it stayed green. +#[test] +fn test_single_line_if_takes_every_statement_after_then() { + let output = compile_and_run( + r#" +X = 0 +IF X = 1 THEN PRINT "A" : PRINT "B" +PRINT "done" +X = 1 +IF X = 1 THEN PRINT "C" : PRINT "D" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["done", "C", "D"], "the whole tail is conditional"); +} + +/// The same, inside a loop, where the leak was loudest. +/// +/// With the trailing statement unconditional this printed `x two x x` across +/// three iterations instead of `two x` on the second alone. +#[test] +fn test_single_line_if_inside_a_loop() { + let output = compile_and_run( + r#" +FOR I = 1 TO 3 +IF I = 2 THEN PRINT "two" : PRINT "x" +NEXT I +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["two", "x"], "only the matching iteration prints"); +} + +/// A bare line number after THEN or ELSE is an implied GOTO. +/// +/// This is how GW-BASIC spells the commonest branch of all, and the form +/// appears throughout published listings. It was rejected outright with +/// "Unexpected token: Integer(30)", since a statement cannot otherwise begin +/// with a number. +#[test] +fn test_if_then_line_number_is_an_implied_goto() { + let output = compile_and_run( + r#" +10 IF 1 = 1 THEN 30 +20 PRINT "skipped" +30 PRINT "target" +"#, + ) + .unwrap(); + assert_eq!(output.trim(), "target", "THEN branches"); +} + +/// The same after ELSE, and mixed with an ordinary statement. +#[test] +fn test_if_then_else_line_numbers() { + let output = compile_and_run( + r#" +10 X = 0 +20 IF X = 1 THEN 40 ELSE 60 +40 PRINT "then" +50 GOTO 70 +60 PRINT "else" +70 IF X = 0 THEN 90 ELSE PRINT "no" +80 PRINT "unreachable" +90 PRINT "done" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + &["else", "done"], + "both branches accept a line number" + ); +} + +/// `ELSE` ends the THEN branch and opens its own colon-separated list. +/// +/// This form did not merely misbehave, it failed to compile: the second +/// statement became a sibling of the IF, so the `ELSE` that followed it +/// reached the top level and was rejected as "ELSE without matching IF". +#[test] +fn test_single_line_if_else_both_take_statement_lists() { + let output = compile_and_run( + r#" +X = 9 +IF X = 1 THEN PRINT "a" : PRINT "b" ELSE PRINT "c" : PRINT "d" +PRINT "end" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["c", "d", "end"], "the ELSE branch takes the tail"); +} + +/// `NEXT` names the loop it closes, and the name is checked. +/// +/// The control variable was parsed and thrown away, so crossed NEXTs compiled +/// into a loop nesting nobody wrote: +/// +/// FOR I = 1 TO 2 +/// FOR J = 1 TO 2 +/// NEXT I ' actually closed the J loop +/// NEXT J ' actually closed the I loop +/// +/// GW-BASIC rejects that as "NEXT without FOR". +#[test] +fn test_next_variable_must_match_its_for() { + for source in [ + "FOR I = 1 TO 2\nPRINT I\nNEXT J\n", + "FOR I = 1 TO 2\nFOR J = 1 TO 2\nPRINT I\nNEXT I\nNEXT J\n", + ] { + let e = crate::common::compile_only(source) + .expect_err(&format!("{source:?} should be refused")); + assert!( + e.contains("NEXT"), + "the diagnostic should name NEXT: {}", + e.stderr + ); + assert!(e.is_clean_rejection()); + } +} + +/// A bare `NEXT` still closes the innermost loop, and a matching name is fine. +#[test] +fn test_next_bare_and_matching_still_work() { + let output = compile_and_run( + r#" +FOR I = 1 TO 2 + FOR J = 1 TO 2 + PRINT I; J + NEXT J +NEXT I +FOR K = 1 TO 2 + FOR L = 1 TO 2 + NEXT +NEXT +PRINT "done" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["11", "12", "21", "22", "done"]); +} + +/// One `NEXT` may close several loops, innermost name first. +#[test] +fn test_next_closes_several_loops() { + let output = compile_and_run( + r#" +FOR I = 1 TO 2 + FOR J = 1 TO 2 + PRINT I; J +NEXT J, I +PRINT "done" +FOR A = 1 TO 2 + FOR B = 1 TO 2 + FOR C = 1 TO 2 + T = T + 1 +NEXT C, B, A +PRINT T +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["11", "12", "21", "22", "done", "8"]); +} + +/// The names in a multi-loop `NEXT` are checked in order, and a name with no +/// loop left to close is refused rather than silently dropped. +#[test] +fn test_next_list_is_checked() { + for source in [ + // Wrong order: J is the inner loop, so `NEXT I, J` is backwards. + "FOR I = 1 TO 2\nFOR J = 1 TO 2\nNEXT I, J\n", + // One name too many. + "FOR I = 1 TO 2\nNEXT I, J\n", + ] { + let e = crate::common::compile_only(source) + .expect_err(&format!("{source:?} should be refused")); + assert!(e.is_clean_rejection(), "stderr: {}", e.stderr); + } +} + +/// `IF cond THEN` followed by only colons opens a block, not a single-line IF. +/// +/// The single-line test treated anything other than end-of-line as the start of +/// a statement, so the colons made this a one-line IF and the END IF below was +/// then unmatched. +#[test] +fn test_if_then_trailing_colon_is_still_a_block() { + let output = compile_and_run( + r#" +X = 1 +IF X = 1 THEN : +PRINT "in" +END IF +PRINT "after" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["in", "after"]); +} diff --git a/tests/data/mod.rs b/tests/data/mod.rs index 88e25e6..7c5683e 100644 --- a/tests/data/mod.rs +++ b/tests/data/mod.rs @@ -130,3 +130,132 @@ fn test_restore_to_label() { .unwrap(); assert_eq!(output.trim(), "13"); } + +/// DATA items need quotes only when they contain a comma, a colon, or +/// significant surrounding spaces -- GW-BASIC's rule. +/// +/// The parser only accepted Integer, Float, String and a leading minus, so an +/// unquoted word ended the item list silently and the word itself was then +/// parsed as a fresh statement, producing an error about the *word* rather than +/// about DATA. Reassembling items from tokens would not have worked either: the +/// lexer uppercases identifiers, so `DATA hello` would have yielded "HELLO". +#[test] +fn test_unquoted_data_items() { + let output = compile_and_run( + r#" +DATA hello, World, MiXeD +READ A$, B$, C$ +PRINT A$ +PRINT B$ +PRINT C$ +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["hello", "World", "MiXeD"], "case is preserved"); +} + +/// Surrounding spaces are trimmed from an unquoted item and kept in a quoted +/// one, and a quoted item may contain the separators. +#[test] +fn test_data_quoting_rules() { + let output = compile_and_run( + r#" +DATA spaced , " kept ", "a,b", "c:d" +READ A$, B$, C$, D$ +PRINT "["; A$; "]" +PRINT "["; B$; "]" +PRINT C$ +PRINT D$ +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + vec!["[spaced]", "[ kept ]", "a,b", "c:d"], + "unquoted items trim, quoted items do not" + ); +} + +/// An omitted item reads as zero, or as the empty string. +#[test] +fn test_data_empty_items() { + let output = compile_and_run( + r#" +DATA 1,,3 +READ A, B, C +PRINT A +PRINT B +PRINT C +DATA ,x +READ D$, E$ +PRINT "["; D$; "]["; E$; "]" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["1", "0", "3", "[][x]"]); +} + +/// A colon still ends the DATA statement, so a statement may follow it on the +/// same line -- as it could before. +#[test] +fn test_data_ends_at_a_colon() { + let output = compile_and_run( + r#" +DATA 1, 2 : PRINT "after" +READ A, B +PRINT A + B +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["after", "3"]); +} + +/// A DATA item may be READ as either type, whichever way its tag points. +/// +/// `_rt_read_number` has always parsed a string entry with strtod, but +/// `_rt_read_string` ignored the tag entirely and passed the entry's second +/// word to strlen whatever it held -- so `DATA 42` followed by `READ A$` +/// measured a string at address 42 and the program died with SIGSEGV. Nothing +/// in sema caught it, and nothing could: pairing a READ with the item it will +/// consume needs the execution path, which RESTORE and branching hide. +#[test] +fn test_numeric_data_read_into_a_string() { + let output = compile_and_run( + r#" +DATA 42, 3.5, -7, text +READ A$, B$, C$, D$ +PRINT A$ +PRINT B$ +PRINT C$ +PRINT D$ +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + vec!["42", "3.5", "-7", "text"], + "rendered as PRINT would" + ); +} + +/// And the mirror image, which already worked: a string item read as a number. +#[test] +fn test_string_data_read_as_a_number() { + let output = compile_and_run( + r#" +DATA "12", "3.5", "notanumber" +READ A, B, C +PRINT A +PRINT B +PRINT C +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, vec!["12", "3.5", "0"]); +} diff --git a/tests/errors/mod.rs b/tests/errors/mod.rs index 093777d..7f2e5d6 100644 --- a/tests/errors/mod.rs +++ b/tests/errors/mod.rs @@ -7,7 +7,7 @@ // Copyright (c) 2025-2026 Jeff Garzik // SPDX-License-Identifier: MIT -use crate::common::{compile_and_run_flags, compile_and_run_raw, compile_only}; +use crate::common::{compile_and_run, compile_and_run_flags, compile_and_run_raw, compile_only}; /// The harness itself must be able to tell a rejected program from an accepted one. #[test] @@ -51,7 +51,9 @@ fn test_harness_supplies_empty_stdin() { let run = compile_and_run_raw("INPUT N\nPRINT \"read\"\n", "").expect("should compile"); assert_eq!( run.lines(), - vec!["read"], + // The promptless INPUT prints `? `, which shares the line with the + // PRINT that follows it. + vec!["? read"], "program ran to completion at EOF" ); assert_eq!(run.exit_code, Some(0)); @@ -933,6 +935,320 @@ fn test_unsupported_diagnostics_explain_themselves() { ); } +/// A name cannot be both an array and a procedure, or an array and a builtin. +/// +/// `A(1)` has to resolve to one thing. Sema resolved such a clash in favour of +/// the array and codegen in favour of the procedure, and the parser's DIM-order +/// heuristic hid the disagreement for as long as it lasted -- both of these +/// compiled silently. Nobody writes this on purpose, and either resolution +/// surprises somebody, so the program is refused instead. +#[test] +fn test_array_and_procedure_name_collisions_are_diagnosed() { + expect_rejected( + "DIM F(5)\nF(1) = 7\nFUNCTION F(X)\nF = X * 2\nEND FUNCTION\n", + "declared both as an array and as a FUNCTION", + ); + expect_rejected( + "DIM S(5)\nSUB S(X)\nPRINT X\nEND SUB\n", + "declared both as an array and as a SUB", + ); + expect_rejected( + "DIM LEN(5)\nLEN(1) = 7\n", + "'LEN' is the name of a built-in function", + ); +} + +/// Every syntax error in a program is reported, not just the first. +/// +/// Sema has always returned a Vec, so five undefined names cost one +/// compile. The parser stopped at the first error, so five typos cost five. +#[test] +fn test_parser_reports_every_error() { + let e = compile_only("X = )\nGOTO +\nY = *\nPRINT \"ok\"\n") + .expect_err("three bad statements must be refused"); + for expected in [ + "unexpected ) in an expression", + "expected a line number or label, got +", + "unexpected * in an expression", + ] { + assert!( + e.contains(expected), + "expected {expected:?} among the diagnostics: {}", + e.stderr + ); + } + assert!( + e.contains("3 errors"), + "the tally should say 3: {}", + e.stderr + ); +} + +/// One error is "1 error", not "1 errors". +#[test] +fn test_single_error_tally_is_singular() { + let e = compile_only("X = )\n").expect_err("must be refused"); + assert!(e.contains("1 error\n") || e.stderr.trim_end().ends_with("1 error")); +} + +/// Recovery happens inside blocks too, so one bad statement costs that +/// statement and not the block around it. +/// +/// Recovering only at the top level would let the error escape the SUB, strand +/// its END SUB, and produce a cascade of complaints about a SUB that was +/// perfectly well closed. +#[test] +fn test_recovery_inside_a_block_does_not_cascade() { + let e = compile_only("SUB Foo\nX = )\nPRINT 1\nEND SUB\nPRINT 2\n") + .expect_err("the bad statement must be refused"); + assert!( + e.contains("1 error"), + "only the bad statement should be reported: {}", + e.stderr + ); + assert!( + !e.contains("without matching") && !e.contains("missing its"), + "the SUB was closed correctly and must not be blamed: {}", + e.stderr + ); +} + +/// Errors found before a block that never closes are kept, not discarded. +#[test] +fn test_hard_error_keeps_the_errors_found_before_it() { + let e = compile_only("X = )\nFOR I = 1 TO 10\nPRINT I\n") + .expect_err("an unclosed FOR must be refused"); + assert!( + e.contains("unexpected ) in an expression"), + "the earlier error must survive: {}", + e.stderr + ); + assert!( + e.contains("FOR is missing its NEXT"), + "the unclosed block must be reported: {}", + e.stderr + ); +} + +/// Diagnostics quote BASIC, not Rust. +/// +/// Errors fell back to `{:?}` on the token, so they showed the lexer's variant +/// names: "Expected To, got Integer(2)" for a missing TO, and `EndSelect`, +/// `LParen` and `Ne` at programmers who had written `END SELECT`, `(` and `<>`. +/// Every fixed token now carries the spelling it was written with. +#[test] +fn test_diagnostics_quote_source_spelling() { + let cases = [ + ("FOR I = 1 2 3\nNEXT\n", "expected TO, got 2"), + ("IF THEN\n", "unexpected THEN in an expression"), + ("GOTO +\n", "expected a line number or label, got +"), + ("X = )\n", "unexpected ) in an expression"), + ( + "OPEN \"f\" FOR BOGUS AS #1\n", + "expected INPUT, OUTPUT, APPEND or RANDOM, got identifier 'BOGUS'", + ), + ]; + for (source, expected) in cases { + expect_rejected(source, expected); + } +} + +/// No diagnostic may leak a Rust variant name. +/// +/// A cheap guard over the whole set: these are the spellings `{:?}` produced, +/// and none of them is a thing anyone can type in BASIC. +#[test] +fn test_diagnostics_never_show_rust_variant_names() { + let sources = [ + "FOR I = 1 2 3\nNEXT\n", + "X = )\n", + "IF THEN\n", + "GOTO +\n", + "X = 1 <> \n", + "SELECT CASE\n", + ]; + for source in sources { + let Err(e) = compile_only(source) else { + continue; + }; + for leaked in [ + "EndSelect", + "LParen", + "RParen", + "Integer(", + "Ident(", + "Newline", + "ElseIf", + ] { + assert!( + !e.stderr.contains(leaked), + "{source:?} leaked the Rust name {leaked:?}: {}", + e.stderr + ); + } + } +} + +/// An unclosed block names the construct and the line that opened it. +/// +/// None of the seven hand-written body loops checked for end of file. They +/// stopped only because an unrecognised token became "Unexpected token: Eof", +/// so every one of these reported that against the last line of the file and +/// named nothing at all -- the reader was told where the parser gave up rather +/// than where the mistake was. +#[test] +fn test_unclosed_blocks_name_their_opener() { + let cases = [ + ( + "PRINT 1\nFOR I = 1 TO 10\nPRINT I\n", + "FOR is missing its NEXT", + ), + ( + "PRINT 1\nSUB Foo\nPRINT 1\n", + "SUB 'FOO' is missing its END SUB", + ), + ( + "FUNCTION Bar(X)\nBar = X\n", + "FUNCTION 'BAR' is missing its END FUNCTION", + ), + ("WHILE X < 3\nX = X + 1\n", "WHILE is missing its WEND"), + ("DO\nX = X + 1\n", "DO is missing its LOOP"), + ("IF X = 1 THEN\nPRINT 1\n", "IF is missing its END IF"), + ( + "SELECT CASE X\nCASE 1\nPRINT 1\n", + "SELECT CASE is missing its END SELECT", + ), + ]; + for (source, expected) in cases { + expect_rejected(source, expected); + } +} + +/// The opener's line is the one reported, not end of file. +#[test] +fn test_unclosed_block_reports_the_opening_line() { + let e = compile_only("PRINT 1\nPRINT 2\nFOR I = 1 TO 10\nPRINT I\nPRINT I\n") + .expect_err("an unclosed FOR must be refused"); + assert!( + e.contains(":3: error:"), + "should blame the FOR on line 3, not end of file: {}", + e.stderr + ); +} + +/// A block closed by the wrong terminator says which one it wanted. +#[test] +fn test_mismatched_block_terminator_names_both() { + expect_rejected( + "FOR I = 1 TO 3\nPRINT I\nWEND\n", + "FOR needs NEXT to close it, but WEND came first", + ); + expect_rejected( + "WHILE X < 3\nX = X + 1\nNEXT\n", + "WHILE needs WEND to close it, but NEXT came first", + ); + expect_rejected( + "DO\nX = X + 1\nEND SUB\n", + "DO needs LOOP to close it, but END SUB came first", + ); +} + +/// Pathological nesting is diagnosed, not fatal. +/// +/// The expression parser recursed without a bound, so 50,000 nested parens -- +/// or 50,000 unary minuses, which descend through the same path -- aborted the +/// process with "fatal runtime error: stack overflow" and exit code 134. A +/// compiler may refuse its input; it may not die on it, and 134 is outside the +/// contract `is_clean_rejection` describes. +#[test] +fn test_deeply_nested_expressions_are_diagnosed_not_fatal() { + let n = 50_000; + expect_rejected( + &format!("X = {}1{}\n", "(".repeat(n), ")".repeat(n)), + "nesting is too deep", + ); + expect_rejected(&format!("X = {}1\n", "-".repeat(n)), "nesting is too deep"); + expect_rejected( + &format!("X = {}1\n", "NOT ".repeat(n)), + "nesting is too deep", + ); +} + +/// Deeply nested blocks descend through the same statement path. +#[test] +fn test_deeply_nested_blocks_are_diagnosed_not_fatal() { + let n = 50_000; + let source = format!( + "{}PRINT 1\n{}", + "IF 1 = 1 THEN\n".repeat(n), + "END IF\n".repeat(n) + ); + expect_rejected(&source, "nesting is too deep"); +} + +/// A long run of statement separators must not consume stack either. +/// +/// `parse_statement_kind` recursed once per separator to skip it. That is a +/// tail call, so a release build optimized it away and only a debug build +/// overflowed on 200,000 colons -- which is the worst way to hold a bug, since +/// CI runs `cargo test --release`. Skipping them in a loop costs no stack in +/// any profile. A separator run is legal, so this is accepted, not diagnosed. +#[test] +fn test_a_long_run_of_separators_costs_no_stack() { + let source = format!("X = 1 {}\nPRINT X\n", ":".repeat(200_000)); + let run = compile_and_run(&source).expect("a run of separators is legal, if pointless"); + assert_eq!(run.trim(), "1"); +} + +/// A line number too large to represent is an error, not a silent zero. +/// +/// The lexer parsed it with `unwrap_or(0)`, so `99999999999 PRINT "hi"` +/// compiled as a definition of label 0 -- and any GOTO written to reach it +/// failed separately, because past LONG range the same digits lex as a Double. +/// Every other numeric form in the lexer already refuses to guess. +#[test] +fn test_line_number_out_of_range_is_diagnosed() { + expect_rejected("99999999999 PRINT \"hi\"\n", "line number"); + // The largest representable one still works. + compile_only("4294967295 PRINT \"ok\"\n").expect("u32::MAX is a valid line number"); +} + +/// A DO loop tests its condition at one end or the other, never both. +/// +/// The two conditions used to be merged with `condition.or(end_condition)`, so +/// the one on the LOOP was silently discarded: the loop below ran three times +/// and printed 3, with `UNTIL I > 100` having no effect whatever. Writing both +/// is a mistake about which test is being applied, and saying so beats picking +/// one. +#[test] +fn test_do_loop_rejects_a_condition_at_both_ends() { + expect_rejected( + "I = 0\nDO WHILE I < 3\nI = I + 1\nLOOP UNTIL I > 100\n", + "only one end", + ); + expect_rejected( + "I = 0\nDO UNTIL I > 3\nI = I + 1\nLOOP WHILE I < 100\n", + "only one end", + ); +} + +/// Each single-ended form still compiles, so the check above is not simply +/// rejecting every DO loop. +#[test] +fn test_do_loop_single_condition_forms_still_compile() { + for source in [ + "I = 0\nDO WHILE I < 3\nI = I + 1\nLOOP\n", + "I = 0\nDO UNTIL I > 3\nI = I + 1\nLOOP\n", + "I = 0\nDO\nI = I + 1\nLOOP WHILE I < 3\n", + "I = 0\nDO\nI = I + 1\nLOOP UNTIL I > 3\n", + "I = 0\nDO\nI = I + 1\nIF I > 3 THEN EXIT DO\nLOOP\n", + ] { + compile_only(source).unwrap_or_else(|e| { + panic!("{:?} should compile, but: {}", source, e.stderr); + }); + } +} + /// The random-access statement names are recognised only in statement /// position, so a program may still use them for its own variables and /// procedures -- which GW-BASIC would not allow, but costs nothing to keep. @@ -951,3 +1267,331 @@ fn test_random_access_keywords_are_not_reserved() { }); } } + +/// The front end must never panic, whatever it is fed. +/// +/// A compiler may reject its input; it may not die on it. The stack overflow on +/// deeply nested expressions was exactly this class of bug and survived 314 +/// tests, because every one of them fed the compiler a program someone had +/// thought about. This feeds it token soup instead: every result is acceptable +/// except a panic or an abort, which `is_clean_rejection` distinguishes by exit +/// code (1 = diagnosed, 101 = Rust panic, 134 = abort). +#[test] +fn test_front_end_never_panics_on_token_soup() { + // Deterministic, so a failure is reproducible from the seed alone. + let pieces = [ + "PRINT", + "IF", + "THEN", + "ELSE", + "END", + "SUB", + "FUNCTION", + "FOR", + "NEXT", + "WHILE", + "WEND", + "DO", + "LOOP", + "UNTIL", + "SELECT", + "CASE", + "DIM", + "TYPE", + "AS", + "GOTO", + "GOSUB", + "RETURN", + "MID$", + "LEN", + "(", + ")", + ",", + ";", + ":", + "#", + ".", + "=", + "<>", + "+", + "-", + "*", + "/", + "^", + "\"s\"", + "1", + "1.5", + "&HFF", + "A", + "B$", + "C%", + "\n", + "REM x", + "'c", + "LINE INPUT", + "SWAP", + "CONST", + "EXIT", + "OPTION", + "BASE", + "REDIM", + "PRESERVE", + "DATA", + "READ", + "RESTORE", + "OPEN", + "FIELD", + "LSET", + "GET", + "PUT", + "LOCK", + "STEP", + "TO", + "NOT", + "AND", + "OR", + "XOR", + "MOD", + ]; + // xorshift, so the corpus is fixed without pulling in a rng crate. + let mut state: u64 = 0x9E3779B97F4A7C15; + let mut next = move || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state + }; + + for case in 0..300 { + let len = 1 + (next() % 40) as usize; + let mut source = String::new(); + for _ in 0..len { + source.push_str(pieces[(next() % pieces.len() as u64) as usize]); + source.push(' '); + } + source.push('\n'); + + if let Err(e) = compile_only(&source) { + assert!( + e.is_clean_rejection(), + "case {case} must be diagnosed, not crash; exit={:?}\nsource: {source:?}\nstderr: {}", + e.exit_code, + e.stderr + ); + } + } +} + +/// A number too large to represent is an error, not infinity. +/// +/// `parse::()` returns `inf` for an overflowing literal rather than Err, +/// so the "malformed number" arm never fired and `1e400` compiled to a silent +/// infinity. Every other numeric form in this lexer refuses to guess. +#[test] +fn test_numeric_overflow_is_diagnosed() { + expect_rejected("PRINT 1e400\n", "too large"); + expect_rejected("X# = 1.5D400\n", "too large"); + // The largest representable double still works. + compile_only("PRINT 1.7976931348623157E+308\n").expect("near the maximum is fine"); +} + +/// A failed block header must not also blame its own terminator. +/// +/// `FOR I = 1 2 3` fails, and the NEXT that follows is then orphaned -- so the +/// reader was told "NEXT without matching FOR" about a FOR sitting one line +/// above. Once anything has gone wrong, stray terminators say nothing useful. +#[test] +fn test_a_failed_block_header_does_not_cascade() { + let e = compile_only("FOR I = 1 2 3\nPRINT I\nNEXT\n").expect_err("must be refused"); + assert!( + e.contains("expected TO"), + "the real error must survive: {}", + e.stderr + ); + assert!( + !e.contains("without matching"), + "the orphaned NEXT must not be reported: {}", + e.stderr + ); + assert!(e.contains("1 error"), "exactly one: {}", e.stderr); +} + +/// A stray terminator in an otherwise clean program is still reported. +#[test] +fn test_cascade_suppression_only_applies_after_an_error() { + expect_rejected("PRINT 1\nNEXT\n", "NEXT without matching FOR"); +} + +/// `RETURN` with no `GOSUB` anywhere is a compile error, not a linker error. +/// +/// codegen only defines the GOSUB return stack when it has seen a GOSUB, so a +/// lone RETURN emitted a reference to `_gosub_sp` that nothing defined and the +/// user was shown `ld: undefined reference to _gosub_sp`. Turning that into a +/// diagnostic is the whole reason sema exists. +#[test] +fn test_return_without_gosub_is_diagnosed() { + expect_rejected("PRINT \"x\"\nRETURN\n", "RETURN"); + expect_rejected("IF 1 = 1 THEN\nRETURN\nEND IF\n", "RETURN"); + // A program that does use GOSUB is unaffected. + compile_only("GOSUB 100\nEND\n100 PRINT 1\nRETURN\n").expect("GOSUB/RETURN pairs compile"); +} + +/// SWAP's type check must look at what the operands actually are, not at the +/// suffix of the variable they hang off. +/// +/// It compared `a.name.ends_with('$')`, which for `P.N` reads *P* -- a record, +/// carrying no suffix. So swapping a string field with a numeric one passed the +/// check, and codegen then read a string into rax/rdx and stored it into an +/// INTEGER slot: SIGSEGV, exit 139. +#[test] +fn test_swap_of_mismatched_record_fields_is_diagnosed() { + let ty = "TYPE R\n N AS STRING * 4\n V AS INTEGER\nEND TYPE\nDIM P AS R\n"; + expect_rejected(&format!("{ty}SWAP P.N, P.V\n"), "same type"); + expect_rejected(&format!("{ty}SWAP P.V, P.N\n"), "same type"); + // Matching fields still swap. + compile_only("TYPE R\n A AS INTEGER\n B AS INTEGER\nEND TYPE\nDIM P AS R\nSWAP P.A, P.B\n") + .expect("two numeric fields are a legal SWAP"); +} + +/// Out-of-range arguments to the string builtins are refused. +/// +/// Each of these returned something plausible instead. The negative-length +/// cases were the worst: `_rt_left` compares the count against the length +/// unsigned, so -1 read as enormous, clamped to the length, and returned the +/// *whole string*. `MID$`'s negative count is worse still -- it is the +/// compiler's own sentinel for the two-argument form, so a program writing one +/// explicitly got "the rest of the string" from a value GW-BASIC rejects. +#[test] +fn test_string_builtin_arguments_are_range_checked() { + for (source, what) in [ + ("PRINT LEFT$(\"abc\", -1)\n", "LEFT$ negative count"), + ("PRINT RIGHT$(\"abc\", -1)\n", "RIGHT$ negative count"), + ("PRINT MID$(\"abc\", 0, 2)\n", "MID$ zero start"), + ("PRINT MID$(\"abc\", -1, 2)\n", "MID$ negative start"), + ("PRINT MID$(\"abc\", 1, -1)\n", "MID$ negative count"), + ("PRINT ASC(\"\")\n", "ASC of the empty string"), + ] { + let run = crate::common::compile_and_run_raw(source, "").expect("should compile"); + assert_eq!(run.exit_code, Some(1), "{what}: stderr={}", run.stderr); + assert!( + run.stderr.contains("Illegal function call"), + "{what}: stderr={}", + run.stderr + ); + } +} + +/// The legal forms of the same calls keep working, including MID$ with the +/// length omitted -- which is what the negative sentinel exists for. +#[test] +fn test_string_builtin_legal_arguments_still_work() { + let output = compile_and_run( + r#" +PRINT LEFT$("abcdef", 3) +PRINT LEFT$("abc", 0) +PRINT LEFT$("abc", 99) +PRINT RIGHT$("abcdef", 2) +PRINT MID$("abcdef", 3) +PRINT MID$("abcdef", 3, 2) +PRINT MID$("abc", 4) +PRINT ASC("A") +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["abc", "", "abc", "ef", "cdef", "cd", "", "65"]); +} + +/// A negative base with a fractional exponent has no real result. It used to +/// print `-nan`. +#[test] +fn test_fractional_power_of_a_negative_is_refused() { + let run = + crate::common::compile_and_run_raw("A = -8\nPRINT A ^ 0.5\n", "").expect("should compile"); + assert_eq!(run.exit_code, Some(1), "stderr: {}", run.stderr); + assert!( + run.stderr.contains("Illegal function call"), + "stderr: {}", + run.stderr + ); +} + +/// Integral exponents of a negative base are fine, and so is everything else +/// that has a real answer. +#[test] +fn test_powers_that_have_real_answers_still_work() { + let output = compile_and_run( + r#" +A = -2 +PRINT A ^ 3 +PRINT A ^ 2 +PRINT 4 ^ 0.5 +PRINT 2 ^ -2 +PRINT 0 ^ 0 +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["-8", "4", "2", "0.25", "1"]); +} + +/// CHR$, SPACE$ and STRING$ refuse counts and codes they cannot represent. +/// +/// CHR$ kept only the low byte, so CHR$(256) was CHR$(0) and CHR$(-1) was +/// CHR$(255). SPACE$ and STRING$ clamped a negative count to zero and returned +/// the empty string. GW-BASIC calls all four an illegal function call. +#[test] +fn test_character_and_count_arguments_are_range_checked() { + for (source, what) in [ + ("PRINT CHR$(256)\n", "CHR$ above 255"), + ("PRINT CHR$(-1)\n", "CHR$ below 0"), + ("PRINT SPACE$(-1)\n", "SPACE$ negative"), + ("PRINT STRING$(-1, \"x\")\n", "STRING$ negative"), + ] { + let run = crate::common::compile_and_run_raw(source, "").expect("should compile"); + assert_eq!(run.exit_code, Some(1), "{what}: stderr={}", run.stderr); + assert!( + run.stderr.contains("Illegal function call"), + "{what}: stderr={}", + run.stderr + ); + } +} + +/// The whole legal range still works, both ends included. +#[test] +fn test_character_and_count_legal_arguments() { + let output = compile_and_run( + r#" +PRINT ASC(CHR$(0)) +PRINT ASC(CHR$(255)) +PRINT ASC(CHR$(65)) +PRINT "["; SPACE$(0); "]" +PRINT "["; SPACE$(3); "]" +PRINT "["; STRING$(0, "x"); "]" +PRINT "["; STRING$(3, "x"); "]" +PRINT HEX$(255) +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + &["0", "255", "65", "[]", "[ ]", "[]", "[xxx]", "FF"] + ); +} + +/// A `DIM ... AS` type must agree with any suffix on the name. +/// +/// The identical check has always existed for a FUNCTION's declared result +/// type, and LANGREF states the rule -- but DIM accepted the contradiction +/// silently, leaving no way to tell which of the two the program meant. +#[test] +fn test_dim_suffix_must_agree_with_its_as_clause() { + expect_rejected("DIM X$ AS INTEGER\n", "different type"); + expect_rejected("DIM X% AS DOUBLE\nX% = 1\n", "different type"); + expect_rejected("DIM A%(3) AS STRING * 4\n", "different type"); + // Agreeing, and unsuffixed, are both fine. + compile_only("DIM X% AS INTEGER\nDIM Y AS DOUBLE\nDIM S$ AS STRING * 4\nDIM N AS LONG\n") + .expect("a suffix that agrees, or none at all, is legal"); +} diff --git a/tests/file_io/mod.rs b/tests/file_io/mod.rs index a13aa26..0875d18 100644 --- a/tests/file_io/mod.rs +++ b/tests/file_io/mod.rs @@ -616,3 +616,91 @@ CLOSE #1 assert_eq!(output.trim(), "[new]"); assert_eq!(fs::read(tmp.path().join("s.dat")).unwrap(), b"original"); } + +/// Writing to or reading from a file number that was never OPENed is a +/// diagnosed abort, not a crash. +/// +/// `_rt_file_print_string`, `_rt_file_print_char`, `_rt_file_print_newline` and +/// `_rt_file_line_input` all loaded the handle-table slot and passed it +/// straight to fprintf or fgets, so a NULL took the process down with SIGSEGV +/// and exit 139 -- outside anything the harness can interpret. +/// `_rt_file_getc` and `_rt_file_eof` had guarded all along; these four had not. +#[test] +fn test_unopened_file_number_is_diagnosed_not_fatal() { + for source in [ + "PRINT #3, \"x\"\n", + "PRINT #3, 5\n", + "PRINT #3,\n", + "LINE INPUT #3, A$\n", + ] { + let run = crate::common::compile_and_run_raw(source, "").expect("should compile"); + assert_eq!( + run.exit_code, + Some(1), + "{source:?} should abort cleanly, not crash; stderr: {}", + run.stderr + ); + assert!( + run.stderr.contains("Bad file number"), + "{source:?} should say why: {}", + run.stderr + ); + } +} + +/// A failed OPEN is reported rather than stored. +/// +/// The fopen/CreateFileA result was written into the handle table whatever it +/// was, so opening a file that is not there "succeeded", `EOF()` answered -1, +/// and the mistake surfaced later as a crash or as silence. +#[test] +fn test_open_of_a_missing_file_is_diagnosed() { + let run = compile_and_run_raw( + "OPEN \"definitely-not-here.txt\" FOR INPUT AS #1\nPRINT \"opened\"\n", + "", + ) + .expect("should compile"); + assert_eq!(run.exit_code, Some(1), "stderr: {}", run.stderr); + assert!(run.stderr.contains("File not found"), "{}", run.stderr); + assert!( + !run.stdout.contains("opened"), + "the OPEN must not appear to succeed: {}", + run.stdout + ); +} + +/// Re-using a file number that is still open is refused, not silently rebound. +#[test] +fn test_open_on_an_already_open_number_is_diagnosed() { + let run = compile_and_run_raw( + "OPEN \"a.txt\" FOR OUTPUT AS #1\nOPEN \"b.txt\" FOR OUTPUT AS #1\n", + "", + ) + .expect("should compile"); + assert_eq!(run.exit_code, Some(1), "stderr: {}", run.stderr); + assert!( + run.stderr.contains("File already open"), + "stderr: {}", + run.stderr + ); +} + +/// Reading past the end is an error, not an endless supply of empty strings. +/// +/// A loop that forgets its `EOF()` test used to run on quietly rather than say +/// what was wrong. +#[test] +fn test_reading_past_the_end_is_diagnosed() { + let run = compile_and_run_raw( + "OPEN \"e.txt\" FOR OUTPUT AS #1\nPRINT #1, \"one\"\nCLOSE #1\n\ + OPEN \"e.txt\" FOR INPUT AS #1\nLINE INPUT #1, A$\nLINE INPUT #1, B$\n", + "", + ) + .expect("should compile"); + assert_eq!(run.exit_code, Some(1), "stderr: {}", run.stderr); + assert!( + run.stderr.contains("Input past end of file"), + "stderr: {}", + run.stderr + ); +} diff --git a/tests/input/mod.rs b/tests/input/mod.rs index d4cbd1a..7400ec4 100644 --- a/tests/input/mod.rs +++ b/tests/input/mod.rs @@ -62,5 +62,76 @@ PRINT A(3) ) .unwrap(); let lines: Vec<&str> = output.trim().lines().collect(); - assert_eq!(lines, vec!["10", "text", "77"]); + // The promptless INPUT prints `? `, which lands on the first output line. + assert_eq!(lines, vec!["? 10", "text", "77"]); +} + +/// The separator after an INPUT prompt decides whether a question mark follows. +/// +/// GW-BASIC prints `p? ` for `INPUT "p"; A` and `p` alone for `INPUT "p", A`, +/// and a promptless `INPUT A` prints a bare `? `. The parser accepted either +/// separator and discarded which, and nothing anywhere emitted a question mark, +/// so all three forms printed the prompt verbatim -- while LANGREF claimed +/// otherwise. +#[test] +fn test_input_prompt_separators() { + let semi = compile_and_run_with_stdin("INPUT \"Name\"; A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!( + semi.trim_end(), + "Name? Bob", + "a semicolon adds the question mark" + ); + + let comma = compile_and_run_with_stdin("INPUT \"Name\", A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!(comma.trim_end(), "NameBob", "a comma suppresses it"); + + let bare = compile_and_run_with_stdin("INPUT A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!(bare.trim_end(), "? Bob", "no prompt still asks"); +} + +/// LINE INPUT never adds a question mark, whichever separator is used. +/// +/// These compare `trim_end()` rather than the raw output: the prompt shares a +/// line with the echoed input, and the line ending that follows is LF on +/// System V and CRLF on Windows. Asserting the raw string passed on Linux and +/// failed the Windows job. +#[test] +fn test_line_input_never_adds_a_question_mark() { + let semi = compile_and_run_with_stdin("LINE INPUT \"N: \"; A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!(semi.trim_end(), "N: Bob"); + + let bare = compile_and_run_with_stdin("LINE INPUT A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!( + bare.trim_end(), + "Bob", + "and prompts for nothing when none is given" + ); +} + +/// A leading `;` is accepted on both statements. +/// +/// It suppresses the newline echoed when the operator presses Return, which +/// here is the terminal's echo rather than anything the program prints -- so it +/// parses and has no effect. Refusing it would turn away a program for asking +/// about a difference this implementation cannot observe. +#[test] +fn test_input_leading_semicolon_is_accepted() { + let out = compile_and_run_with_stdin("INPUT ; \"Name\"; A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!(out.trim_end(), "Name? Bob"); + + let line = compile_and_run_with_stdin("LINE INPUT ; \"N: \"; A$\nPRINT A$\n", "Bob\n").unwrap(); + assert_eq!(line.trim_end(), "N: Bob"); +} + +/// INPUT into a narrow scalar must narrow it, like every other store. +#[test] +fn test_input_into_narrow_scalars() { + let out = compile_and_run_with_stdin("INPUT A%\nPRINT A%\n", "5\n").unwrap(); + assert_eq!(out.trim(), "? 5"); + + let single = compile_and_run_with_stdin("INPUT A!\nPRINT A!\n", "6\n").unwrap(); + assert_eq!(single.trim(), "? 6"); + + let long = compile_and_run_with_stdin("INPUT A&\nPRINT A&\n", "70000\n").unwrap(); + assert_eq!(long.trim(), "? 70000"); } diff --git a/tests/procedures/mod.rs b/tests/procedures/mod.rs index bb6f2d3..920c4a2 100644 --- a/tests/procedures/mod.rs +++ b/tests/procedures/mod.rs @@ -396,3 +396,37 @@ fn test_function_return_type_without_as() { let output = compile_and_run("FUNCTION K(X)\nK = X / 2\nEND FUNCTION\nPRINT K(7)\n").unwrap(); assert_eq!(output.trim(), "3.5"); } + +/// `CALL` is the explicit form of a procedure call. +/// +/// It was not recognised at all, so `CALL MySub(1)` parsed as a paren-less call +/// to a subroutine named CALL whose argument was `MySub(1)`, and the diagnostic +/// talked about MySub having no value rather than about CALL. +#[test] +fn test_call_statement() { + let output = compile_and_run( + r#" +SUB Greet(N) + PRINT "n="; N +END SUB +SUB Plain + PRINT "plain" +END SUB +CALL Greet(7) +CALL Plain +Greet 8 +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["n=7", "plain", "n=8"]); +} + +/// CALL is recognised only in statement position before a name, so a program +/// may still use it as a variable -- the same rule the random-access statement +/// names follow. +#[test] +fn test_call_is_not_reserved() { + let output = compile_and_run("CALL = 5\nPRINT CALL\n").unwrap(); + assert_eq!(output.trim(), "5"); +} diff --git a/tests/strings/mod.rs b/tests/strings/mod.rs index 8be306f..7cd9ae9 100644 --- a/tests/strings/mod.rs +++ b/tests/strings/mod.rs @@ -407,3 +407,48 @@ PRINT STR$(X!) ] ); } + +/// A start position past the end of the string finds nothing. +/// +/// The runtime subtracted `start - 1` from the remaining length without +/// checking, so a start beyond the string made that length go negative -- +/// which, unsigned, is enormous. The "is there room for the needle" test then +/// passed and memcmp read past the end of the buffer, returning whatever +/// position the garbage happened to match at: 253 and 261 on two runs of the +/// same program. +#[test] +fn test_instr_start_beyond_the_string() { + let output = compile_and_run( + r#" +PRINT INSTR(10, "abc", "b") +PRINT INSTR(4, "abc", "b") +PRINT INSTR(3, "abc", "c") +PRINT INSTR(1, "abc", "a") +S = 99 +PRINT INSTR(S, "abc", "b") +PRINT INSTR(2, "", "x") +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["0", "0", "3", "1", "0", "0"]); +} + +/// A start position below 1 is an illegal argument, as it is in GW-BASIC. +/// +/// It used to move the search pointer *backwards* out of the buffer. +#[test] +fn test_instr_start_below_one_is_refused() { + for source in [ + "PRINT INSTR(0, \"abc\", \"b\")\n", + "PRINT INSTR(-1, \"abc\", \"b\")\n", + ] { + let run = crate::common::compile_and_run_raw(source, "").expect("should compile"); + assert_eq!(run.exit_code, Some(1), "stderr: {}", run.stderr); + assert!( + run.stderr.contains("Illegal function call"), + "stderr: {}", + run.stderr + ); + } +} diff --git a/tests/types/mod.rs b/tests/types/mod.rs index 8e607b2..e715ccb 100644 --- a/tests/types/mod.rs +++ b/tests/types/mod.rs @@ -209,7 +209,10 @@ fn test_type_records() { let lines: Vec<&str> = output.trim().lines().collect(); assert_eq!(lines[0], "0", "a record starts zeroed"); assert_eq!(lines[1], "71000002.51.25", "each field keeps its own type"); - assert_eq!(lines[2], "hello5"); + // A `STRING * 20` field holds twenty characters whatever it is given, so + // "hello" is space-padded and LEN is 20. This asserted "hello5" while + // assignment stored the source verbatim and the declared width did nothing. + assert_eq!(lines[2], "hello 20"); } /// A TYPE may contain another TYPE, to any depth. @@ -330,7 +333,8 @@ fn test_as_typed_variables() { ) .unwrap(); let lines: Vec<&str> = output.trim().lines().collect(); - assert_eq!(lines, vec!["42hi", "84"]); + // `S` is `STRING * 10`, so "hi" is padded to ten characters. + assert_eq!(lines, vec!["42hi ", "84"]); } /// A typed parameter, and a FUNCTION with a declared result type. @@ -379,3 +383,59 @@ fn test_record_argument_from_nested_field() { .unwrap(); assert_eq!(output.trim(), "4"); } + +/// A `STRING * n` field always holds exactly n characters. +/// +/// Assignment stored the source verbatim, so the declared width did nothing at +/// all: a longer value was kept whole and a shorter one stayed short. That is +/// the entire purpose of a fixed-length string, and it is what makes a record +/// laid out over a random-access file line up. +#[test] +fn test_fixed_length_string_pads_and_truncates() { + let output = compile_and_run( + r#" +TYPE R + N AS STRING * 5 +END TYPE +DIM P AS R +P.N = "abcdefgh" +PRINT "["; P.N; "]" +PRINT LEN(P.N) +P.N = "ab" +PRINT "["; P.N; "]" +PRINT LEN(P.N) +P.N = "" +PRINT "["; P.N; "]" +PRINT LEN(P.N) +P.N = "exact" +PRINT "["; P.N; "]" +PRINT LEN(P.N) +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!( + lines, + &[ + "[abcde]", "5", "[ab ]", "5", "[ ]", "5", "[exact]", "5" + ] + ); +} + +/// The same for a standalone `DIM ... AS STRING * n`, and it compares equal to +/// the padded text. +#[test] +fn test_fixed_length_string_variable() { + let output = compile_and_run( + r#" +DIM S AS STRING * 4 +S = "xy" +PRINT "["; S; "]" +PRINT S = "xy " +PRINT LEN(S) +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["[xy ]", "-1", "4"]); +} diff --git a/tests/variables/mod.rs b/tests/variables/mod.rs index 14a87e3..8fb8e6b 100644 --- a/tests/variables/mod.rs +++ b/tests/variables/mod.rs @@ -143,3 +143,124 @@ fn test_let_accepts_every_assignment_form() { let lines: Vec<&str> = output.trim().lines().collect(); assert_eq!(lines, vec!["5734", "HEllo"]); } + +/// A trailing `_` continues a statement on the next line. +#[test] +fn test_line_continuation() { + let output = compile_and_run( + r#" +X = 1 + _ + 2 + _ + 3 +PRINT X +Y$ = "a" + _ + "b" +PRINT Y$ +IF X = 6 AND _ + Y$ = "ab" THEN PRINT "both" +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["6", "ab", "both"]); +} + +/// Storing into a narrow scalar must narrow, whichever statement does it. +/// +/// `gen_store_lvalue` -- the path READ, INPUT and SWAP all share -- wrote the +/// value as an 8-byte double whatever the variable's declared type was. Reading +/// an INTEGER slot back through `movsx WORD PTR` then gave the low 16 bits of a +/// double's bit pattern, which for any small value is 0: +/// +/// DATA 7 +/// READ A% +/// PRINT A% ' printed 0 +/// +/// Its own array-element path narrowed correctly twenty lines below, and plain +/// `A% = 7` goes through a different path entirely, which is why only these +/// three statements were affected and only for `%`, `&` and `!`. +#[test] +fn test_read_into_narrow_scalars() { + let output = compile_and_run( + r#" +DATA 7, 8, 9, 10 +READ A% +READ B& +READ C! +READ D# +PRINT A% +PRINT B& +PRINT C! +PRINT D# +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["7", "8", "9", "10"]); +} + +/// The same through a variable declared with `AS` rather than a suffix. +#[test] +fn test_read_into_a_declared_scalar() { + let output = compile_and_run( + r#" +DIM N AS INTEGER +DIM L AS LONG +DATA 11, 12 +READ N +READ L +PRINT N +PRINT L +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["11", "12"]); +} + +/// SWAP exchanges values; it used to destroy both of them. +/// +/// `SWAP A%, B%` left A% and B% at 0 -- not swapped, erased -- because both +/// stores went through the same un-narrowed path. +#[test] +fn test_swap_across_every_scalar_type() { + let output = compile_and_run( + r#" +A% = 1 : B% = 2 +SWAP A%, B% +PRINT A%; B% +C& = 3 : D& = 4 +SWAP C&, D& +PRINT C&; D& +E! = 5 : F! = 6 +SWAP E!, F! +PRINT E!; F! +G# = 7 : H# = 8 +SWAP G#, H# +PRINT G#; H# +I$ = "x" : J$ = "y" +SWAP I$, J$ +PRINT I$; J$ +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["21", "43", "65", "87", "yx"]); +} + +/// SWAP between different numeric types converts, as an assignment would. +#[test] +fn test_swap_between_numeric_types() { + let output = compile_and_run( + r#" +A% = 1 +B# = 2.5 +SWAP A%, B# +PRINT A% +PRINT B# +"#, + ) + .unwrap(); + let lines: Vec<&str> = output.trim().lines().collect(); + assert_eq!(lines, &["2", "1"], "A% takes CINT(2.5), B# takes 1"); +}