Andrew Kesterson
7897a49fb5
AKBASIC_WITH_AKGL currently builds and links four tested adaptors, but basic never selects them. It remains the stdio driver, creates no SDL resources, attaches no devices and pumps no events. Narrow the completed claim to the core port and adaptor layer. Add the missing host responsibilities, affected files and acceptance criteria, then put the standalone frontend and its line editor at the top of the remaining-work queue. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
977 lines
60 KiB
Markdown
977 lines
60 KiB
Markdown
# TODO
|
||
|
||
Implementation plan for the Go → C port of `deps/basicinterpret`.
|
||
|
||
This file is written to be executed by AI agents, not read by humans for inspiration. Every
|
||
item names the files it touches, the exact deliverable, and the command that proves it done.
|
||
Work the phases in order. Inside a phase, items with no `Depends:` line may be done in
|
||
parallel.
|
||
|
||
---
|
||
|
||
## 0. Agent protocol
|
||
|
||
**Read these before touching anything**, in this order:
|
||
|
||
1. `CLAUDE.md` in this repository — project goals, the `libakerror` convention, the error-code
|
||
range map, and the dependency versions.
|
||
2. `deps/libakerror/AGENTS.md` — the `ATTEMPT`/`CLEANUP`/`PROCESS`/`HANDLE`/`FINISH` protocol.
|
||
3. `deps/libakerror/UPGRADING.md` — 1.0.0's status registry. Required before writing an error
|
||
code; the mechanism it replaced is gone.
|
||
4. `deps/libakstdlib/TODO.md` §2.1 — six **confirmed** defects in the library this port calls
|
||
into. §1.9 below says which calls are cleared for use; that section is not optional
|
||
reading, it bans a family of functions the port would otherwise reach for by reflex.
|
||
5. `deps/libakgl/AGENTS.md` — the no-`malloc` rule and the commit co-author requirement.
|
||
6. `deps/basicinterpret/README.md` — the language reference and the unimplemented list.
|
||
|
||
**Rules for working this file:**
|
||
|
||
- **Do not** mark an item done until its acceptance command passes on a clean out-of-tree
|
||
build. "It compiles" is not acceptance.
|
||
- **Do** update this file in the same commit as the work: strike the item, or replace it with
|
||
the defect it uncovered. This file holds outstanding items only.
|
||
- **Do** add the agent program name, model name and version as a commit co-author. That rule
|
||
comes from `libakgl` and applies here.
|
||
- **Never** hand-edit generated output. `build/` trees and the generated `akerror.h` are
|
||
off-limits; change the generator.
|
||
- **Never** reformat a file you are not otherwise changing.
|
||
- When a step is blocked because `libakgl` cannot supply a capability, **do not work around
|
||
it here.** File it in `deps/libakgl/TODO.md` in the numbered prose style of that file's
|
||
"Carried over" section and note the block in §7 below.
|
||
|
||
**Style**, restated so nobody has to go look: C99, 4-space indent, tabs at width 8
|
||
(`stroustrup`), function-body braces in column 0 on their own line, control braces on the same
|
||
line, **always brace**, spaces inside control-flow parens — `if ( x == y ) {`. Pointer star
|
||
binds to the identifier: `char *name`. Prefix is `akbasic_` for functions,
|
||
`akbasic_TypeName` for types, `AKBASIC_UPPER_SNAKE` for macros. `static` helpers drop the
|
||
prefix. Parameter names must match between header and source.
|
||
|
||
---
|
||
|
||
## 1. Design decisions already made
|
||
|
||
These are settled. Do not relitigate them mid-port; if evidence says one is wrong, say so in
|
||
a commit that changes it deliberately, and update this section.
|
||
|
||
### 1.1 Reflection becomes one aligned dispatch table
|
||
|
||
The Go runtime resolves verbs with `reflect.MethodByName("Command" + NAME)`
|
||
(`basicruntime.go:401`), functions with `"Function" + NAME`, and special parse paths with
|
||
`"ParseCommand" + NAME` (`basicparser.go:103`). C has no reflection and we are not adding
|
||
any. Replace all three with **one** static table in `src/verbs.c`, sorted by name, searched
|
||
with `bsearch(3)`:
|
||
|
||
```c
|
||
/* name token type parse handler exec handler */
|
||
{ "AUTO", AKBASIC_TOK_CMDIMM, NULL, cmd_auto },
|
||
{ "DATA", AKBASIC_TOK_COMMAND, parse_data, cmd_data },
|
||
{ "DEF", AKBASIC_TOK_COMMAND, parse_def, cmd_def },
|
||
```
|
||
|
||
A `NULL` parse handler means "parse the rval as a plain expression", which is exactly what
|
||
`commandByReflection` returning `(nil, nil)` means today. Adding a verb is adding one row plus
|
||
two functions. **Keep the table column-aligned and one row per verb** — it is a table, so it
|
||
gets laid out as one.
|
||
|
||
This also kills the Go scanner's three separate maps (`reservedwords`, `commands`,
|
||
`functions`, `basicscanner.go:64-67`): the token type lives in the same row.
|
||
|
||
### 1.2 Strings are fixed-size and live inline
|
||
|
||
`libakstdlib` has no string type. `libakgl` has `akgl_String` but it is `PATH_MAX` bytes,
|
||
refcounted, and pool-allocated — wrong shape for a value that gets copied on every
|
||
assignment, and it would drag a `libakgl` dependency into the core interpreter.
|
||
|
||
Define in `include/akbasic/types.h`:
|
||
|
||
```c
|
||
#define AKBASIC_MAX_STRING_LENGTH 256 /* matches AKBASIC_MAX_LINE_LENGTH */
|
||
```
|
||
|
||
and give `akbasic_Value` a `char stringval[AKBASIC_MAX_STRING_LENGTH]` **inline**. `clone()`
|
||
becomes a struct assignment. No allocator, no refcount, no lifetime question.
|
||
|
||
**Tradeoff, stated:** every `akbasic_Value` is ~300 bytes, so one environment's
|
||
`values[AKBASIC_MAX_VALUES]` pool is ~19KB, and 32 environments is ~610KB of BSS. That is
|
||
fine on a PC and is the price of never calling `malloc`. If it ever isn't fine, the knob is
|
||
`AKBASIC_MAX_STRING_LENGTH`, not the allocator.
|
||
|
||
Truncation is an **error**, not a silent clamp: `FAIL_RETURN(e, AKBASIC_ERR_VALUE, ...)`.
|
||
|
||
### 1.3 Maps become fixed-capacity open-addressed tables
|
||
|
||
Five Go maps need replacing:
|
||
|
||
| Go site | Purpose | C replacement |
|
||
|---|---|---|
|
||
| `BasicScanner.reservedwords/commands/functions` | keyword → token type | the §1.1 static table + `bsearch` |
|
||
| `BasicEnvironment.variables` | name → `*BasicVariable` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_VARIABLES` |
|
||
| `BasicEnvironment.functions` | name → `*BasicFunctionDef` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_FUNCTIONS` |
|
||
| `BasicEnvironment.labels` | name → line number | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_LABELS` |
|
||
|
||
One implementation, `src/symtab.c`, keyed by `aksl_strhash_djb2()` (already in
|
||
`libakstdlib`) with linear probing and a fixed slot array. **Use the existing hash; do not
|
||
write another one.** Table full is an error, not a resize.
|
||
|
||
Caveat, from `deps/libakstdlib/TODO.md` §1.6/§2.2.6: the wrapper sign-extends `char`, so a
|
||
high-bit byte hashes wrong — `"\xff\xfe"` returns 5859874 where the `unsigned char` answer is
|
||
5868578. BASIC identifiers are 7-bit ASCII (the scanner only accepts `IsLetter`/`IsDigit` plus
|
||
a type suffix), so this cannot bite the symbol tables. It **would** bite if anyone later keys
|
||
a table on a string literal or a filename. Do not work around it here; it is already filed
|
||
upstream.
|
||
|
||
### 1.4 Environments come from a pool and are released
|
||
|
||
Go calls `new(BasicEnvironment)` at `basicruntime.go:121` and `basicparser_commands.go:124`
|
||
and never frees one. A long-running `GOSUB` or `FOR` in Go leaks; the GC eventually catches
|
||
some of it, and nothing in the tests notices.
|
||
|
||
C gets `HEAP_ENVIRONMENT[AKBASIC_MAX_ENVIRONMENTS]` with
|
||
`akbasic_env_acquire()` / `akbasic_env_release()`, in the shape of `akgl_heap_next_*`.
|
||
`akbasic_runtime_prev_environment()` **must** release the environment it pops. Pool
|
||
exhaustion is `AKBASIC_ERR_ENVIRONMENT`, reported with the current line number.
|
||
|
||
Watch the one place this is not a clean stack: `userFunction` (`basicruntime.go:348`) stores
|
||
a `BasicEnvironment` **by value** inside `BasicFunctionDef` and re-`init()`s it on every call.
|
||
In C the funcdef holds an `akbasic_Environment *` acquired at `DEF` time and reset per call —
|
||
it is owned by the funcdef, not the pool's free list, until the funcdef dies.
|
||
|
||
### 1.5 Output goes through a text sink backend
|
||
|
||
`Write()` and `Println()` (`basicruntime_graphics.go:140,148`) mirror every line to stdout
|
||
*and* to an SDL surface. That mirror is the only reason the golden-file suite works. Do not
|
||
reproduce it as a hardcoded pair of calls.
|
||
|
||
Define a record of function pointers, populated by an initializer — the house pattern:
|
||
|
||
```c
|
||
typedef struct akbasic_TextSink
|
||
{
|
||
void *self;
|
||
akerr_ErrorContext AKERR_NOIGNORE *(*write)(struct akbasic_TextSink *self, char *text);
|
||
akerr_ErrorContext AKERR_NOIGNORE *(*writeln)(struct akbasic_TextSink *self, char *text);
|
||
akerr_ErrorContext AKERR_NOIGNORE *(*readline)(struct akbasic_TextSink *self, char *dest, size_t len);
|
||
akerr_ErrorContext AKERR_NOIGNORE *(*clear)(struct akbasic_TextSink *self);
|
||
} akbasic_TextSink;
|
||
```
|
||
|
||
`akbasic_sink_init_stdio()` is in the library and `akbasic_sink_init_akgl()` is in the
|
||
akgl-backed module. The driver is supposed to pick: a default build selects stdio, while an
|
||
`AKBASIC_WITH_AKGL` build selects the SDL text path and mirrors its output to stdout. It does
|
||
not do that yet; §3 records the missing standalone frontend. Cursor arithmetic, wrapping and
|
||
scrolling belong to the akgl sink, not to the interpreter.
|
||
|
||
### 1.6 The interpreter steps; it does not run
|
||
|
||
Go's `run()` (`basicruntime.go:682`) is a `for {}` that owns the process until `MODE_QUIT`.
|
||
Goal 3 forbids that: a host game must be able to bound execution.
|
||
|
||
The library exposes:
|
||
|
||
```c
|
||
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_step(akbasic_Runtime *obj);
|
||
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_run(akbasic_Runtime *obj, int maxsteps);
|
||
```
|
||
|
||
`_step()` does exactly what one iteration of Go's `for {}` body does and returns.
|
||
`_run(obj, maxsteps)` loops `_step()` until the mode is `AKBASIC_MODE_QUIT` or `maxsteps`
|
||
steps have elapsed; `maxsteps <= 0` means unbounded, which is what the standalone driver
|
||
passes. **This is a deliberate restructure, and it must not change a single byte of golden
|
||
output.**
|
||
|
||
`QUIT` sets `AKBASIC_MODE_QUIT` and returns. **Nothing in the library calls `exit()`,
|
||
`abort()`, or `FINISH_NORETURN`.** `FINISH_NORETURN` appears exactly once in the tree, in
|
||
`src/main.c`.
|
||
|
||
### 1.7 Error codes are absolute, reserved from the 1.0.0 registry
|
||
|
||
`deps/libakerror` is at **1.0.0**. Read `deps/libakerror/UPGRADING.md` before writing an error
|
||
code; `CLAUDE.md`'s error-code section is the condensed version. Three things this changes
|
||
from what you may have seen in an older draft or in `libakgl`:
|
||
|
||
- `AKERR_MAX_ERR_VALUE` **no longer exists**. The name registry is sparse and takes any `int`.
|
||
There is nothing for a consumer to size, and no compile definition to set. Anywhere you find
|
||
one, delete it.
|
||
- `__AKERR_ERROR_NAMES` **no longer exists**. The table is private.
|
||
- Codes are **absolute integer constants at 256 or above**, never `AKERR_LAST_ERRNO_VALUE + N`.
|
||
The offset scheme is what produced the live `libakgl` collision documented in `CLAUDE.md`.
|
||
|
||
`akbasic` owns **512–767** per the range map in `CLAUDE.md`. Declare it as an `enum`, so the
|
||
values stay compile-time integer constants (`HANDLE` expands to `case` labels, which require
|
||
that) and adding a code does not mean renumbering an offset:
|
||
|
||
```c
|
||
#define AKBASIC_OWNER "akbasic"
|
||
|
||
enum {
|
||
AKBASIC_ERR_BASE = 512, /** Start of akbasic's reserved status range */
|
||
AKBASIC_ERR_SYNTAX = AKBASIC_ERR_BASE,
|
||
/** Parse-time grammar violation */
|
||
AKBASIC_ERR_TYPE, /** Incompatible types in an operation */
|
||
AKBASIC_ERR_UNDEFINED, /** Reference to an undefined verb, function or label */
|
||
AKBASIC_ERR_BOUNDS, /** Array subscript or pool index out of range */
|
||
AKBASIC_ERR_ENVIRONMENT, /** Environment pool exhausted or orphaned environment */
|
||
AKBASIC_ERR_VALUE, /** A value was malformed, truncated or unconvertible */
|
||
AKBASIC_ERR_STATE, /** A verb ran outside the block structure it requires */
|
||
AKBASIC_ERR_LIMIT = AKBASIC_ERR_BASE + 256
|
||
};
|
||
```
|
||
|
||
`akbasic_init()` reserves the **whole** 256 in one call and then names each code. Both
|
||
registry calls return `akerr_ErrorContext *` and are `AKERR_NOIGNORE`, so a collision is an
|
||
ordinary error — `PASS` it and let it propagate out of init:
|
||
|
||
```c
|
||
akerr_ErrorContext AKERR_NOIGNORE *akbasic_init(void)
|
||
{
|
||
PREPARE_ERROR(errctx);
|
||
|
||
PASS(errctx, akerr_reserve_status_range(AKBASIC_ERR_BASE,
|
||
AKBASIC_ERR_LIMIT - AKBASIC_ERR_BASE,
|
||
AKBASIC_OWNER));
|
||
PASS(errctx, akerr_register_status_name(AKBASIC_OWNER, AKBASIC_ERR_SYNTAX,
|
||
"Syntax Error"));
|
||
/* ... one per code ... */
|
||
SUCCEED_RETURN(errctx);
|
||
}
|
||
```
|
||
|
||
Reserve the whole range in **one** call — a subset or superset of your own range raises
|
||
`AKERR_STATUS_RANGE_OVERLAP`, not a no-op. Use `akerr_register_status_name()`, never the
|
||
two-argument `akerr_name_for_status(status, name)` set path: the owned form is the one that
|
||
catches a component writing into a range that is not its own, and the two-argument form is
|
||
what let `libakgl` silently clobber `libakerror`'s names.
|
||
|
||
There is no startup ceiling to assert any more — the BSS-overflow hazard the old draft
|
||
defended against was deleted along with `AKERR_MAX_ERR_VALUE`. What replaces it is the
|
||
reservation itself: if `akbasic_init()` returns an error, something else owns part of 512–767
|
||
and the process must not continue as though it does not.
|
||
|
||
Do not call `akerr_init()` first. Every registry entry point calls it, and since 1.0.0 it no
|
||
longer clears reservations made before it ran.
|
||
|
||
### 1.8 Error message text is part of the acceptance contract
|
||
|
||
`tests/language/array_outofbounds.txt` is, verbatim:
|
||
|
||
```
|
||
? 20 : RUNTIME ERROR Variable index access out of bounds at dimension 0: 4 (max 2)\n\n
|
||
```
|
||
|
||
The trailing double newline is real: `basicError` builds a string ending in `\n` and hands it
|
||
to `Println`, which adds another. **Reproduce the message strings and the newline behaviour
|
||
byte for byte** or the golden suite fails for reasons that have nothing to do with the
|
||
interpreter. Where a Go message reads awkwardly, keep it awkward and note it here.
|
||
|
||
Numeric formatting must match too: integers via `%" PRId64 "`, floats via `%f` (Go's `%f` and
|
||
C's `%f` both give six decimals — `tests/language/arithmetic/float.txt` confirms).
|
||
|
||
### 1.9 Which `libakstdlib` calls are cleared for use — **the bans are lifted**
|
||
|
||
`deps/libakstdlib` is at **0.2.0**, and that release *fixed all six* of the confirmed defects
|
||
this section was built around, plus seventeen contract gaps. The table of bans that used to
|
||
live here is gone because it is no longer true, and leaving it would send an agent around a
|
||
workaround for a function that now works.
|
||
|
||
What changed, in the four that mattered to this port:
|
||
|
||
| Was banned | Now |
|
||
|---|---|
|
||
| `aksl_atoi` / `_atol` / `_atoll` / `_atof` | Every one takes a `dest` pointer and raises `AKERR_VALUE` on no digits or trailing junk, `ERANGE` on overflow — exactly the contract this section demanded |
|
||
| `aksl_list_append` / `_iterate` | Append no longer truncates to two nodes; iterate no longer skips the first half |
|
||
| `aksl_tree_iterate` | `AKERR_ITERATOR_BREAK` stops the traversal |
|
||
| `aksl_realpath` | Rewritten; no longer reads uninitialised memory on its error path |
|
||
|
||
Two signature changes reach this repository. `aksl_fread` and `aksl_fwrite` now take a
|
||
required `size_t *nmemb_out` and report a short transfer as `AKERR_IO` instead of a silent
|
||
success — so a `DSAVE` onto a full disk is now an error a program sees, where before it
|
||
reported nothing. `src/runtime_commands.c` passes the count and discards it, which is correct:
|
||
the library does the noticing now.
|
||
|
||
**`src/convert.c` is gone.** It existed only because `aksl_ato*` could not report a conversion
|
||
failure, and its own note said what to do when that changed: *"When it grows it, delete
|
||
`src/convert.c` and switch the call sites over."* That is done. Six call sites now go straight
|
||
to the library — `newLiteralInt` and `newLiteralFloat` in `src/grammar.c`, the line number in
|
||
`src/scanner.c`, `FunctionVAL` in `src/runtime_functions.c`, `INPUT` in
|
||
`src/runtime_commands.c`, and `GSHAPE`'s handle in `src/runtime_graphics.c`.
|
||
|
||
Three things that fell out of it, recorded because each was checked rather than assumed:
|
||
|
||
- **`aksl_strtoll(str, NULL, base, &dest)` is the exact replacement**, not `aksl_atoll`, wherever
|
||
a base is involved. The `ato*` forms are base 10; `src/grammar.c` picks base 8 or 16 from the
|
||
lexeme's prefix. A `NULL` endptr is what makes trailing junk an error rather than a stopping
|
||
point, and that is the whole contract this port needs.
|
||
- **The raised status changed** from `AKBASIC_ERR_VALUE` to `AKERR_VALUE`, and the message text
|
||
with it — `VAL("garbage")` now says `no digits in "garbage"`. §1.8 makes message text part of
|
||
the acceptance contract, so this was checked against the corpus first: **no golden file
|
||
contained a conversion message**, which is also why §1.9 used to ask for one. Two now exist,
|
||
`tests/language/numeric/val_refuses_garbage` and `octal_literal`, so the next change to that
|
||
text has to move a golden file.
|
||
- **`tests/convert.c` became `tests/numeric_contract.c`.** The assertions did not become
|
||
worthless when the wrapper went away — they became assertions about a contract this port
|
||
depends on and no longer owns, which is worth pinning exactly because a regression in it would
|
||
make `VAL("garbage")` return 0 silently. Same reasoning `libakstdlib`'s own
|
||
`tests/test_status_registry.c` gives for pinning that it reserves no status range.
|
||
|
||
One caveat survives the upgrade unchanged: `aksl_strhash_djb2` still sign-extends `char`, so a
|
||
high-bit byte hashes differently from the `unsigned char` answer. BASIC identifiers are 7-bit
|
||
ASCII so the symbol tables cannot reach it — see §1.3, which is still accurate.
|
||
|
||
---
|
||
|
||
## 2. What exists — **the core port is complete and green**
|
||
|
||
Phases 0 through 6 of the original plan are done. The interpreter builds clean under
|
||
`-Wall -Wextra`, reproduces the reference byte for byte, and passes under ASan and UBSan.
|
||
|
||
```sh
|
||
cmake -S . -B build && cmake --build build --parallel
|
||
ctest --test-dir build --output-on-failure # 59/59
|
||
cmake -S . -B build-asan -DAKBASIC_SANITIZE=ON # 59/59
|
||
cmake -S . -B build-cov -DAKBASIC_COVERAGE=ON # 92.3% line, 96.9% function
|
||
```
|
||
|
||
| Module | Source | Reference |
|
||
|---|---|---|
|
||
| Symbol table | `src/symtab.c` | the five Go maps |
|
||
| Value | `src/value.c` | `basicvalue.go` |
|
||
| Variable | `src/variable.c` | `basicvariable.go` |
|
||
| Tokens and AST leaves | `src/grammar.c` | `basicgrammar.go` + `basicparser.go` |
|
||
| Dispatch table | `src/verbs.c` | the three reflection lookups |
|
||
| Scanner | `src/scanner.c` | `basicscanner.go` |
|
||
| Parser | `src/parser.c`, `src/parser_commands.c` | `basicparser*.go` |
|
||
| Environment | `src/environment.c` | `basicenvironment.go` |
|
||
| Runtime | `src/runtime.c` | `basicruntime.go` minus SDL |
|
||
| Verbs and functions | `src/runtime_commands.c`, `src/runtime_functions.c` | `basicruntime_{commands,functions}.go` |
|
||
| Text sink | `src/sink_stdio.c` | `basicruntime_graphics.go`'s stdout mirror |
|
||
| Stdio driver | `src/main.c` | `main.go` minus its SDL frontend |
|
||
| Embedding examples | `examples/embed.c`, `examples/hostvars.c` | *(new)* |
|
||
|
||
`akbasic_runtime_load(rt, source)` was added while writing the README's embedding
|
||
section: a host usually holds its script as a string and wants the sink reserved for output,
|
||
and the only path that existed — `AKBASIC_MODE_RUNSTREAM` reading through the sink's
|
||
`readline` — forces a game to point its output device at its source text. `examples/embed.c`
|
||
is the code the README quotes, built by every build and registered as a CTest case so a
|
||
signature change breaks the build rather than rotting the document.
|
||
|
||
**The acceptance suite is the reference's own corpus, driven in place.** All 41 `.bas` files
|
||
under `deps/basicinterpret/tests/` are registered as individual CTest cases and byte-compared
|
||
against their `.txt` — including the trailing double newline on an error line (§1.8). Nothing
|
||
is copied, so the corpus cannot drift from its upstream.
|
||
|
||
Eighteen unit tests cover the modules the corpus cannot reach on its own.
|
||
`tests/known_reference_defects.c` is registered in `AKBASIC_KNOWN_FAILING_TESTS` and asserts
|
||
the **correct** contract for six of the defects in §6; when one is fixed CTest reports
|
||
"unexpectedly passed", which is the cue to split that assertion out and strike the item.
|
||
|
||
Two budget numbers moved during implementation and are worth knowing:
|
||
`AKBASIC_MAX_ARRAY_ELEMENTS` (1024) caps one array and `AKBASIC_MAX_ARRAY_VALUES` (4096) caps
|
||
all of them together, drawn from an `akbasic_ValuePool` the runtime owns. The reference had no
|
||
such ceiling because it called `make()`.
|
||
|
||
---
|
||
|
||
|
||
## 3. The akgl-backed sink and devices — **adaptors done; standalone frontend missing**
|
||
|
||
`-DAKBASIC_WITH_AKGL=ON` builds and its suite passes. Four adaptors in the `akbasic_akgl`
|
||
target, which is the only thing here that links SDL, are complete and tested:
|
||
|
||
| File | Backs |
|
||
|---|---|
|
||
| `src/sink_akgl.c` | the §1.5 text sink, over `akgl_text_measure` and `akgl_text_rendertextat` |
|
||
| `src/graphics_akgl.c` | `akbasic_GraphicsBackend`, over `akgl_draw_*`, and the SSHAPE surface pool |
|
||
| `src/audio_akgl.c` | `akbasic_AudioBackend`, over `akgl_audio_*` |
|
||
| `src/input_akgl.c` | `akbasic_InputBackend`, over `akgl_controller_poll_key` |
|
||
|
||
The character grid comes from `akgl_text_measure(font, "A", &w, &h)` — the direct equivalent of
|
||
the `font.SizeUTF8("A")` at `basicruntime.go:96`, and the call this was blocked on until
|
||
42b60f7. Wrapping is done on the character grid rather than by handing SDL_ttf a `wraplength`,
|
||
because the cursor has to land somewhere definite: a program that `PRINT`s a long string and
|
||
then `PRINT`s again expects the second to start on the row after the first ended, and only the
|
||
code that placed the characters knows which row that is.
|
||
|
||
**The interpreter owns no window, renderer or event loop.** Every initializer takes something
|
||
the host already made. Each calls `akgl_error_init()` first: `akgl_game_init()` would have,
|
||
but we drive subsystems directly and never call it, and a code raised before that registration
|
||
carries no name into its stack trace. It is idempotent.
|
||
|
||
The standalone executable is itself a host, and that part of the Go frontend has not been
|
||
ported. `CMakeLists.txt` links `akbasic_akgl` into `basic` and defines
|
||
`AKBASIC_HAVE_AKGL`, but `src/main.c` never tests that definition. It always initializes the
|
||
stdio sink, attaches no device backends, and runs without creating a window or pumping an SDL
|
||
event. The result is a terminal program with unused SDL code linked into it.
|
||
|
||
*Acceptance:* `tests/akgl_backends.c`, a 128x128 software renderer under the dummy video
|
||
driver read back with `SDL_RenderReadPixels` — the pattern `deps/libakgl/tests/draw.c`
|
||
established, which needs no display and no offscreen harness. Registered as the `akgl_backends`
|
||
CTest case, and only when `AKBASIC_WITH_AKGL` is on.
|
||
|
||
### Four things that had to be worked around to get there
|
||
|
||
All four are `libakgl`'s and all four are filed in `deps/libakgl/TODO.md`. Each workaround is
|
||
commented at its site with the words "filed upstream" so it can be found and deleted later.
|
||
|
||
1. **An embedded `libakgl` demands its dependencies be *installed*.** It builds its vendored
|
||
SDL, SDL_image, SDL_mixer, SDL_ttf and jansson only when it is top-level; embedded, it goes
|
||
down a `find_package` path and fails on a machine that has none of them — while the
|
||
submodules sit right there in `deps/libakgl/deps`. Every lookup is guarded with
|
||
`if(NOT TARGET ...)`, so our `CMakeLists.txt` adds those five subdirectories *before*
|
||
`add_subdirectory(deps/libakgl)` and the vendored copies get used after all. Same trick and
|
||
same ordering requirement `akerror::akerror` and `akstdlib::akstdlib` already need.
|
||
|
||
2. **`akgl/controller.h` does not compile on its own.** It declares two handler function
|
||
pointers taking an `akgl_Actor *` and includes nothing that declares the type.
|
||
`src/input_akgl.c` includes `akgl/actor.h` ahead of it.
|
||
|
||
3. **There is no way to attach a 2D backend to a renderer you already have.**
|
||
`akgl_render_init2d()` installs the six vtable pointers, but it also creates its own window
|
||
from the game properties and writes to the `camera` global, so it belongs to the
|
||
`akgl_game_init()` path — which is exactly the path an embedding host is not on.
|
||
`tests/akgl_backends.c` assigns the six pointers by hand. What is wanted upstream is the
|
||
vtable half of `init2d` on its own.
|
||
|
||
4. **`akgl_text_rendertextat()` segfaults on a backend whose vtable is empty.** It reaches
|
||
through `renderer->draw_texture` without checking it, so the first `PRINT` through the sink
|
||
dereferences NULL. That is the same class of defect 42b60f7's own commit added a draw test
|
||
for — "a backend that exists but was never given an `SDL_Renderer`" — and the text path
|
||
still has it.
|
||
|
||
### Still missing from the sink
|
||
|
||
`readline` reports end of input rather than reading anything: a drawn text layer is not a
|
||
source of lines, and `INPUT` through a graphics sink wants a line editor built on the keystroke
|
||
ring. EOF rather than an error is the contract `sink.h` states, so `INPUT` handles it already.
|
||
That editor is the next piece of work here, and it is what `WINDOW` and `KEY` in group E want
|
||
as well.
|
||
|
||
### Still missing from the standalone driver
|
||
|
||
**Port the Go program's SDL frontend when `AKBASIC_HAVE_AKGL` is defined.** This is separate
|
||
from the four adaptors above: they deliberately accept resources owned by a host, and
|
||
`src/main.c` is the host for the standalone executable.
|
||
|
||
The work touches `src/main.c`, `CMakeLists.txt`, the text-sink interface or a new composite
|
||
sink module, and the driver tests. It must:
|
||
|
||
1. Initialize SDL and SDL_ttf, create the 800x600 `BASIC` window and renderer, and open
|
||
`deps/basicinterpret/fonts/C64_Pro_Mono-STYLE.ttf` at 16 points, matching
|
||
`deps/basicinterpret/main.go`.
|
||
2. Initialize the akgl text, graphics, audio and input adaptors and attach the three device
|
||
records with `akbasic_runtime_set_devices()`.
|
||
3. Pump SDL events every frame, pass keyboard events to `akgl_controller_handle_event()`, and
|
||
make window close terminate the runtime cleanly.
|
||
4. Clear, draw the BASIC text with `akbasic_sink_akgl_render()`, and present the renderer every
|
||
frame. Keep interpreter execution bounded so events and repainting cannot starve.
|
||
5. Mirror `write`, `writeln` and `clear` to both the akgl sink and stdout through a composed
|
||
text-sink backend. **Do not** put a hardcoded second write in the interpreter; §1.5 made the
|
||
sink boundary specifically to avoid that coupling.
|
||
6. Preserve the current stdio-only driver when `AKBASIC_HAVE_AKGL` is absent. File input in an
|
||
AKGL build must still appear in the window and on stdout. Interactive REPL and `INPUT`
|
||
additionally depend on the line editor above; do not claim them complete until keyboard
|
||
editing, submission and echo all work in the SDL window.
|
||
|
||
**Acceptance:** add an SDL dummy-driver CTest that launches `basic` on a short `.bas` file,
|
||
asserts its stdout byte-for-byte, and reads back the renderer to prove the same text was drawn
|
||
with the Commodore font. Add a synthetic keyboard-event test that proves the standalone event
|
||
pump feeds the input backend. Finally, run `build-akgl/basic` under a real video driver and
|
||
verify the visible window, keyboard focus, stdout mirror and clean window-close path; record
|
||
that manual check here because the dummy driver cannot prove focus or presentation to a real
|
||
display.
|
||
|
||
---
|
||
|
||
## 4. Remaining work: language completion (goal 2)
|
||
|
||
The work queue is the "What Isn't Implemented" list in `deps/basicinterpret/README.md`.
|
||
Each verb is: table row (§1.1) → parse handler if it needs one → exec handler → unit test →
|
||
a new `.bas`/`.txt` pair. **Both** kinds of test; they answer different questions.
|
||
|
||
Order by what unblocks the most, and by what does not need `libakgl` to grow first:
|
||
|
||
| Group | Verbs | Blocked on |
|
||
|---|---|---|
|
||
| A. Structure | `DO`, `LOOP`, `WHILE`, `UNTIL`, `ON`, `BEGIN`, `BEND`, `END` | nothing — reuses `waitingForCommand` |
|
||
| B. Housekeeping | `NEW`, `CLR`, `CONT`, `RESTORE`, `RENUMBER`, `SWAP`, `TRON`, `TROFF`, `HELP` | nothing |
|
||
| 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 |
|
||
| ~~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 |
|
||
| ~~I. Audio~~ | ~~`PLAY`, `SOUND`, `ENVELOPE`, `VOL`, `TEMPO`~~ | **done** — `src/runtime_audio.c`, `src/play.c`, `tests/audio_verbs.c` |
|
||
| I′. Audio, still gapped | `FILTER`, `SOUND`'s sweep arguments | `libakgl` has neither; both refused with `AKBASIC_ERR_DEVICE`, both filed in §7 |
|
||
| ~~E′. Console input~~ | ~~`GET`, `GETKEY`, `SCNCLR`~~ | **done** — `src/runtime_input.c`, `tests/input_verbs.c` |
|
||
| J. Machine | `SYS`, `FETCH`, `STASH`, `POKE`/`PEEK` variants | nothing |
|
||
|
||
`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
|
||
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).
|
||
|
||
Also on the queue, from the reference's own defect list:
|
||
|
||
- **Multiple statements per line** (`10 PRINT A$ : REM ...`). The `COLON` token exists
|
||
(`basicscanner.go:40`) and nothing consumes it. This changes the parser's statement loop and
|
||
should be done before group A, because `DO`/`LOOP` bodies read badly without it.
|
||
- **Array references in parameter lists** (`READ A$(0), B#`) currently fail to parse. Fix in
|
||
`argumentList`.
|
||
|
||
---
|
||
|
||
## 5. Deliberate deviations from the reference
|
||
|
||
Keep this list current. Each of these changes structure without changing observable output,
|
||
and each one has to be defensible against the golden suite.
|
||
|
||
1. Reflection → static dispatch table (§1.1).
|
||
2. Go maps → fixed open-addressed tables (§1.3).
|
||
3. Unbounded `new(BasicEnvironment)` → pool with release (§1.4).
|
||
4. Hardcoded stdout+SDL mirror → text sink backend (§1.5).
|
||
5. `run()` owns the process → `step()` / bounded `run()` (§1.6).
|
||
6. `panic()` → `FAIL_RETURN`; no library call terminates the process (§1.6).
|
||
7. `debug.PrintStack()` on parse error → the `akerr` stack trace only (§3.1 of the original plan).
|
||
8. `BasicEnvironment.eval_clone_identifiers` deleted as dead state (§4.2 of the original plan).
|
||
9. `BasicValue.name` and `BasicEnvironment.update()` deleted as dead: nothing ever writes the
|
||
field a non-empty string, and `update()` — its only reader — has no callers.
|
||
10. The `DEF`-statement bootstrap is gone. The reference declares every builtin by *running a
|
||
BASIC program of `DEF` lines through the interpreter* at startup and then nulling out the
|
||
expressions (`basicruntime_functions.go:14`); here a builtin's name, arity and handler are
|
||
a row in the dispatch table, and `MOD`, `SPC` and `STR` are ordinary native handlers. This
|
||
removes the need to run the interpreter before the interpreter is ready.
|
||
11. Undefined behaviour the reference reaches by a defined route is refused rather than
|
||
inherited: a shift count outside 0..63, a negative string multiplier, and integer division
|
||
by zero all raise. Go defines all three (or panics); C does not. No golden case exercises
|
||
any of them, so observable behaviour is unchanged.
|
||
12. `CommandEXIT` clears the pending `NEXT` wait before popping. The reference does not, which
|
||
leaves the parent waiting for a `NEXT` that never arrives (§6 item 8) — in C that is a hang
|
||
rather than a misbehaviour, and no golden case depends on it.
|
||
|
||
### Deviations in the verbs the reference never implemented
|
||
|
||
Items 1–12 above are deviations *from ported code*. These are deviations from **Commodore
|
||
BASIC 7.0**, in verbs the reference lists as unimplemented and which therefore had no Go
|
||
behaviour to port. They matter to somebody typing in a listing out of a C128 manual.
|
||
|
||
13. **Hardware verbs reach a device through a backend record, and refuse when there is none.**
|
||
`akbasic_GraphicsBackend`, `_AudioBackend` and `_InputBackend` are records of function
|
||
pointers on the runtime; all three may be `NULL`, which is what the current stdio-only
|
||
standalone driver gives them. That is correct for a default build and unfinished for an
|
||
`AKBASIC_WITH_AKGL` build; see §3. A verb that needs a device it was not given raises
|
||
`AKBASIC_ERR_DEVICE` naming itself. `COLOR`, `LOCATE`, `SCALE`, `ENVELOPE`, `TEMPO` and
|
||
`VOL` deliberately do *not* require one — they change interpreter state, so a program can
|
||
configure itself before the host lends it a renderer.
|
||
|
||
14. **`CIRCLE` is a polygon, always.** BASIC 7.0's `CIRCLE` takes two radii, a start and end
|
||
angle, a rotation and a degree increment, which makes it an inc-degree polygon by
|
||
definition. `akgl_draw_circle` exists and is deliberately **not** used: it draws one
|
||
radius, full sweep, unrotated, so it could serve only the case where every optional
|
||
argument is defaulted — and a shape that changed character depending on whether the two
|
||
radii happened to be equal is worse than one that is uniformly a polygon.
|
||
|
||
15. **`SSHAPE` stores a handle in the string, not the pixels.** A real C128 packs the
|
||
region's bitmap into the string variable. Here a value's string is a fixed 256 bytes
|
||
(§1.2) and a saved region is a device surface, so the variable gets `SHAPE:<n>` and the
|
||
surface stays in a fixed pool on the backend. `GSHAPE` accepts only a string carrying that
|
||
prefix, so a hand-written one is refused rather than pasting an unrelated slot. **What
|
||
this costs:** a shape cannot be written to disk, cannot be concatenated, and `LEN` of it
|
||
is not its size. Nothing in BASIC does any of those to a shape string except a program
|
||
deliberately poking at it.
|
||
|
||
16. **`BOX` fills on a negative angle rather than on a seventh argument.** 7.0's last
|
||
argument selects outline or fill and sits *after* the rotation, which would make a filled
|
||
box a seven-argument call. That is filed rather than implemented; today a rotation of zero
|
||
outlines through the renderer's rectangle and any other rotation draws four lines.
|
||
|
||
17. **`GRAPHIC` records its mode but honours only one consequence of it.** 7.0's five modes
|
||
differ in bitmap resolution and in whether the bottom of the screen stays text. Neither
|
||
means anything against a host's renderer, whose size the interpreter does not know and
|
||
does not own. The mode is stored, an out-of-range one is refused because that is a typo
|
||
worth catching, and the one observable rule is that mode 0 is text.
|
||
|
||
18. **Coordinates reaching a backend are BASIC's 320x200 space, and scaling to the window is
|
||
the host's job.** The interpreter cannot ask the renderer how big it is without owning it.
|
||
`SCALE` maps user coordinates onto that space; with `SCALE` off they pass through.
|
||
|
||
19. **`PLAY` does not block.** On a C128 `PLAY` holds the program until the last note ends.
|
||
§1.6 forbids that outright, so `PLAY` parses the string into a fixed queue and returns;
|
||
`akbasic_runtime_step()` releases one note at a time against whatever time the host last
|
||
passed to `akbasic_runtime_settime()`. **What this changes for a program:** the statement
|
||
after a `PLAY` runs immediately rather than a bar later, so a listing that relied on `PLAY`
|
||
for timing will race ahead. A program that wants to wait should test the queue rather than
|
||
assume the verb waits for it.
|
||
|
||
20. **The host owns the clock, and an unset clock rushes the music.** The library reads no
|
||
clock — it owns no loop and must not block. A host calls `akbasic_runtime_settime()` once a
|
||
frame; the standalone driver calls it every step off `CLOCK_MONOTONIC`. Left unset it reads
|
||
zero forever, so every duration has already expired and the queue drains as fast as
|
||
`step()` is called. Audible, deterministic, and never a hang, which is the right way for
|
||
that to fail.
|
||
|
||
21. **`PLAY`'s `M` (measure) is a no-op and `SOUND`'s frequency sweep is refused.** `M`
|
||
synchronises the three voices on a C128; with one sequential queue there is nothing to
|
||
synchronise, and a listing full of them should still play, so it is accepted and ignored.
|
||
The sweep is the opposite call: `SOUND`'s `dir`/`min`/`step` arguments have no
|
||
`akgl_audio_*` equivalent, and the only way to fake one is to re-issue tones from
|
||
`step()` — which would tie audible pitch to how often the host happens to call us, a tune
|
||
that changes key with the frame rate. Filed upstream as `akgl_audio_sweep`; see §7.
|
||
|
||
22. **`TEMPO`'s constant is a calibration choice, not a transcription.** BASIC 7.0 documents
|
||
`TEMPO` as a relative duration from 1 to 255 defaulting to 8, and does not publish what a
|
||
whole note actually lasts. `src/audio_tables.c` uses 16000 ms at `TEMPO` 1, which puts a
|
||
default-tempo quarter note at 500 ms — 120 beats per minute. If the real constant turns
|
||
up, that is the one line to change.
|
||
|
||
23. **`GETKEY` holds the step loop rather than blocking.** A C128 sits on the keyboard inside
|
||
`GETKEY` until somebody types. §1.6 forbids the library blocking, so the verb sets a flag
|
||
and `akbasic_runtime_step()` declines to advance the program until a key arrives. Every
|
||
step still returns and a bounded `akbasic_runtime_run()` still comes back, so a host keeps
|
||
its frame rate; the program simply does not move past the `GETKEY`, which is the part a
|
||
program author cares about. The `PLAY` queue is serviced *before* this check on purpose —
|
||
music should keep playing while a program waits for a keypress. Withdrawing the input
|
||
device while a `GETKEY` is holding releases it rather than wedging the script forever.
|
||
|
||
24. **`GET` and `GETKEY` into a numeric variable yield the key code.** BASIC 7.0's `GET` takes
|
||
a string variable. Accepting an integer one as well, and giving it the raw code, is what a
|
||
program testing for cursor or function keys needs and costs nothing. No key is code zero,
|
||
matching what a C128 reports. A float variable is refused: it is neither a character nor a
|
||
code.
|
||
|
||
---
|
||
|
||
## 6. Reference defects — ported faithfully, fix them deliberately
|
||
|
||
These are real bugs in `deps/basicinterpret`. **Port the behaviour first** so the golden suite
|
||
passes and the port is provably faithful, then fix each one in its own commit with a test that
|
||
asserts the correct contract. Until fixed, the correct-contract test lives in
|
||
`AKBASIC_KNOWN_FAILING_TESTS`.
|
||
|
||
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.
|
||
10. **`basicgrammar.go:224`** — `newLiteralInt` selects base 8 whenever the lexeme starts with
|
||
`0`. *Consequence:* `PRINT 08` is a parse error and `PRINT 010` prints 8. Commodore BASIC
|
||
has no octal literals. Fix: base 10 unless prefixed `0x`.
|
||
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. Each is reproduced faithfully,
|
||
pinned by an assertion in the relevant unit test, and asserted *correctly* in
|
||
`tests/known_reference_defects.c`.
|
||
|
||
12. **`basicparser.go:329` — `subtraction()` returns after one operator where `addition()`
|
||
loops, so `1 - 2 - 3` computes `1 - 2` and abandons the rest of the line.** The remaining
|
||
`- 3` is left in the token stream and the statement loop picks it up as a second statement,
|
||
which evaluates and is discarded. *Consequence:* a chain of two or more subtractions
|
||
silently produces the wrong answer. This is the highest-impact item on the list — it is a
|
||
wrong result, not a refused one. The same shape appears in `exponent()`, so `2 ^ 3 ^ 2`
|
||
has it too. Fix: make both loop the way `addition()` does. Pinned in
|
||
`tests/parser_expressions.c`.
|
||
|
||
13. **`basicparser.go:565` — a unary-minus argument inflates a function's arity count.** The
|
||
counter walks the `.right` chain, and a unary leaf keeps its operand on `.right`, so
|
||
`ABS(-9)` is counted as two arguments and refused with "function ABS takes 1 arguments,
|
||
received 2". *Consequence:* no builtin can be called with a negative literal. The corpus
|
||
hides it — `sgn.bas` assigns `-1` to a variable first. Fix: count only the top-level
|
||
argument chain, not whatever a leaf hangs off `.right`. Pinned in
|
||
`tests/runtime_evaluate.c`.
|
||
|
||
14. **`basicscanner.go:272` — `matchNextChar` returns without setting a token type when it
|
||
cannot peek past the end of the line, so a comparison operator in the final column is
|
||
silently dropped.** `A# =` produces one token, not two. *Consequence:* a line ending in a
|
||
bare `<`, `>` or `=` loses it, and the parse error that follows points at the wrong place.
|
||
Fix: set `falsetype` before returning on the peek failure. Pinned in
|
||
`tests/scanner_tokens.c`.
|
||
|
||
15. **`basicscanner.go:318` — hex literals do not survive the scanner.** `matchNumber` allows
|
||
`x` through but not the hex digits after it, so `0xff` lexes as `0x` and `ff` is scanned
|
||
as a separate identifier. *Consequence:* the base-16 branch in `newLiteralInt`
|
||
(`basicgrammar.go:224`) is unreachable, and the README's hex support does not exist. Fix:
|
||
once `x` has been seen, accept `[0-9a-fA-F]`. Pinned in `tests/scanner_tokens.c`.
|
||
|
||
16. **`basicscanner.go:349` — the "Reserved word in variable name" check never fires.** The
|
||
lexeme still carries its type suffix when the keyword tables are searched, so `PRINT$`
|
||
misses every table and becomes an ordinary string variable. *Consequence:* the diagnostic
|
||
is dead code and `PRINT$ = 1` is accepted. Fix: strip the suffix before the lookup. Pinned
|
||
in `tests/scanner_tokens.c`.
|
||
|
||
### Ours, not the reference's
|
||
|
||
17. **A host cannot reliably create a script variable while a script is suspended.** This one
|
||
is not inherited — it falls out of §1.4's environment pool meeting §1.6's bounded `run()`,
|
||
a combination the reference never had because its `run()` never returned.
|
||
|
||
Exchanging variables with an embedding host works and is documented in `README.md`: the
|
||
host calls `akbasic_environment_get()` to find or create a variable and
|
||
`akbasic_variable_set_*` / `_get_subscript` to move values across. Seeding before
|
||
`akbasic_runtime_start()` and reading after the script stops is safe, and so is *updating*
|
||
an existing global at any time, because the parent chain is searched. **Creating** one
|
||
mid-run is not, in two ways, and both are silent:
|
||
|
||
- A script suspended part-way through a bounded `akbasic_runtime_run()` is usually inside a
|
||
`FOR` or `GOSUB` scope, so `obj->environment` is that scope. A variable created through
|
||
it lands in the loop's scope and dies when the environment pops — the script reads the
|
||
value correctly inside the loop and gets `0` immediately after it.
|
||
- Reaching for the root explicitly does not help. `akbasic_environment_get` only
|
||
auto-creates when `obj->runtime->environment == obj` (`src/environment.c:242`), so with a
|
||
child active it returns `NULL` through `dest` **without raising**, and an unchecked host
|
||
dereferences it.
|
||
|
||
`examples/hostvars.c` demonstrates both rather than describing them, and prints what it
|
||
observes, so the day this is fixed that output changes and the example needs revisiting.
|
||
|
||
Fix: add `akbasic_runtime_global(akbasic_Runtime *obj, const char *name, akbasic_Variable **dest)`
|
||
that walks `obj->environment` to the root and creates there unconditionally, and point the
|
||
README at it instead of at `akbasic_environment_get`. Roughly fifteen lines. The one design
|
||
question worth settling first is what it should do when the script is suspended inside a
|
||
*user function's* scope — that environment is owned by the funcdef rather than the pool
|
||
(§1.4), and "root" is still the right answer there, but it is worth being deliberate about
|
||
it rather than discovering it. Tests: create a global while suspended in a `FOR` body and
|
||
assert the script still sees it after `NEXT`; the same from inside a `GOSUB`; and the
|
||
NULL-without-error path, which should become an impossible state.
|
||
|
||
---
|
||
|
||
## 7. Filing gaps against `libakgl`
|
||
|
||
When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in
|
||
`deps/libakgl/TODO.md` in that file's numbered prose style: what the BASIC verb requires, what
|
||
the `akgl_*` entry point should look like, and what tests would cover it. Growing `libakgl` to
|
||
serve the interpreter is a wanted outcome, not a detour.
|
||
|
||
**All four gaps filed so far are now closed upstream**, at `libakgl` 42b60f7. Nothing here is
|
||
blocked on `libakgl` any more; what remains is akbasic-side work.
|
||
|
||
| Was blocking | Closed by |
|
||
|---|---|
|
||
| Text measurement (§3, the akgl sink) | `akgl_text_measure`, `akgl_text_measure_wrapped` |
|
||
| Immediate-mode drawing (§4 group G) | `akgl_draw_point`/`_line`/`_rect`/`_filled_rect`/`_circle`/`_flood_fill`/`_copy_region`/`_paste_region` |
|
||
| Audio (§4 group I) | `akgl_audio_init`/`_tone`/`_envelope`/`_waveform`/`_volume`/`_stop`/`_voice_active`/`_mix` |
|
||
| Console input (§4 group E) | `akgl_controller_poll_key`, `akgl_controller_flush_keys` |
|
||
|
||
Two notes for whoever picks up §3 next. The draw calls take an `akgl_RenderBackend *` as their
|
||
first argument rather than reaching for a global renderer, which fits the rule that the
|
||
interpreter draws through whatever renderer the host already initialized. And the audio API is
|
||
a synthesised-voice one — `akgl_audio_tone(voice, hz, ms)` with a separate ADSR envelope —
|
||
which is the shape `PLAY` and `ENVELOPE` need, rather than the sample playback SDL3_mixer
|
||
would have given.
|
||
|
||
Building against any of it needs `-DAKBASIC_WITH_AKGL=ON`, which pulls in `libakgl`'s own
|
||
submodules (SDL and friends). Those are not initialized in a default clone; `git submodule
|
||
update --init --recursive` gets them.
|
||
|
||
**Six items are filed upstream and outstanding**, at `deps/libakgl/TODO.md` items 5-9 of "API
|
||
gaps blocking akbasic" and item 14 of "Defects". Four are workarounds this repository carries
|
||
today, each commented at its site with the words "filed upstream" so it can be found and
|
||
deleted: the embedded-dependency CMake trick in our `CMakeLists.txt`, the `akgl/actor.h`
|
||
include in `src/input_akgl.c`, and the hand-populated vtable and its NULL-deref hazard in
|
||
`tests/akgl_backends.c`. §3 describes all four. The fifth is `SOUND`'s frequency sweep and the
|
||
sixth is the missing version bump recorded in §8.
|
||
|
||
**One further gap is outstanding, and upstream named it itself.** The commit that added the audio API
|
||
says plainly what it does not cover: *"Still missing for a complete BASIC sound vocabulary:
|
||
`FILTER` (SDL3 has no filter primitive; this would need writing), `TEMPO` and the `PLAY`
|
||
note-string parser, both of which belong in the interpreter rather than here."*
|
||
|
||
- **`TEMPO` and the `PLAY` parser are ours**, not a gap. They are group I work in this
|
||
repository and nothing upstream is waiting on.
|
||
- **`FILTER` is a real gap.** BASIC 7.0's `FILTER freq, lp, bp, hp, res` sets the SID's
|
||
filter cutoff, the three band-pass switches and the resonance. `akgl_audio_*` synthesises
|
||
raw waveforms and mixes them; there is no filter stage to configure and SDL3 supplies no
|
||
primitive to build one from. Until an `akgl_audio_filter()` exists, `FILTER` is accepted by
|
||
the parser and refused at execution with `AKBASIC_ERR_DEVICE` rather than silently ignored —
|
||
a program that asks for a low-pass and gets an unfiltered square wave has been lied to.
|
||
|
||
A second gap turned up while implementing group I, and is filed alongside it:
|
||
|
||
- **`SOUND` has no frequency sweep.** BASIC 7.0's `SOUND voice, freq, dur, dir, min, step`
|
||
ramps the pitch from `freq` toward `min` in `step` increments per tick, in the direction
|
||
`dir` selects. `akgl_audio_tone(voice, hz, ms)` holds one pitch for one duration and there
|
||
is no entry point that changes pitch over the life of a note. The wanted API is roughly
|
||
`akgl_audio_sweep(int voice, float32_t from_hz, float32_t to_hz, float32_t step_hz, uint32_t ms)`,
|
||
advanced on the mixer's own frame counter — the same place the phase is already derived
|
||
from, and the reason it belongs there rather than here. Faking it in the interpreter means
|
||
re-issuing tones from `akbasic_runtime_step()`, which ties audible pitch to how often a host
|
||
calls us: a tune that changes key with the frame rate. Refused with `AKBASIC_ERR_DEVICE`
|
||
until it lands. Tests would drive `akgl_audio_mix` by hand and assert the frame at which the
|
||
pitch has moved, exactly as `tests/audio.c` already does for the envelope.
|
||
|
||
When the next gap turns up, file it there rather than working around it here.
|
||
|
||
---
|
||
|
||
## 8. Status
|
||
|
||
**The port is done.** The C interpreter reproduces the Go reference byte for byte across its
|
||
entire test corpus and passes clean under ASan and UBSan.
|
||
|
||
| Gate | Result |
|
||
|---|---|
|
||
| `ctest` | 72/72 — 41 upstream golden cases, 5 local ones, 21 unit tests, 2 embedding examples, 1 known-failing |
|
||
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 73/73 — the above plus `akgl_backends`; the `akgl_build` CI job |
|
||
| Golden corpus | 41/41 byte-exact, driven in place from the submodule |
|
||
| ASan + UBSan | 72/72 |
|
||
| Line coverage | 93.6% (3618/3867) — above the 90% gate |
|
||
| Function coverage | 97.8% (267/273) |
|
||
| Warnings | none under `-Wall -Wextra` |
|
||
| `doxygen Doxyfile` | clean |
|
||
| Mutation (`src/symtab.c`) | 74.1%, against a gate of 65 — see `.gitea/workflows/ci.yaml` |
|
||
|
||
Coverage of the verb groups added for goal 2, since they are the new surface: `src/audio_tables.c`
|
||
and `src/graphics_tables.c` 100%, `src/runtime_input.c` 100%, `src/runtime_graphics.c` and
|
||
`src/runtime_audio.c` 98%, `src/play.c` 87%. The `akbasic_akgl` target is not in the coverage
|
||
figure — it is not built in a default configuration.
|
||
|
||
**A note on reading that number, because it was misread once while producing it.** A stale
|
||
`build-cov/` left in the source directory from an earlier session was silently folded into the
|
||
report by `gcovr --root .`, which produced the *previous* run's 92.3% for a tree that had grown
|
||
by 800 lines. That is `libakgl` defect #13 happening here rather than there, and `build*/` being
|
||
gitignored is exactly what makes it invisible. **Keep build trees out of the source directory**,
|
||
and if a coverage number looks suspiciously unchanged, `find . -name '*.gcda'` before believing
|
||
it.
|
||
|
||
Branch coverage reads 18.0% and is not a target, for the reason `libakgl/TODO.md` and
|
||
`libakstdlib/TODO.md` both give: the akerror control-flow macros expand into large branch trees
|
||
at every call site, most of them unreachable in normal operation. Track line and function
|
||
coverage.
|
||
|
||
Dependency baseline:
|
||
|
||
| Submodule | Version | Notes |
|
||
|---|---|---|
|
||
| `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.2.0 | soname `libakgl.so.0.2`. 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. It requires `libakstdlib` 0.2, which is why the two moved together. |
|
||
|
||
**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
|
||
`libakgl.so.0.1` soname unchanged, so nothing here could feature-test for the new API and the
|
||
submodule pointer was the only thing expressing the requirement. That was filed upstream as
|
||
`libakgl` defect #14 and fixed by its commit 1066ac7 — the two crossed rather than one
|
||
following the other. `libakgl` is 0.2.0 with an `libakgl.so.0.2` soname, so
|
||
`AKGL_VERSION_AT_LEAST(0, 2, 0)` now means something and an ABI mismatch is refused at load
|
||
time. Kept here because the *reason* is the reusable part: an additive release under an
|
||
unchanged soname costs its consumers the ability to detect it at all.
|
||
|
||
CI is split in two. `.gitea/workflows/ci.yaml` runs on push -- the suite, ASan+UBSan, coverage
|
||
and a two-file mutation run -- and `.gitea/workflows/release.yaml` is manual
|
||
(`workflow_dispatch`), carrying the doxygen gate and the whole-tree mutation run. The split is
|
||
about cost: the full mutation set is 3675 mutants and hours of runner time, which is a release
|
||
gate rather than a per-commit one, and the docs are wanted at release time. Three notes worth
|
||
keeping:
|
||
|
||
- The checkout is `submodules: true`, **not** `recursive`. The build needs `libakerror`,
|
||
`libakstdlib` and `basicinterpret`; it does not need `libakgl`, which is guarded behind
|
||
`AKBASIC_WITH_AKGL` and defaults OFF — and recursing into it would clone SDL, SDL_image,
|
||
SDL_mixer, SDL_ttf and jansson for a target that is never configured. The `docs` job needs
|
||
no submodules at all.
|
||
- The push-path `mutation_test` job is bounded to `src/symtab.c`, scoring 74.1% against a gate
|
||
of 65. It was two files until `src/convert.c` was deleted. `src/audio_tables.c` was measured
|
||
as a replacement and scored 64.7%, below the gate — a real gap rather than a reason to skip
|
||
the file: almost every survivor is in `akbasic_audio_state_init`, where nothing asserts that
|
||
a freshly initialised audio state is actually zeroed and defaulted. That is the same gap this
|
||
job's history records closing for `src/symtab.c`, and closing it is what earns the file its
|
||
place back in CI.
|
||
|
||
Worth knowing before reading a score: the harness's `ICR` operator only rewrites the
|
||
constants `0` and `1`, so a lookup table of other values produces no mutants at all and a
|
||
high score over one says nothing about whether its entries are right. `tests/audio_verbs.c`
|
||
asserts all 32 ADSR entries anyway — that catches a hand-edit typo, which is the real failure
|
||
mode, but it did not and could not move the mutation number. `src/value.c` is the file most
|
||
worth mutating and is deliberately not on that path: 368 mutants at roughly 11 seconds each
|
||
is about 70 minutes, because almost everything links against it. It is covered by
|
||
`release.yaml`, or locally with `cmake --build build --target mutation`.
|
||
- `release.yaml`'s threshold defaults to 65, the same as the push path, rather than something
|
||
stricter. Only two files have ever been measured; a tighter whole-tree number would be a
|
||
guess until a full run establishes a baseline. Raise it through the workflow input once one
|
||
has.
|
||
|
||
What remains, in priority order:
|
||
|
||
1. **The standalone AKGL frontend in §3.** An AKGL-enabled `basic` currently remains the
|
||
stdio driver with unused SDL adaptors linked into it. Port the window, font, composite
|
||
stdout+SDL sink, device attachment, bounded frame loop, event pump and renderer presentation
|
||
from the Go frontend. The build option must change the executable's observable behaviour.
|
||
2. **A line editor for the akgl sink.** `readline` reports EOF, so the SDL frontend cannot
|
||
provide an interactive REPL or `INPUT` after it exists. It wants the keystroke ring plus a
|
||
cursor, and it is also what `WINDOW` and `KEY` in group E need. See §3.
|
||
3. **§4 — the language completion work queue.** Groups G and I and the `GET`/`GETKEY`/`SCNCLR`
|
||
part of E are done. Of what is left, **multiple statements per line** should come first:
|
||
the `COLON` token exists and nothing consumes it, and `DO`/`LOOP` in group A reads badly
|
||
without it. Then groups A, B, D, F and J, none of which need anything from `libakgl`.
|
||
4. ~~**CI does not cover `-DAKBASIC_WITH_AKGL=ON`.**~~ **Done** — the `akgl_build` job in
|
||
`.gitea/workflows/ci.yaml`. It turned out much cheaper than the deferral assumed, and the
|
||
two reasons are worth keeping because both were guesses that measurement corrected:
|
||
|
||
- **`submodules: recursive` is not needed and costs 461 MB.** It descends into
|
||
`SDL_image/external`, `SDL_mixer/external` and `SDL_ttf/external` — aom, dav1d, libjxl,
|
||
libtiff, mpg123, opus, flac and a dozen more — and *not one of them is used*. Every one
|
||
configures as "Could NOT find". The job initialises exactly six submodules
|
||
non-recursively instead, which takes about 25 seconds.
|
||
- **The build is about a minute of CPU, not "a long build".** 330 object files, because
|
||
SDL3 compiles out almost every backend that is not wanted here.
|
||
|
||
The one real system dependency is `libfreetype-dev` and `libharfbuzz-dev`: SDL_ttf prefers
|
||
the system copies and would otherwise reach for `external/freetype`, which the checkout
|
||
deliberately does not clone. Two apt packages against two more submodules is an easy trade.
|
||
|
||
Every step was verified from a clean clone before being written down, not inferred.
|
||
5. **§6 items 12–17** — the defects the port uncovered. Item 12 is the one that produces a
|
||
*wrong answer* rather than a refused one, and should be fixed first.
|
||
6. **Mutation survivors in `src/value.c`.** The harness is in place (`scripts/mutation_test.py`,
|
||
the `mutation` CMake target, and a CI job on `src/symtab.c`), and a partial run over
|
||
`src/value.c` turned up test gaps worth closing. These are *not* equivalent mutants; each
|
||
is a real bug of that shape the suite would not notice:
|
||
|
||
- `AKBASIC_MAX_STRING_LENGTH - 1` → `+ 1` and → `- 0` survive at `src/value.c:47-48`, in
|
||
`set_string`'s `strncpy` and its NUL terminator. Nothing in the suite writes a
|
||
*maximum-length* string, so the off-by-one that would truncate or overrun goes unseen.
|
||
One test that round-trips a 255-character string kills all five.
|
||
- `memset(obj, 0, ...)` → `memset(obj, 1, ...)` and its deletion survive in
|
||
`akbasic_valuepool_init`, as does `obj->next = 0` → `= 1`. Nothing asserts a freshly
|
||
initialised pool is actually empty and zeroed.
|
||
|
||
Two survivors there are genuinely equivalent and cannot be killed: `rval_as_int` and
|
||
`rval_as_float` (`src/value.c:30,35`) tolerate `+` → `-` because the reference's habit of
|
||
adding *both* of the right operand's numeric fields only works when the unused one is zero
|
||
— §6 item 5. Fixing that defect would make these two mutants killable, which is a small
|
||
argument for doing it.
|
||
|
||
`src/value.c` takes about 70 minutes to mutate in full (368 mutants, ~11s each, because
|
||
almost everything links against it), which is why CI runs `src/symtab.c` instead and the
|
||
whole-tree run is a local `cmake --build build --target mutation`.
|