Finish the language: every remaining verb group, and the defects that blocked them
Closes groups A, C, D, E, F, H and J of TODO.md section 4, plus RESTORE and RENUMBER, and closes section 6 -- all seventeen reference defects. Seven of those turned out to have been fixed or never ported and nobody had written it down; the audit records the evidence for each. Two of the seventeen were real. math_plus mutated its left operand when the operand was mutable, so A# + 1 could modify A#; it was gated on FOR/NEXT coverage because NEXT relied on the mutation, so tests/for_next.c came first and NEXT now writes the counter back itself. And the binary operators summed both numeric fields of their right operand, which no BASIC program can reach -- that one needed a test written against the value API. Writing the tests turned up eight defects nobody had listed. Seven are fixed: IF A = 2 THEN was a parse error; only == worked IF ... AND ... was a parse error, because a condition parsed as one relation IF A = 1 OR B = 2 THEN was silently always false, and so was IF A THEN EXIT before any NEXT restarted the program and exhausted the variable pool READ never found a DATA line above it, and swallowed the lines between PRINT 2 + 2 at the prompt was filed as program text instead of answering a short read discarded its bytes, so COPY produced empty files every verb taking an argument list said "peek() returned nil token!" on none The eighth is not fixed and cannot be quietly: a FOR whose step overshoots runs its body one extra time, and FOR I = 1 TO 1 runs it zero times. The two errors cancel for a step of 1, which is why neither was noticed. Correcting them changes the expected output of a checked-in acceptance file, and tests/reference/README.md forbids editing one to suit this interpreter. It is tests/for_semantics.c in AKBASIC_KNOWN_FAILING_TESTS, asserting the correct contract, and TODO.md items 19 and 20. Sprites are real libakgl actors with a renderfunc of their own, because akgl_actor_render draws every sprite square and an actor has no per-axis scale. Both are filed upstream. SPRSAV takes an image file, an SSHAPE handle or a 63-element integer array -- a string here cannot hold a zero byte. Verbs that need hardware that does not exist are refused by name with the reason rather than faked: SYS, HEADER, COLLECT, BACKUP, BOOT, FILTER, and DIRECTORY, which is refused for a missing libakstdlib wrapper filed upstream. 94 tests in the default build, 93 with SDL, 94 under ASan and UBSan, doxygen clean. The Go acceptance corpus stayed green throughout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
487
TODO.md
487
TODO.md
@@ -623,37 +623,58 @@ Order by what unblocks the most, and by what does not need `libakgl` to grow fir
|
||||
|
||||
| Group | Verbs | Blocked on |
|
||||
|---|---|---|
|
||||
| A. Structure | `DO`, `LOOP`, `WHILE`, `UNTIL`, `ON`, `BEGIN`, `BEND`, `END` | nothing — reuses `waitingForCommand` |
|
||||
| ~~A. Structure~~ | ~~`DO`, `LOOP`, `WHILE`, `UNTIL`, `ON`, `BEGIN`, `BEND`, `END`~~ | **done** — `src/runtime_structure.c`, `tests/structure_verbs.c` |
|
||||
| ~~B. Housekeeping~~ | ~~`NEW`, `CLR`, `CONT`, `SWAP`, `TRON`, `TROFF`, `HELP`~~ | **done** — `src/runtime_housekeeping.c`, `tests/housekeeping_verbs.c` |
|
||||
| B′. Housekeeping, deferred | `RESTORE`, `RENUMBER` | neither is small; see below |
|
||||
| C. Errors | `TRAP`, `RESUME`, `ER`, `ERR` | needs a BASIC-visible error object |
|
||||
| D. Strings/format | `USING`, `PUDEF`, `WIDTH`, `CHAR` | nothing |
|
||||
| E. Console | `WINDOW`, `KEY`, `SLEEP`, `WAIT`, `TI` | the sink, or the clock `akbasic_runtime_settime()` now carries |
|
||||
| F. Disk | `DOPEN`, `DCLOSE`, `APPEND`, `RECORD`, `HEADER`, `COLLECT`, `BACKUP`, `COPY`, `CONCAT`, `RENAME`, `SCRATCH`, `DIRECTORY`, `CATALOG`, `DCLEAR`, `DVERIFY`, `SAVE`, `LOAD`, `VERIFY`, `BLOAD`, `BSAVE`, `BOOT` | `aksl_f*` only; no new akgl |
|
||||
| ~~B′. Housekeeping, deferred~~ | ~~`RESTORE`, `RENUMBER`~~ | **done** — `src/data.c` and `src/renumber.c`, `tests/read_data.c` and `tests/renumber.c` |
|
||||
| ~~C. Errors~~ | ~~`TRAP`, `RESUME`, `ER`, `ERR`~~ | **done** — `src/runtime_trap.c`, `tests/trap_verbs.c`. `ER`/`EL` are `ER#`/`EL#`; see §5 |
|
||||
| ~~D. Strings/format~~ | ~~`USING`, `PUDEF`, `WIDTH`, `CHAR`~~ | **done** — `src/format.c`, `src/runtime_format.c`, `tests/format_verbs.c` |
|
||||
| ~~E. Console~~ | ~~`WINDOW`, `KEY`, `SLEEP`, `WAIT`, `TI`~~ | **done** — `src/runtime_console.c`, `tests/console_verbs.c` |
|
||||
| ~~F. Disk~~ | ~~all 21~~ | **done** — `src/runtime_disk.c`, `tests/disk_verbs.c`. Five refuse for want of a drive and `DIRECTORY` for want of an upstream wrapper; see §5 |
|
||||
| ~~G. Graphics~~ | ~~`GRAPHIC`, `DRAW`, `BOX`, `CIRCLE`, `PAINT`, `COLOR`, `SCALE`, `SSHAPE`, `GSHAPE`, `LOCATE`~~ | **done** — `src/runtime_graphics.c`, `tests/graphics_verbs.c` |
|
||||
| H. Sprites | `SPRITE`, `MOVSPR`, `SPRCOLOR`, `SPRDEF`, `SPRSAV`, `COLLISION` | `libakgl` sprite/actor API — check before filing |
|
||||
| ~~H. Sprites~~ | ~~`SPRITE`, `MOVSPR`, `SPRCOLOR`, `SPRSAV`, `COLLISION`~~ | **done** — `src/runtime_sprite.c`, `src/sprite_akgl.c`, `tests/sprite_verbs.c`. `SPRDEF` is out of scope; see below |
|
||||
| ~~I. Audio~~ | ~~`PLAY`, `SOUND`, `ENVELOPE`, `VOL`, `TEMPO`~~ | **done** — `src/runtime_audio.c`, `src/play.c`, `tests/audio_verbs.c` |
|
||||
| I′. Audio, still gapped | `FILTER` | `libakgl` has no filter stage and SDL3 supplies no primitive; refused with `AKBASIC_ERR_DEVICE`. `SOUND`'s sweep arguments used to be here and landed with `akgl_audio_sweep` in libakgl 0.3.0 |
|
||||
| ~~E′. Console input~~ | ~~`GET`, `GETKEY`, `SCNCLR`~~ | **done** — `src/runtime_input.c`, `tests/input_verbs.c` |
|
||||
| J. Machine | `SYS`, `FETCH`, `STASH`, `POKE`/`PEEK` variants | nothing |
|
||||
| ~~J. Machine~~ | ~~`SYS`, `FETCH`, `STASH`~~ | **done** — `src/runtime_machine.c`, `tests/machine_verbs.c`. `SYS` is refused by name; see §5 |
|
||||
|
||||
**`RESTORE` and `RENUMBER` are deferred, and neither is a small job.**
|
||||
|
||||
- **`RESTORE` needs a DATA pointer, and there isn't one.** `READ` does not read: it declares
|
||||
that the *next* `DATA` line reached in execution order should fill its identifiers, and skips
|
||||
forward until one turns up (`akbasic_cmd_read`, ported faithfully). There is no cursor into a
|
||||
list of `DATA` items, so there is nothing for `RESTORE` to reset. Implementing it honestly
|
||||
means implementing the pointer, which is worth doing on its own merits, because the current
|
||||
arrangement has a defect the corpus does not catch: **a `DATA` line that appears *before* its
|
||||
`READ` is never found**, so `10 DATA 1,2` / `20 READ A#` runs off the end of the program in
|
||||
silence, and a second `READ` re-reads the same line rather than continuing. The fix is a
|
||||
pre-scan of every `DATA` statement into a flat list plus a read cursor, at which point
|
||||
`RESTORE [line]` is three lines. Its own commit, with the golden cases the change would need.
|
||||
- **`RENUMBER` has to rewrite references or it is worse than useless.** Moving lines is easy —
|
||||
source is filed under its line number in `source[]`. Rewriting every `GOTO`, `GOSUB`, `THEN`,
|
||||
`ELSE`, `RUN` and `LIST` target to match is the job, and doing it on source text means
|
||||
distinguishing a line number from a digit inside a string literal. Refusing it is better than
|
||||
a `RENUMBER` that silently breaks every branch in the program.
|
||||
- ~~**`RESTORE` needs a DATA pointer, and there isn't one.**~~ **Done.** Every `DATA` statement
|
||||
is pre-scanned into one flat list before the program runs -- `src/data.c`, called from
|
||||
`akbasic_runtime_set_mode()` beside the label prescan -- and `READ` walks a cursor along it.
|
||||
`RESTORE` resets the cursor; `RESTORE (line)` moves it to the first item on or after that
|
||||
line.
|
||||
|
||||
It fixed two defects on the way, both of which were consequences of `READ` not reading.
|
||||
**A `DATA` line above its `READ` is now found**, where skipping forward from the `READ` used
|
||||
to run off the end of the program in silence. And **the lines between a `READ` and its `DATA`
|
||||
now execute**, where the skip used to swallow them -- a C128 runs them, because `READ` takes
|
||||
the next item and execution carries on. The one corpus case has its `DATA` immediately after
|
||||
its `READ`, so neither was observable there. `tests/read_data.c`.
|
||||
|
||||
`DATA` at run time is now a no-op: it is a declaration, and reaching the statement means
|
||||
walking past it.
|
||||
|
||||
- ~~**`RENUMBER` has to rewrite references or it is worse than useless.**~~ **Done.**
|
||||
`src/renumber.c` builds the whole old-to-new map first, then rewrites every line -- including
|
||||
lines *before* the renumbered region, which can branch into it -- and only then moves them.
|
||||
|
||||
The rewrite is textual and understands exactly one thing about the rest of the language:
|
||||
where a string literal starts and stops, so `PRINT "GOTO 10"` survives. `GOTO`, `GOSUB`,
|
||||
`RUN`, `RESTORE` and `TRAP` targets are rewritten, and so is the comma-separated list after
|
||||
`ON X GOTO` -- for free, since the targets follow the `GOTO`. `COLLISION` is special-cased
|
||||
because its handler is its *second* argument.
|
||||
|
||||
Two deliberate non-rewrites. **A target naming a line that does not exist is left alone**:
|
||||
`GOTO 9999` in a program with no line 9999 is already broken, and inventing a destination
|
||||
would hide that. And **`THEN`/`ELSE` do not take bare line numbers in this dialect** -- the
|
||||
corpus writes `THEN GOTO 100` -- so a number after `THEN` is an expression, not a target.
|
||||
|
||||
A renumbering that would land a moved line on a kept one is refused before anything is
|
||||
written, because losing a line to a renumbering is not recoverable.
|
||||
|
||||
**A program written with labels needs none of this.** `GOTO DONE` is unaffected by any
|
||||
renumbering, which is the strongest argument for `LABEL` there is.
|
||||
|
||||
`LOCATE` used to appear in both E and G. It belongs to G alone: in BASIC 7.0 `LOCATE` moves the
|
||||
*graphics* pixel cursor that `DRAW` starts from, and the text cursor verb is `CHAR`, which is
|
||||
@@ -661,7 +682,21 @@ already in group D.
|
||||
|
||||
**Out of scope, and staying that way** — the reference marks these as incompatible with a
|
||||
modern PC and that reasoning stands: `BANK` (no bank switching), `FAST` (irrelevant CPU speed
|
||||
control), `MONITOR` (no machine-language monitor).
|
||||
control), `MONITOR` (no machine-language monitor). **`SPRDEF` joins them** on the same
|
||||
reasoning: it is not a programmable verb but an interactive full-screen sprite editor, driven
|
||||
by single keystrokes and its own cursor. A program cannot call it usefully and this interpreter
|
||||
does not own the screen it would take over. The three `SPRSAV` source forms are what replace it.
|
||||
|
||||
**The `libakgl` JSON layer turned out not to be in the way at all.** Group H was parked on
|
||||
"check the sprite/actor API before filing", and the concern was that libakgl's sprites are
|
||||
described by JSON documents that would be cumbersome to reach through BASIC.
|
||||
`akgl_sprite_load_json()` is a thin wrapper over `akgl_spritesheet_initialize` +
|
||||
`akgl_sprite_initialize` + writes to public struct fields, and every field it fills from a
|
||||
document — frame list, animation speed, loop flags, state-to-sprite map — is something a
|
||||
Commodore sprite does not have. So `src/sprite_akgl.c` builds the same four objects directly.
|
||||
The one field that *did* survive into the verb syntax is the spritesheet's filename:
|
||||
`SPRSAV "ship.png", 1` goes through `akgl_path_relative()` and `akgl_spritesheet_initialize()`,
|
||||
which is the same pair of calls the JSON loader makes.
|
||||
|
||||
Also on the queue, from the reference's own defect list:
|
||||
|
||||
@@ -940,6 +975,278 @@ deviations from the reference's *program*: `main.go` and the SDL half of
|
||||
when that arm is the one not taken — with an `ELSE` the last arm is `ELSE`, without one it
|
||||
is `THEN`. That is one line of code and it reads as an oddity without the table above it.
|
||||
|
||||
### Deviations in the structure and error verbs (groups A and C)
|
||||
|
||||
44. **A whole loop on one line does not loop.** `DO : PRINT 1 : LOOP` runs once, exactly as
|
||||
`FOR I=1 TO 3 : PRINT I : NEXT I` does. Block skipping walks *source lines*, which is the
|
||||
reference's `waitingForCommand` model (§1.6), so a `LOOP` on the same line as its `DO` is
|
||||
never reached as a separate step. Fixing it means making the skip operate on statements
|
||||
rather than lines — a deliberate piece of work, already noted in §4 against `FOR`.
|
||||
|
||||
45. **`ER` and `EL` are `ER#` and `EL#`.** A C128 exposes them as bare reserved names. This
|
||||
dialect cannot: an identifier carries its type in a suffix, and a name without one is a
|
||||
*label*. They are written into the global scope when a trap fires, so `PRINT ER#` reads the
|
||||
way the rest of the language does and needs no new syntax.
|
||||
|
||||
`ER#` carries the `libakerror` status code, not a Commodore error number — 515 for an
|
||||
out-of-bounds subscript rather than C128 error 9. There is no correspondence to reproduce:
|
||||
the errors this interpreter raises are not the errors a 1985 ROM raised. `ERR(ER#)` gives
|
||||
the name, and that is the part a program should be printing anyway.
|
||||
|
||||
46. **`END` does not arm `CONT`.** A C128 lets `CONT` resume after `END` as well as after
|
||||
`STOP`. Here `END` means the program finished, and continuing from a finished program
|
||||
resumes at whatever line happens to follow — so `CONT` refuses instead. `STOP` still arms it.
|
||||
|
||||
### Deviations in the format verbs (group D)
|
||||
|
||||
47. **`PRINT USING` renders one field per statement.** BASIC 7.0 lets a format string hold
|
||||
several fields and `PRINT USING` take a list of values against them. Here the *first* field
|
||||
is used and any literal text around it is copied through, so `PRINT USING "A: ###"; X` works
|
||||
and `PRINT USING "### ###"; X, Y` does not. Multiple fields need `PRINT` to take a value
|
||||
list, which it does not yet — a separate piece of work from the field rendering itself,
|
||||
which `src/format.c` does properly.
|
||||
|
||||
Exponential fields (`^^^^`) are not implemented either. Nothing in the language produces a
|
||||
value that needs one, since `PRINT` renders a float with `%f`.
|
||||
|
||||
48. **`WIDTH` is emulated with parallel passes.** It sets the thickness of drawn lines, and
|
||||
neither the graphics backend record nor `akgl_draw_line` takes a thickness — so a `WIDTH 2`
|
||||
line is drawn twice, offset one pixel along whichever axis the line does *not* mostly run
|
||||
along. Visually right at the only other value the verb accepts; a general thickness would
|
||||
want a real perpendicular offset and an API that takes one.
|
||||
|
||||
49. **`CHAR` ignores its colour argument, and needs a sink that has a cursor.** The text sink
|
||||
draws in one colour, chosen by the host when it built the sink, so a per-call colour would
|
||||
have to become part of the sink interface. The argument is accepted and ignored rather than
|
||||
refused, because refusing it would make every published `CHAR` line a syntax error.
|
||||
|
||||
Positioning is a new optional `moveto` on the sink record. A stdio sink leaves it NULL and
|
||||
`CHAR` refuses by name — a terminal's cursor is not this library's to move and a pipe has no
|
||||
cursor at all — so `CHAR` works in an AKGL build and says why it cannot in a stdio one.
|
||||
|
||||
### Deviations in the console verbs (group E)
|
||||
|
||||
50. **`SLEEP` and `WAIT` hold the step loop; they do not block.** §1.6 forbids blocking, so a
|
||||
waiting program is one that does not *advance* — `akbasic_runtime_step()` still returns, a
|
||||
bounded `akbasic_runtime_run()` still comes back on time, and an embedded game keeps its
|
||||
frame rate while a script sleeps.
|
||||
|
||||
`SLEEP` with no host clock does nothing at all rather than waiting forever. A deadline
|
||||
computed from a clock that never advances is never reached, and the first version of this
|
||||
hung the test suite proving it.
|
||||
|
||||
51. **`TI` and `TI$` are `TI#` and `TI$`.** Same reason as `ER#`/`EL#`: no bare variable names.
|
||||
They are ordinary globals refreshed once per step from the host's clock, rather than
|
||||
pseudo-variables, because this dialect has no mechanism for a name that computes itself.
|
||||
A host that never sets a clock gets a stopped clock rather than a wrong one.
|
||||
|
||||
52. **`WAIT` polls ordinary process memory.** On a C128 the byte is a hardware register an
|
||||
interrupt is changing. Here nothing changes it but the host, another thread, or a `POKE`
|
||||
from a `TRAP` handler — so a program that waits on a byte nobody writes waits forever, which
|
||||
is exactly what the same program does on a C128 with the wrong address. Read through a
|
||||
`volatile` pointer, because the whole point is that something outside this program writes it.
|
||||
|
||||
53. **`KEY` stores its macros and nothing expands them.** The definitions are kept and bare
|
||||
`KEY` lists them, which is the half that is this library's business. Expanding a macro when
|
||||
a function key is pressed belongs to whatever owns the keyboard — the frontend's line editor
|
||||
— and the input backend delivers keycodes rather than editor commands.
|
||||
|
||||
### Deviations in the machine verbs (group J)
|
||||
|
||||
54. **`FETCH` and `STASH` are the same byte copy.** On a C128 they differ by which side is the
|
||||
RAM Expansion Unit — `STASH` writes out to it, `FETCH` reads back. There is no expansion
|
||||
unit and there are no banks, so both are `memmove` between two addresses and saying so is
|
||||
better than inventing a distinction. `memmove` rather than `memcpy` because a program
|
||||
shifting a buffer along by a few bytes overlaps its own source, which is a normal ask.
|
||||
|
||||
Addresses are real process addresses, which is the decision `POKE`, `PEEK` and `POINTER`
|
||||
already made. There is no bounds check and there cannot be one: a wrong address is a
|
||||
segmentation fault, not an error message.
|
||||
|
||||
55. **`SYS` is refused by name.** It calls machine code at an address. There is no 6502 and no
|
||||
ROM to call, so there is nothing to jump to and nothing sensible to emulate — and jumping to
|
||||
a real address in this process would be a way to crash it on purpose. It parses first and
|
||||
refuses second, so a listing containing `SYS` still loads and still lists; only running it
|
||||
fails, and it says why.
|
||||
|
||||
Same reasoning as `BANK`, `FAST` and `MONITOR`, with one difference: those have no table row
|
||||
at all, and `SYS` has a handler because it is common enough in published listings to deserve
|
||||
a specific message rather than "Unknown command".
|
||||
|
||||
### Deviations in the disk verbs (group F)
|
||||
|
||||
56. **There is no 1541, so the verbs split three ways.** The ones that mean something on a
|
||||
filesystem are implemented against `aksl_f*` — `DOPEN`, `DCLOSE`, `APPEND`, `RECORD`,
|
||||
`SCRATCH`, `RENAME`, `COPY`, `CONCAT`, `BSAVE`, `BLOAD`, `VERIFY`. The ones that are
|
||||
spellings of verbs already here are aliases: `SAVE` is `DSAVE`, `LOAD` is `DLOAD`, `CATALOG`
|
||||
is `DIRECTORY`, `DVERIFY` is `VERIFY`.
|
||||
|
||||
**`HEADER`, `COLLECT`, `BACKUP` and `BOOT` are refused by name**, with the reason. Each
|
||||
operates on a physical disk — formatting one, validating its block allocation map,
|
||||
duplicating it, booting from it — and a filesystem has no equivalent that is not a lie.
|
||||
`HEADER` would have to mean "delete everything in this directory", which is a
|
||||
spectacularly bad thing to do to somebody who typed a C128 verb. `DCLEAR` is the exception:
|
||||
resetting a drive also closes its channels, and closing the channels is real, so that is
|
||||
what it does.
|
||||
|
||||
57. **`DIRECTORY` is refused for want of an upstream wrapper.** `libakstdlib` has no
|
||||
`aksl_opendir`, and this project's rule is that a missing capability is filed upstream
|
||||
rather than worked around — so it is filed in `deps/libakstdlib/TODO.md` and the verb says
|
||||
so. Calling `opendir(3)` here would mean reporting through `errno` in a file where
|
||||
everything else reports through an `akerr_ErrorContext *`.
|
||||
|
||||
58. **`PRINT #` and `INPUT #` need the space before the `#`.** A C128 writes `PRINT#1,A$`. The
|
||||
scanner reads `#` as a type suffix, so `PRINT#` comes out as an identifier named `PRINT`
|
||||
with an integer suffix — and a verb name carrying a suffix is refused (deviation 30). With
|
||||
a space the `#` is its own token and the parse handler reads it.
|
||||
|
||||
59. **`RECORD` counts lines, not fixed-length records.** A C128's relative file has a record
|
||||
length fixed when the file was created; a filesystem file has none. So a record is a line
|
||||
and `RECORD` rewinds and reads forward to it, which is slower than a seek and is the only
|
||||
definition that does not invent a record length the file does not have.
|
||||
|
||||
60. **`BLOAD` requires a length.** A C128 reads until the file ends. Here the address is a real
|
||||
process address, so a file longer than the caller expected would write past whatever it was
|
||||
pointed at with no way to notice. Refusing to guess is the only safe answer.
|
||||
|
||||
### Deviations in conditions
|
||||
|
||||
41. **A lone `=` is equality inside a condition.** `IF A# = 2 THEN` works, which it did not
|
||||
before: the scanner reads `==` as EQUAL and a single `=` as ASSIGNMENT, because at that
|
||||
point it cannot know whether it is looking at a statement or a condition, and the reference
|
||||
never resolved the ambiguity anywhere. A condition is the one context where it has an
|
||||
answer — BASIC has no assignment expression — so `akbasic_Parser::comparing` is set around
|
||||
the condition and the relation parser offers ASSIGNMENT as a comparison operator only while
|
||||
it is. `==` keeps working, and the whole checked-in corpus is written in it.
|
||||
|
||||
Outside a condition `=` has to stay an assignment, which is what the flag is for: the first
|
||||
attempt put ASSIGNMENT in the operator list unconditionally, and `FOR I# = 1 TO 5` promptly
|
||||
stopped initializing its counter.
|
||||
|
||||
42. **A condition is a whole expression, so `AND` and `OR` work in one.**
|
||||
`IF A# = 5 AND B# = 3 THEN` used to report "Incomplete IF statement". The reference parses a
|
||||
single *relation* after `IF`, and a relation sits below `AND` and `OR` in the grammar chain,
|
||||
so the `AND` was never consumed and the `THEN` check found the wrong token.
|
||||
`akbasic_parse_if()` calls `akbasic_parser_expression()` now.
|
||||
|
||||
43. **Truth is nonzero, not "a boolean holding true".** `akbasic_value_is_truthy()` is what a
|
||||
branch tests. Commodore BASIC has no boolean type: a comparison yields -1 or 0, `AND` and
|
||||
`OR` *are* the bitwise operators, and `IF A THEN` is legal for any numeric A. The reference
|
||||
tests `boolvalue == -1` regardless of the value's type, so `IF A# = 1 OR B# = 2 THEN` — whose
|
||||
`OR` yields an integer — was silently always false, and so was `IF A# THEN`.
|
||||
|
||||
The bitwise operators accept a truth value as an operand for the same reason: -1 is every
|
||||
bit set precisely so that `AND` and `OR` double as the logical pair. A string is still never
|
||||
true; a C128 raises a type mismatch there, which is a stricter answer this could adopt later.
|
||||
|
||||
### Deviations in the sprite verbs (group H)
|
||||
|
||||
32. **Labels are filed before the program runs, not as each `LABEL` executes.**
|
||||
`akbasic_runtime_scan_labels()` walks the stored source on every entry into
|
||||
`AKBASIC_MODE_RUN` and files every `LABEL <name>` it finds, textually, without parsing the
|
||||
lines around it. The reference resolves a label only once its `LABEL` statement has run, so
|
||||
`GOTO` and `GOSUB` reached backwards and never forwards.
|
||||
|
||||
**This was not a nice-to-have.** An interrupt handler by definition sits on a line normal
|
||||
flow does not fall into, so `COLLISION 1, BUMPED` could not be made to work at all under the
|
||||
old rule — neither resolving at arm time nor at fire time helps when the `LABEL` line is
|
||||
never executed. Forward `GOTO` is the improvement that came with it, covered by
|
||||
`tests/language/statements/label_forward.bas`.
|
||||
|
||||
`LABEL` still executes and still files itself, so a name that appears twice resolves to the
|
||||
last one in the source until one of them runs. The scan is textual on purpose: parsing every
|
||||
line up front would raise on lines the program would never have reached.
|
||||
|
||||
33. **Sprite coordinates are BASIC's 320×200, not the VIC-II's 0–511 by 0–255.** `MOVSPR` takes
|
||||
the same coordinate space `DRAW` does. A C128's sprite coordinates are the raster's, offset
|
||||
so that (24, 50) is the top-left of the visible screen; reproducing that would give the
|
||||
language two coordinate systems and make `MOVSPR 1, 0, 0` put a sprite off-screen. Consistent
|
||||
with deviation 18.
|
||||
|
||||
34. **`MOVSPR`'s speed unit is ours.** BASIC 7.0 documents speed 0–15 with 15 fastest and says
|
||||
nothing about what a unit is worth. Here one unit is
|
||||
`AKBASIC_SPRITE_SPEED_PIXELS_PER_SECOND` (5) pixels per second, so speed 15 crosses the
|
||||
320-pixel screen in about four seconds. Paced off `akbasic_runtime_settime()` from
|
||||
`akbasic_runtime_step()`, exactly as the `PLAY` queue is, so a host that never sets a clock
|
||||
gets a still picture rather than a hang — same trade as deviation 20.
|
||||
|
||||
35. **`SPRSAV` takes an integer array where a C128 takes a string, and takes a file path as
|
||||
well.** Three source forms, one of which is the C128's:
|
||||
|
||||
| Form | Source |
|
||||
|---|---|
|
||||
| `SPRSAV A$, n` | a region `SSHAPE` saved. The C128 documents `SPRSAV`'s string as *the `SSHAPE` data format at a fixed 24×21*, so sharing the shape pool is faithful rather than a shortcut |
|
||||
| `SPRSAV A#, n` | 63 elements of a DIMmed integer array — the DATA-driven path a type-in listing uses |
|
||||
| `SPRSAV "ship.png", n` | an image file |
|
||||
|
||||
**The array form exists because a string here cannot hold a sprite.** A value carries a
|
||||
NUL-terminated `char[256]` (§1.2) and a pattern is 63 raw bytes including zeros, so the
|
||||
C128's own spelling is unrepresentable. `DIM P#(63)` is exactly the right size.
|
||||
|
||||
**The file form is a deliberate addition, not a fallback.** A string source is a path unless
|
||||
it starts with `SHAPE:`, which is the prefix `SSHAPE` mints, so the two never collide. It
|
||||
resolves through `akgl_path_relative()` — the working directory first, then the directory the
|
||||
running program was loaded from — and loads through `akgl_spritesheet_initialize()`, which
|
||||
are the same two calls `akgl_sprite_load_json()` makes for a spritesheet. A sprite loaded
|
||||
this way takes **the image's own size** rather than being forced to 24×21; a modern PC has
|
||||
no reason to throw away art that is not sprite-shaped.
|
||||
|
||||
`akbasic_runtime_set_source_path()` is what tells the interpreter where the program came
|
||||
from. Group F's disk verbs will want the same thing.
|
||||
|
||||
**The reverse direction is refused.** `SPRSAV n, A$` on a C128 copies a sprite *out*; here
|
||||
that would mean writing an image file, which is a disk operation, and group F is
|
||||
unimplemented as a whole. Refused by name rather than half-built.
|
||||
|
||||
36. **`COLLISION` implements type 1 only, and types 2 and 3 are refused by name.**
|
||||
Sprite-to-background needs the whole render target read back and compared against each
|
||||
sprite every frame; a light pen has no meaning on a machine with no light pen. Both raise
|
||||
`AKBASIC_ERR_DEVICE` with the reason rather than being accepted and never firing.
|
||||
|
||||
Collision is **bounding-box, not pixel**. A C128's VIC-II collides on set pixels, so two
|
||||
sprites whose boxes overlap but whose art does not are reported as colliding here and would
|
||||
not be there. Pixel-exact collision would mean keeping every sprite's unpacked bitmap and
|
||||
testing the overlap rectangle a pixel at a time; the box test is four comparisons.
|
||||
`akgl_collide_rectangles()` is deliberately not used — it has a documented
|
||||
corner-containment defect that misses a plus-shaped overlap, and nothing in libakgl calls it.
|
||||
|
||||
37. **A collision handler is entered between source lines, and must end in `RETURN`.**
|
||||
`akbasic_runtime_service_interrupts()` runs at the top of `akbasic_runtime_step()` and
|
||||
injects what amounts to a `GOSUB` the program did not write. Between lines is the only safe
|
||||
place: a handler entered mid-statement would have to return into the middle of a line and
|
||||
the parser keeps no state that could resume there. That is the same granularity block
|
||||
skipping already works at (§1.6).
|
||||
|
||||
An interrupt does not interrupt an interrupt. A collision that is still true while its own
|
||||
handler runs would otherwise re-enter on the next line until the environment pool was gone.
|
||||
The event is not lost — it is taken as soon as the handler's `RETURN` lands.
|
||||
|
||||
38. **Sprite priority is recorded and not honoured.** `SPRITE n,,,1` sets the bit, `RSPRITE`
|
||||
reads it back, and nothing draws differently. A C128 draws a low-priority sprite behind the
|
||||
bitmap plane; here the text layer, the drawing surface and the sprites share one render
|
||||
target and sprites are composited on top of it every frame, so there is nothing to go
|
||||
behind. Honouring it would mean a separate composited surface per plane — which is the same
|
||||
piece of work deviation 19 already needs.
|
||||
|
||||
39. **Multicolour mode is recorded and not honoured.** Same shape as priority: `SPRITE`'s
|
||||
seventh argument and `SPRCOLOR`'s two registers are kept, `RSPRITE` and `RSPCOLOR` read them
|
||||
back, and no sprite is drawn in multicolour. Multicolour packs *two* bitmap bits per pixel
|
||||
and selects between the sprite's own colour and the two shared registers; every `SPRSAV`
|
||||
source form here carries one bit per pixel or a full-colour image, so there is no second bit
|
||||
to select with. The argument positions are kept honest for the day a multicolour pattern
|
||||
format exists.
|
||||
|
||||
40. **A sprite is a real `libakgl` actor, and it is drawn by a renderfunc of ours.** Each of the
|
||||
eight becomes a `SpriteSheet` + `Sprite` + `Character` + `Actor` registered in
|
||||
`AKGL_REGISTRY_ACTOR`, so an embedding game sees BASIC's sprites alongside its own (goal 3).
|
||||
But `akgl_actor_render()` is not what draws them: it computes its destination *height* from
|
||||
the sprite's **width**, which would draw a 24×21 Commodore sprite as a 24×24 square, and an
|
||||
actor carries a single scalar `scale` that cannot express `SPRITE`'s separate x- and
|
||||
y-expand bits. `src/sprite_akgl.c` installs its own `renderfunc`, which is libakgl's own
|
||||
extension point for exactly this. Both defects are filed upstream.
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
## 6. Reference defects — **fix them; fidelity is no longer a reason not to**
|
||||
@@ -958,53 +1265,62 @@ correct contract, and a golden file moved in the same commit if the fix changes
|
||||
`AKBASIC_KNOWN_FAILING_TESTS` is empty now and is kept declared for the next defect that has to
|
||||
be reproduced before it can be fixed.
|
||||
|
||||
1. **`basicvariable.go:176`** — `toString()` tests `len(self.values) == 0` and then indexes
|
||||
`self.values[0]`. Inverted condition; on an empty variable it panics, and on a non-empty one
|
||||
it returns the "not implemented for arrays" string. *Consequence:* `PRINT` of a scalar
|
||||
through this path is wrong. Fix: `if ( count == 1 )`.
|
||||
2. **`basicvariable.go:108`** — `setBoolean` builds a value with `valuetype: TYPE_STRING`.
|
||||
*Consequence:* a boolean stored in a variable reads back as a string with an empty
|
||||
`stringval`. Fix: `TYPE_BOOLEAN`.
|
||||
3. **`basicenvironment.go:157`** — `stopWaiting(command)` ignores `command` and clears the
|
||||
wait unconditionally. *Consequence:* an inner block can clear an outer block's wait. Fix:
|
||||
compare before clearing, and error on a mismatch.
|
||||
4. **`basicvalue.go:181`** — `mathPlus` mutates `self` in place when `self.mutable` is true,
|
||||
while every other operator always clones. *Consequence:* `A# + 1` can modify `A#` depending
|
||||
on where the value came from. This one is high blast radius; it interacts with
|
||||
`eval_clone_identifiers` and with `CommandNEXT`'s increment
|
||||
(`basicruntime_commands.go:663`), which **relies** on the mutation. Do not fix it before
|
||||
`FOR`/`NEXT` has full test coverage.
|
||||
5. **`basicvalue.go:191` and siblings** — every binary operator adds both of the right-hand
|
||||
operand's numeric fields: `rval.intval + int64(rval.floatval)`. Works only because the
|
||||
unused field is always zero. *Consequence:* none today; it is a landmine for any future
|
||||
value that carries both.
|
||||
6. **`basicvalue.go`, all comparisons** — `dest` is cloned from `self`, so the resulting
|
||||
boolean inherits `self.name`. *Consequence:* a comparison result can be written back to the
|
||||
wrong variable through `environment.update()`.
|
||||
7. **`basicruntime_commands.go:484`** — `CommandIF` walks `expr.right` to the end of the chain
|
||||
looking for a `THEN` command, then dereferences `expr.right` after the loop guaranteed it
|
||||
is `nil`. *Consequence:* the "Malformed IF statement" check is unreachable and nested
|
||||
`IF ... THEN ... ELSE` is unverified.
|
||||
8. **`basicruntime_commands.go:669`** — `CommandEXIT` pops the environment without
|
||||
`stopWaiting`, leaving the parent waiting on a `NEXT` that will never come.
|
||||
9. **`basicruntime.go:149`** — `newVariable()` and `BasicRuntime.variables[MAX_VARIABLES]` are
|
||||
dead; variables live in the environment's map. Do not port them.
|
||||
1. ~~**`basicvariable.go:176`** — `toString()` tests `len(self.values) == 0` and then indexes
|
||||
`self.values[0]`.~~ **Never ported.** There is no `akbasic_variable_to_string`: rendering a
|
||||
value is `akbasic_value_to_string()`, which switches on the value type and has no count test
|
||||
to invert. The buggy function had no counterpart to reproduce.
|
||||
2. ~~**`basicvariable.go:108`** — `setBoolean` builds a value with `valuetype: TYPE_STRING`.~~
|
||||
**Fixed in the port.** `akbasic_value_set_bool()` sets `AKBASIC_TYPE_BOOLEAN`, and
|
||||
`tests/value_compare.c` reads a boolean back through every comparison operator.
|
||||
3. ~~**`basicenvironment.go:157`** — `stopWaiting(command)` ignores `command`.~~ **Fixed in
|
||||
the port.** `akbasic_environment_stop_waiting()` walks the scope chain and clears the first
|
||||
environment actually waiting for *that* verb, so an inner block can no longer clear an outer
|
||||
block's wait. A miss is tolerated rather than raised, which is the one part of the suggested
|
||||
fix not taken: `EXIT` and the end of a `DEF` body both call it speculatively.
|
||||
4. ~~**`basicvalue.go:181`** — `mathPlus` mutates `self` in place when `self.mutable` is
|
||||
true.~~ **Fixed.** It clones like every other operator now, so `A# + 1` cannot modify `A#`.
|
||||
|
||||
The gate on this item was `FOR`/`NEXT` coverage, because `NEXT`'s increment *relied* on the
|
||||
mutation to advance the counter. `tests/for_next.c` is that coverage -- `STEP`, a negative
|
||||
step, a float counter, a body that assigns to the counter, `EXIT`, and nesting, none of which
|
||||
the golden corpus reaches -- and `akbasic_cmd_next()` now writes the incremented value back
|
||||
with `akbasic_variable_set_subscript()`. Removing that write-back does not fail the suite, it
|
||||
*hangs* it: the counter stops advancing and the loop never ends.
|
||||
5. ~~**`basicvalue.go:191` and siblings** — every binary operator adds both of the right-hand
|
||||
operand's numeric fields.~~ **Fixed.** `rval_as_int()` and `rval_as_float()` select on the
|
||||
operand's type instead of summing `intval + (int64_t)floatval`.
|
||||
|
||||
It was filed as a landmine with no consequence today, and that is why it needed a test
|
||||
written against the value API rather than against the language: no BASIC program can build a
|
||||
value carrying both fields, and the old code passes every other test in the tree.
|
||||
`tests/value_arithmetic.c` constructs one directly.
|
||||
6. ~~**`basicvalue.go`, all comparisons** — `dest` is cloned from `self`, so the resulting
|
||||
boolean inherits `self.name`.~~ **Moot.** `BasicValue.name` was dropped as dead during the
|
||||
port along with `BasicEnvironment.update()`, its only reader — deviation 9 above. There is no
|
||||
name for a comparison result to inherit.
|
||||
7. ~~**`basicruntime_commands.go:484`** — `CommandIF` dereferences `expr.right` after the loop
|
||||
guaranteed it is `nil`, so the "Malformed IF statement" check is unreachable.~~ **Fixed in
|
||||
the port.** `akbasic_parse_if()` matches the `THEN` token and compares its lexeme directly,
|
||||
so the check runs on every `IF`.
|
||||
8. ~~**`basicruntime_commands.go:669`** — `CommandEXIT` pops the environment without
|
||||
`stopWaiting`.~~ **Fixed**, and then fixed again — see item 18, which is what clearing the
|
||||
wait left behind.
|
||||
9. ~~**`basicruntime.go:149`** — `newVariable()` and `BasicRuntime.variables[MAX_VARIABLES]`
|
||||
are dead.~~ **Not ported as dead code.** The pair is live here and is the storage model:
|
||||
`akbasic_environment_create()` takes a slot from `akbasic_runtime_new_variable()`, because a
|
||||
pool is what replaced Go's per-environment map (§1.3). The reference's versions were dead;
|
||||
these are not.
|
||||
10. ~~**`basicgrammar.go:224`** — `newLiteralInt` selects base 8 whenever the lexeme starts
|
||||
with `0`.~~ **Fixed.** Base 10 unless prefixed `0x`; a leading zero in a listing is padding,
|
||||
not a radix. `PRINT 010` prints 10 and `PRINT 08` parses. Nothing in the upstream corpus
|
||||
had a leading-zero literal, which is why nothing there noticed. Regression tests in
|
||||
`tests/grammar_leaves.c` and `tests/language/numeric/octal_literal.bas`, which was rewritten
|
||||
from pinning the defect to pinning the fix.
|
||||
11. **`basicruntime.go:121` / `basicparser_commands.go:124`** — environments are never freed.
|
||||
Addressed structurally by §1.4; no separate fix needed, but the C pool **will** exhaust
|
||||
where Go merely leaked, so `tests/environment_scope.c`'s exhaustion assertion matters.
|
||||
|
||||
### Found during the port
|
||||
|
||||
Five more, none of which the reference's own corpus catches. **Four are now fixed**, each with
|
||||
a regression test in the suite that pinned it; the fifth is described below and is not a coding
|
||||
question.
|
||||
|
||||
11. ~~**`basicruntime.go:121` / `basicparser_commands.go:124`** — environments are never
|
||||
freed.~~ **Fixed in the port.** They come from a fixed pool and
|
||||
`akbasic_runtime_prev_environment()` releases each one — deviation 3 above. An unreleased
|
||||
scope shows up as pool exhaustion rather than as unbounded growth, which is how item 18
|
||||
announced itself.
|
||||
12. ~~**`basicparser.go:329` — `subtraction()` returns after one operator where `addition()`
|
||||
loops.**~~ **Fixed.** `1 - 2 - 3` computed `1 - 2` and abandoned the rest of the line; the
|
||||
remaining `- 3` was left in the token stream, picked up by the statement loop as a second
|
||||
@@ -1108,6 +1424,45 @@ question.
|
||||
|
||||
---
|
||||
|
||||
|
||||
### Found while auditing this section against the C port
|
||||
|
||||
Writing the `FOR`/`NEXT` coverage that item 4 was gated on turned these up. The first two are
|
||||
the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
|
||||
18. ~~**`EXIT` on the first pass through a loop restarts the program.**~~ **Fixed.** `EXIT`
|
||||
jumped to `loopExitLine`, which is only ever written by `NEXT` — so an `EXIT` before any
|
||||
`NEXT` had run jumped to line 0. Each restart entered the `FOR` again and took another
|
||||
environment and another variable, so what a reader saw was "Maximum runtime variables
|
||||
reached" reported against the `FOR`, on a four-line program.
|
||||
|
||||
It now skips forward to the `NEXT` using the same `waitingForCommand` machinery a
|
||||
zero-iteration body uses, and the `NEXT` pops on an `exiting` flag. Neither verb has to know
|
||||
where the loop ends. `tests/for_next.c`.
|
||||
|
||||
19. **A `FOR` body runs once with the overshot counter.** The loop condition is tested against
|
||||
the counter *before* the increment, so the increment's result reaches the body before
|
||||
anything compares it to the limit. `FOR I = 1 TO 9 STEP 3` runs its body with 1, 4, 7 **and
|
||||
then 10**. Invisible whenever the step lands exactly on the limit, which is every case in
|
||||
the golden corpus and every case in the reference's own tests.
|
||||
|
||||
Related, and the same off-by-one from the other end: `FOR I = 1 TO 1` does not run its body
|
||||
at all, where every BASIC ever written runs it once. The two errors compensate for a step of
|
||||
1, which is why neither was noticed.
|
||||
|
||||
**Not fixed, because the corpus pins it.**
|
||||
`tests/reference/language/flowcontrol/forloopwaitingforcommand.bas` depends on
|
||||
`FOR I# = 1 TO 1` skipping its body, and `tests/reference/README.md` forbids editing an
|
||||
expectation to suit this interpreter. Registered as `tests/for_semantics.c` in
|
||||
`AKBASIC_KNOWN_FAILING_TESTS`, asserting the correct contract. Fixing it means deciding what
|
||||
to do with that golden file, and that is a decision rather than a patch.
|
||||
|
||||
20. **A `FOR` counter does not survive its loop.** It is created in the environment the loop
|
||||
pushes, which pops when the loop ends, so reading it afterwards finds a fresh variable
|
||||
holding zero. A C128 leaves it at the value that ended the loop and plenty of published
|
||||
listings read it there. Same known-failing test as item 19; the fix is a scoping decision
|
||||
about where a loop counter is created, not an ordering one.
|
||||
|
||||
## 7. Filing gaps against `libakgl`
|
||||
|
||||
When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in
|
||||
@@ -1207,7 +1562,7 @@ Dependency baseline:
|
||||
|---|---|---|
|
||||
| `deps/libakerror` | 1.0.0 | Private ownership-enforced status registry. akbasic reserves 512–767 in `akbasic_error_register()`. Does not namespace its `coverage` target when embedded — worked around in our `CMakeLists.txt`. |
|
||||
| `deps/libakstdlib` | 0.2.0 | soname `libakstdlib.so.0.2`. `AKSL_VERSION_CHECK()` asserted in `tests/version_check.c`. This release fixed all six confirmed defects the port was working around — see §1.9, where the bans are now lifted. |
|
||||
| `deps/libakgl` | 0.3.0 | soname `libakgl.so.0.3`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. **0.3.0 closed every API gap this port had filed** — see §7 — so the four workarounds §3 used to list are gone. |
|
||||
| `deps/libakgl` | 0.4.0 | soname `libakgl.so.0.4`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. **0.3.0 closed every API gap this port had filed** — see §7 — so the four workarounds §3 used to list are gone. 0.4.0 is a leak-and-overread release that changed no public struct, but the soname carries `MAJOR.MINOR` while the major is 0, so rebuild rather than relink. |
|
||||
|
||||
**The `libakgl` requirement used to be pinned by commit, and no longer is.** 42b60f7 added 22
|
||||
public symbols across four headers and left `project(akgl VERSION 0.1.0)` and the
|
||||
|
||||
Reference in New Issue
Block a user