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>
Each module-level statement gets a label before its marker store and one after its code, and two .quad tables collect them. RESUME jumps through the first, RESUME NEXT through the second. Two tables rather than one plus arithmetic, because "the statement after this one" is a position in the emitted code and not the next index. That is what makes every awkward case fall out for free: after the last statement of a FOR body comes the increment and the back-jump, so RESUME NEXT continues the loop; after the last statement of an IF branch comes that branch's exit jump; after the last statement of the program comes main's epilogue. The nested test -- an IF inside a FOR inside a WHILE -- passed first try, which is the claim. The marker store sits *after* its label so a resumed statement re-establishes its own index, and `_err_resume` is a snapshot taken at dispatch: the handler is ordinary module-level code, so its own statements overwrite `_err_stmt` the moment it starts running. ERL needed the same snapshot in the previous commit and for the same reason. gen_stmt became a wrapper. Several of its arms return early and would have skipped a closing label written at the bottom -- and the closing label is the one RESUME NEXT needs. A statement inside a procedure gets no resume point, and `_err_depth` counts the nesting so the trap can mark such an error unresumable. A counter rather than a saved slot because the trap resets it, so the unwind skipping every pending decrement costs nothing. `X = F(1) / D` still resumes correctly: the depth is back to zero by the time the division runs, which is the case that rules out simply setting a sentinel on entry. RESUME's own two failures are untrappable, so `_rt_error` splits: `_rt_fatal` is today's body, reached directly by those trampolines. A handler that trapped its own failing RESUME would be re-entered by it forever. Sema refuses RESUME with no ON ERROR anywhere in the program -- modelled on `check_return_has_a_gosub`, because otherwise the failure is a link error against a table that a non-trapping program never emits -- as well as RESUME inside a procedure and a target that is not at module level. examples/errtrap.bas shows all three forms and is deliberately cwd-independent: an earlier draft fell back to a file that exists only in examples/, which from the repository root would have failed again and looped through the handler forever. It opens a file that is not there, and creates nothing. Verified under both runtimes; the Win64 model prints the example's four lines identically. The Windows probe step gains three RESUME programs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its entry in the UNSUPPORTED table still read "error trapping is not implemented yet", which stopped being true two commits ago. ERROR stays in the table for a different reason: it is a statement and never a name, so that `PRINT ERROR` and `X = ERROR + 1` say so rather than quietly becoming a variable that reads as zero. The note now says that instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`PRINT "Guess"; Tries; "is"; Guess` wrote "Guess1is50". A listing's output ran together, and nothing in a column lined up, because a number here carried no spacing of its own. In GW-BASIC it carries two blanks: one where the minus sign would go if the number is not negative, and one after it. So `PRINT 1; 2; 3` is " 1 2 3 " and `PRINT 1; -2; 3` is " 1 -2 3 ", the minus taking the leading blank's place. Three smaller rules travel with it: a value below one drops its leading zero, an exponent is spelled D for a DOUBLE and E for a SINGLE rather than C's lowercase e, and a comma moves to the next 14-column print zone instead of emitting a literal tab whose width is whatever the terminal says. Where the blanks live matters. `_rt_fmt_double` keeps rendering bare digits, because WRITE and PRINT USING go through it and must not gain them: WRITE's output is meant to be read back by INPUT, and PRINT USING lays out its own. The sign position is added by `_rt_fmt_basic`, which STR$ also uses -- GW-BASIC defines STR$ as what PRINT writes less the trailing blank, which is why `MID$(STR$(N), 2)` is the idiom for stripping it and why leaving STR$ alone would have made that idiom eat a digit. The trailing blank belongs to PRINT alone, so PRINT gets its own pair of helpers and WRITE keeps the old ones. 182 tests moved. Most only ever cared what a program computed, so the harness now trims each line rather than the whole output once, and the spacing has explicit tests of its own instead of being implicit in every assertion. The rest were rewritten from the diffs the failures printed, one function at a time -- a global replace of "21" would have quietly corrupted tests that were passing. The megatest's own harness strips the sign position in `Check`, which is what its comment always claimed to do. Four expectations changed for a real reason rather than for spacing: .5 for 0.5, twice for a repeating double, and STR$(P) + STR$(Q) being "2 1" because joining two of them keeps the second one's sign position. Verified under both runtimes; the Win64 model prints all of it identically. guess.bas now reads "Guess 1 is 50 - too high". 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.