Conversation
`DEFINT A-Z` made `ON ERROR GOTO 100` compile clean again. The default-type
pass renames every unsuffixed name a DEF* range covers, so `ERROR` became
`ERROR%`, and `unsupported_reason` looks up the unsuffixed spelling and missed.
The statement then went back to being a computed GOTO on a variable that is
always zero -- the precise silent fall-through the UNSUPPORTED table was
written to prevent, restored by a feature added four commits after it.
Measured before the fix:
ON ERROR GOTO 100 / END / 100 END -> refused
DEFINT A-Z + the same -> compiled, ran, exit 0
DEFINT A-Z + PRINT ERR -> compiled, printed 0
`defaulted` already declined to rename a procedure or a builtin, for the same
reason in each case: the name does not denote a variable. A name this compiler
knows it does not provide belongs in that guard too, and now is.
The test covers every route the refusal is reached by -- a bare name in an
expression, a statement-position call, and ON ERROR -- under three different
DEF* types, because the hole was in the renaming rather than in any one name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A program whose line 110 failed said "in 4". Codegen kept one line number, `current_line`, and it is the lexer's physical source line -- while the `_line_NNN` labels a program branches to come from `StmtKind::Label(n)`. Two numbering systems, and every diagnostic quoted the one the programmer cannot see. LANGREF:1309 has claimed otherwise since it was written. Track the most recent BASIC line beside the physical one and prefer it. A program with no line numbers has none to prefer, so it keeps reporting the source line, which is the only number its author can act on and the style everything in examples/ is written in; a statement ahead of the first line number does the same. LANGREF now states which of the two you get. Reset per function, because main is rendered into a scratch buffer before the procedures and shares the field with them. This is a prerequisite for ERL rather than a tidy-up: ERL returns the BASIC line, so the number in the message and the number the handler reads have to be the same one. Verified across the four shapes that differ: a line-numbered listing (110), a failure inside a GOSUB'd subroutine (110), a SUB carrying its own numbers (900), and an unnumbered program (source line 2, unchanged). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The numbers are the interface error trapping is built on: a listing writes `IF ERR = 53 THEN` and means "file not found". Nothing here had numbers at all, so they come first, and `ERROR n` is the half of the feature that is fully meaningful before any handler exists -- it raises by number and aborts, exactly as GW-BASIC does when nothing is trapping. Both trees gain a table of (message, number) pairs rather than a third argument to `_rt_error`. That argument would have to be threaded through thirteen call sites per tree plus every trampoline codegen emits, and a helper here may take only four arguments because Win64 passes four in registers. A linear scan on the way to exit costs nothing. `ERROR` stays contextual rather than reserved: it is a statement only when something follows it to be the number, so a bare `ERROR` still reaches the UNSUPPORTED table and `ON ERROR GOTO` is still refused with a reason until it is written. The two wildcard-free matches in sema and the one in codegen each refused to compile until the new statement said what it was, which is what they are for. ERR and ERL are deliberately not in this commit. They would read 0 until trapping exists, and a program branching on `IF ERR = 53` would take the wrong arm with no diagnostic -- the silent wrong answer this compiler refuses elsewhere. They land with the trap that gives them values. Verified under both runtimes: the Win64 tree is only ever built in CI, so `ERROR 53` was also run through an ms_abi model of it, where it agrees with System V. That first disagreed only because the model's own sprintf stub could not handle `_rt_error`'s "%s in %lld"; the stub was wrong, not the runtime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends xbasic64’s GW-BASIC compatibility by introducing the ERROR n statement (raising runtime errors by GW-BASIC error number), adds a runtime mapping table for error numbers ↔ messages across both runtimes, and corrects runtime diagnostics to report the user-visible BASIC line number (falling back to physical source lines when appropriate).
Changes:
- Add parsing, codegen, runtime support, and tests for the
ERROR nstatement (including expression operands and unknown-code behavior). - Change runtime error reporting to prefer the most recently reached BASIC line number for diagnostics (with a fallback to physical source line numbers).
- Prevent
DEF*default-type rewriting from renaming unsupported GW-BASIC names (so refusal checks continue to work correctly).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/errors/mod.rs | Adds integration tests covering ERROR n, unknown numbers, line reporting behavior, and DEF* refusal regression cases. |
| src/sema.rs | Prevents DEF* rewriting of unsupported names; wires RaiseError into name-rewrite/expression-walk logic. |
| src/runtime/win64-native/error.s | Adds error-number table and _rt_error_num helper to raise errors by GW-BASIC number on Win64. |
| src/runtime/sysv/error.s | Adds the same error-number table and _rt_error_num helper for the SysV runtime. |
| src/parser.rs | Introduces StmtKind::RaiseError and contextual parsing for ERROR n. |
| src/codegen.rs | Implements ERROR n codegen, adds BASIC-vs-physical line tracking, and updates diagnostics to report the correct line number. |
| LANGREF.md | Documents BASIC line-number reporting and adds ERROR statement documentation with numbering table. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Both reported by Copilot against the ERROR statement, both real, and both
wider than reported.
`check_stmt` ends in a wildcard, so nothing forced an arm for a new statement
and the operands of four of them were never checked at all -- not just ERROR,
but LOCATE, COLOR and RANDOMIZE, added in the last two batches. Measured, the
compiler panicked with exit 101:
ERROR NoSuchFn(1) codegen.rs:6405 "sema checked the array is declared"
A$="x" : ERROR A$ codegen.rs:1330 "Cannot implicitly convert to/from String"
A$="x" : LOCATE A$,1 codegen.rs:1330 the same
A$="x" : RANDOMIZE A$ codegen.rs:1330 the same
The first message is the finding in one line: codegen asserts a check that
sema never performed. A builtin's arguments were always checked, which is why
`SQR(A$)` has always said so properly; a statement's operands were not. One
`check_numeric_operand` now does both halves for all four.
Separately, the token test that decides whether `ERROR` leads a statement was
written as a list of the tokens an expression may begin with, and had already
rotted: it was missing `NOT` and a string literal, so `ERROR NOT 0` -- an
ordinary GW-BASIC expression -- was refused as though the statement did not
exist, and `ERROR "boom"` blamed the wrong thing. Inverted to ask whether the
statement has ended, a set of four tokens that does not grow, so anything else
goes to the expression parser and gets a real diagnostic. A token added later
needs no edit.
`ERROR` alone, `PRINT ERROR` and `ON ERROR GOTO` all still reach the
UNSUPPORTED table, which is what holds the line until trapping is written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_rt_error` was documented "Returns: never" and meant it -- no `ret` at all, straight into exit(1). It is reached from thirteen places inside each runtime tree with live frames, some two deep, some holding a lock structure on the stack. Trapping abandons all of them, so this is a longjmp in all but name. The context is captured once, in main's prologue, by a helper in each runtime tree rather than by codegen: the callee-saved set differs between the ABIs -- Win64 counts rdi and rsi where System V does not -- and the tree that knows that should be the one that writes it down. The values saved are the ones the C runtime handed main, because main's prologue has not touched a callee-saved register yet, so putting them back leaves main's own `leave; ret` exactly as clean as it is without trapping. Saving only rsp and rbp would have left the abandoned helper's rbx and r12-r15 in place all the way back to the startup code -- an ABI violation of the kind that cost four CI rounds last week. The globals live in error.s's .bss in both trees, never in codegen's output. `_rt_error` is assembled into every program and its preamble reads them, so a codegen-emitted symbol would fail to link in every program that does not trap. Three things the mechanism needs that are not obvious: ERL is stored per statement, not taken from the line `_rt_error` is passed. Seven of the file helpers' error sites have no line to pass and say so in their own comments -- and those are exactly the errors ON ERROR is used to catch. It is also snapshotted at dispatch, because the handler is ordinary module-level code whose own statements overwrite the live line: measured, a handler on line 100 reported ERL 100 for an error on line 30. FOR-loop register promotion is switched off in a trapping program, and that is load-bearing rather than a concession. A promoted counter's register is gone after the unwind and its memory copy is written back only at the loop's exit label, so a handler would read a stale value; the promotion's stack saves are also the only thing in codegen that moves rsp across a statement boundary, which is what lets one captured rsp be correct everywhere. Two latent corruptions become reachable the moment a program can carry on, and are closed here. `gen_array_alloc` stored the new bounds before the allocation, so a caught Out of memory left them beside the old element pointer and every later subscript passed its bounds check into a stale block. `.Lrandom_nomem` left the freed record buffer in the table, which `_rt_random_prepare` tests for NULL to decide "Bad file mode", so a caught failure meant GET read freed memory. Neither is testable here -- Linux overcommit will not fail a 16 GB calloc -- so both are defensive, and both are wrong today regardless. A GOSUB in progress deliberately survives, since the GOSUB stack is independent of rsp; a handler can RETURN from a subroutine the error interrupted, as in GW-BASIC. A GOSUB *inside a procedure* cannot, so sema refuses it in a trapping program: its return address is a label in a frame the unwind discards. ON ERROR inside a procedure, a handler that is not at module level, and --unsafe are refused for related reasons, each with a note saying which. ERR and ERL join as zero-argument builtins now that they have values to report. ERROR stays in the UNSUPPORTED table: it is a statement, never a name. Verified under both runtimes. The Win64 tree is only ever built in CI, so the trap was also run through an ms_abi model of it -- including the deep case, where the error comes from inside `_rt_file_open` with six callee-saved registers pushed and eighty bytes allocated. All fourteen examples still run clean there. The Windows probe step gains five trapping programs, since unwinding out of hand-written assembly is precisely what the Linux job cannot exercise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.