Rebasing the defect filing onto libakgl's origin/main brought two upstream commits with it -- "Consume libakstdlib 0.2.0" and "Bump libakgl to 0.2.0" -- and libakgl's sources now use the 0.2.0 akstdlib API. Our CMakeLists declares akstdlib::akstdlib from our own tree first, so libakgl was compiling 0.2.0-era code against 0.1.0 headers and failing on aksl_fwrite, aksl_fread and aksl_realpath. The two have to move together. The code change is one call site. aksl_fwrite now takes a required size_t *nmemb_out and reports a short transfer as AKERR_IO rather than a silent success, so DSAVE onto a full disk is an error a program sees where before it reported nothing. The count is passed and discarded on purpose: the library does the noticing now. The documentation change is larger, because libakstdlib 0.2.0 fixed all six of the confirmed defects TODO.md section 1.9 was built around. That section was a table of bans; leaving it would send an agent around a workaround for functions that now work. The aksl_ato* family raises AKERR_VALUE on no digits or trailing junk and ERANGE on overflow -- exactly the contract that section demanded -- aksl_list_append no longer truncates, aksl_list_iterate no longer skips the first half, AKERR_ITERATOR_BREAK stops a tree traversal, and aksl_realpath no longer reads uninitialised memory. One caveat survives: aksl_strhash_djb2 still sign-extends char, which section 1.3 already covers and the symbol tables still cannot reach. src/convert.c has therefore outlived its reason, and is deliberately left in place. Its own note said to delete it when libakstdlib grew the contract, and that condition is now met -- but doing it touches four call sites, changes the raised status from AKBASIC_ERR_VALUE to AKERR_VALUE where section 1.8 says message text is part of the acceptance contract, and would silently gut the CI mutation job, which is bounded to src/convert.c and src/symtab.c. Section 1.9 now lists all of that. Worth doing on purpose rather than as a side effect of a version bump. libakgl defect #14 is closed by its own 1066ac7, which bumped it to 0.2.0 while this was being written. The requirement is no longer pinned by submodule commit in the README, because AKGL_VERSION_AT_LEAST(0, 2, 0) finally means something. 70/70 core, 71/71 with libakgl, 70/70 under ASan+UBSan, clean under -Wall -Wextra, doxygen clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
911 lines
56 KiB
Markdown
911 lines
56 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()` in the library, `akbasic_sink_init_akgl()` in the akgl-backed
|
||
module (phase 9). The driver picks. 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 the outstanding consequence, and it is deliberately not done yet.** It
|
||
exists only because `aksl_ato*` could not report a conversion failure, and its own note says
|
||
what to do when that changed: *"When it grows it, delete `src/convert.c` and switch the call
|
||
sites over."* That condition is now met. Doing it touches more than the two functions:
|
||
|
||
- Four call sites — `newLiteralInt`/`newLiteralFloat` in `src/grammar.c`, `matchNumber` in
|
||
`src/scanner.c`, and `FunctionVAL` in `src/runtime_functions.c`.
|
||
- `tests/convert.c`, which would move to asserting `libakstdlib`'s contract rather than ours.
|
||
- **`.gitea/workflows/ci.yaml`'s mutation job, which is bounded to `src/convert.c` and
|
||
`src/symtab.c`.** Deleting the file without repointing that job silently removes half of the
|
||
only gate that checks the error-handling control flow at all.
|
||
- The error *status* changes from `AKBASIC_ERR_VALUE` to `AKERR_VALUE`, which is visible in
|
||
the message text a program sees — so §1.8 applies and the golden files have to be checked.
|
||
|
||
Worth doing, and worth doing on purpose rather than as a side effect of a version bump.
|
||
|
||
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 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 |
|
||
|---|---|---|
|
||
| Strict numeric conversion | `src/convert.c` | *(new; see §1.9)* |
|
||
| 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 |
|
||
| Driver | `src/main.c` | `main.go` |
|
||
| 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 — **done**
|
||
|
||
`-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:
|
||
|
||
| 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.
|
||
|
||
*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.
|
||
|
||
---
|
||
|
||
## 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 standalone driver
|
||
gives them. 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` | 70/70 — 41 upstream golden cases, 5 local ones, 21 unit tests, 2 embedding examples, 1 known-failing |
|
||
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 71/71 — the above plus `akgl_backends`; the `akgl_build` CI job |
|
||
| Golden corpus | 41/41 byte-exact, driven in place from the submodule |
|
||
| ASan + UBSan | 70/70 |
|
||
| Line coverage | 93.5% (3631/3884) — above the 90% gate |
|
||
| Function coverage | 97.8% (269/275) |
|
||
| Warnings | none under `-Wall -Wextra` |
|
||
| `doxygen Doxyfile` | clean |
|
||
| Mutation (`src/convert.c`, `src/symtab.c`) | see `.gitea/workflows/ci.yaml` for the current score and gate |
|
||
|
||
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/convert.c` and `src/symtab.c`, about
|
||
four minutes between them, scoring 77.8% against a gate of 65. `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. **§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`.
|
||
2. **A line editor for the akgl sink.** `readline` reports EOF, so `INPUT` does not work
|
||
through a drawn text layer. It wants the keystroke ring plus a cursor, and it is also what
|
||
`WINDOW` and `KEY` in group E need. See §3.
|
||
3. ~~**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.
|
||
4. **§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.
|
||
5. **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/convert.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/convert.c` instead and the
|
||
whole-tree run is a local `cmake --build build --target mutation`.
|